harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexei Fedotov (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HARMONY-6039) [drlvm][classloader] On-demand class library parsing to avoid unnecessary jar/zip parsing during VM creation
Date Thu, 15 Jan 2009 11:09:59 GMT

    [ https://issues.apache.org/jira/browse/HARMONY-6039?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12664090#action_12664090

Alexei Fedotov commented on HARMONY-6039:

The line numbers in the review comments apply to the H6039.patch_2

excessive comment length

missed description of parameters (e.g. @param mapFile <description>)
and return value
do we need to pass mapFile through the parameter chain? may it be an element of

22, 24
we don't need both versions of each function, do we?
using one version (esp. of SetBCPElement) would make the whole code size smaller

it would be easier for me to review your deltas of functions if you
don't make the full copies of them

seems that environment.h has c/apr style set of includes
can we hide <map> and related typedef in sources to maintain C/apr
style of interfaces
is it possible to use more specific header (e.g. related to jar
parsing) than environment.h for JarFilePackageMapping definition?

*the* bootstrap classpath

the proper bracket style is specified here
[well, the whole file is formatted strangely - Pavel, could you comment?]

such -> the

*a* temp pool

putting map file operations into separate .cpp file with a clear and
readable interface function names in the corresponding .h interface
would not make existing code less readable

that .h file would be a proper place for new types you introduce, not

you may also use the proper Apache formatting in the new file

the explanatory comment is welcome
Why we are using two files instead of their concatenation in one file?

can this be replaced with assert(luni_path)?

+1 to Aleksey's comment on literals

[have to go, will continue later]

constant sizes for arrays on stack and heap worth at least comment and better be avoided

do we need both vmboot and requierdLib?

commented code

rewriting this 

if(entry) {
            *not_found = true;
            return NULL;

would make shifting of 40 lines to the right unnesesary (thus saving 1600 spaces and readability)

suggestion to add an asssert remains

My suggestion to remove cut&pasted code from copied class path parsing functions applies
to Jar::Parse as well. Either one of two copies should remain, or the common code should be
separated into one function.

> [drlvm][classloader] On-demand class library parsing to avoid unnecessary jar/zip parsing
during VM creation
> ------------------------------------------------------------------------------------------------------------
>                 Key: HARMONY-6039
>                 URL: https://issues.apache.org/jira/browse/HARMONY-6039
>             Project: Harmony
>          Issue Type: Improvement
>          Components: VM
>         Environment: Conduct experiments on Windows XP, Core 2 Quad-core machine
>            Reporter: Wenlong Li
>            Assignee: Xiao-Feng Li
>         Attachments: h6039.patch_1, H6039.patch_2, H6039.patch_3
> During VM creation, Harmony will parse all class libraried defined in bootclasspath.properties
under jre/lib/boot directory. However, not all class libraries will be used in the lifetime
of java applications. That means, it is not necessary to open and resolve all these class
> In this patch, I implement the on-demand jar parsing, which only loads and parses required

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message