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

9
excessive comment length

9-
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

37
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?

93
*the* bootstrap classpath

96-
the proper bracket style is specified here
https://issues.apache.org/jira/secure/attachment/12353745/format.sh
[well, the whole file is formatted strangely - Pavel, could you comment?]

97
such -> the

150
*a* temp pool

154-
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
environment.h

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

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


200
can this be replaced with assert(luni_path)?

213
+1 to Aleksey's comment on literals

[have to go, will continue later]

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

299-
do we need both vmboot and requierdLib?

434
commented code

442
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)

449
suggestion to add an asssert remains

513
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
libraries.
> In this patch, I implement the on-demand jar parsing, which only loads and parses required
jar.

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


Mime
View raw message