You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@harmony.apache.org by Wenlong Li <we...@gmail.com> on 2009/01/06 03:09:59 UTC

Re: [VM] On-demand class library parsing is ready to commit

Yes, Xiao-Feng's understanding is correct. The patch loads and parses
modules on demand. If no class in swing.jar is not requested, then
this module will not be loaded.

btw, Alexei, you said "SetClasspathFromString" and "pdest >
(*it).second->bytes" are not efficient. Can you share more comments on
them? I just reused some code in Harmony, and didn't optimize them
further.

Thx, Wenlong
Managed Runtime Technology Center, Intel

On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com> wrote:
> On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
> <al...@gmail.com> wrote:
>> Xiao Feng,
>> Thank you for explaining.
>>
>> I get more minor comments on more commented code, ineffective
>> SetClasspathFromString usage, non-covered unexpected case when pdest >
>> (*it).second->bytes. One major comment on crossing vm module boundary
>> still remains. But now I'm happy with the design.
>
> Alexei, yes, I agree with your comments. These parts should be
> improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
> speaking.)
>
> Thanks,
> xiaofeng
>
>> Sorry for being slow.
>>
>>
>>
>> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com> wrote:
>>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>> <al...@gmail.com> wrote:
>>>> Xiao-Feng,
>>>> Continuing with the server example could you please give me a hint where
>>>> decision to load swing.jar or not is taken in the patch? My initial
>>>> perception was that the list of what to load was hardcoded and was not
>>>> constructed dynamically depending on application.
>>>
>>> Alexei, here is the patch code I found:
>>>
>>> line 245:
>>> +            // Find which jar exports this package
>>> +            if (pkgName != NULL) {
>>> +                char *boot_class_path =
>>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>> +                char *pendingClassPath = NULL;
>>> +                apr_pool_t *tmp_pool;
>>> +                apr_pool_create(&tmp_pool, NULL);
>>> +                while (it != env->pending_jar_set.end()) {
>>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>> +                    if (pdest != NULL) {
>>> +                        pendingClassPath =
>>> (char*)STD_MALLOC(strlen(boot_class_path)
>>> +                                               + strlen((*it).first->bytes) + 1);
>>> +                        strcpy(pendingClassPath, boot_class_path);
>>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>> +                        // Open this found jar, and read all classes
>>> contained in this jar
>>> +                        SetClasspathFromString(pendingClassPath, tmp_pool);
>>> +                        // Erase the found jar from pending jar list
>>> as it has been parsed
>>> +                        env->pending_jar_set.erase(it++);
>>> +                        STD_FREE(pendingClassPath);
>>> +                    } else {
>>>
>>> It checks if a JAR has the requested package, then loads it if yes. I
>>> am not sure if this is what you were asking. (Btw, this is only my
>>> understanding of his patch. I am not speaking for Wenlong.)
>>>
>>> Thanks,
>>> xiaofeng
>>>
>>>> Thanks.
>>>>
>>>>
>>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com> wrote:
>>>>
>>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>> <al...@gmail.com> wrote:
>>>>> > Aleksey,
>>>>> > I like your conclusion.
>>>>> >
>>>>> > Wenlong,
>>>>> > I'm trying to understand the real life value of the "abstract" startup
>>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>> > swing.jar for a server application? Do I understand that loading
>>>>> > happens, though it happens later compared to VM without your patch? I
>>>>> > believe that the proper design of delayed loading should answer "no"
>>>>> > to this question.
>>>>>
>>>>> I checked the patch, and I found the answer is indeed "No" as you expected.
>>>>>
>>>>> Thanks,
>>>>> xiaofeng
>>>>>
>>>>> > In other words, I appreciate if you describe which real use cases are
>>>>> > improved by this patch.
>>>>> > Thanks!
>>>>>
>>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>> > <al...@gmail.com> wrote:
>>>>> >> Ok, here's the catch.
>>>>> >>
>>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which enumerates
>>>>> >> the JARs available for bootclassloader. The set of such the JARs is
>>>>> >> really stable because modular decomposition of classlib is stable.
>>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>> >> should be updated when new JAR file arrives.
>>>>> >> Modulelibrarymapping.properties is different on this point, it's the
>>>>> >> Map<PackageName,JARfile>, which should be updated each time the new
>>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>> >>
>>>>> >> Automatic generation of this property file gives two advantages:
>>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping and
>>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour in
>>>>> >> case the mapping is wrong?
>>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>> >> modularity of Harmony classlib and eventually they might want to split
>>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>> >> generation would enable them to quickly roll-in and experiment with
>>>>> >> different package layouts and their impact on performance. They could
>>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't be
>>>>> >> used by them then ;)
>>>>> >>
>>>>> >> That's merely a housekeeping procedure. I believe that anything which
>>>>> >> could be done more than once, eventually would be done more than once.
>>>>> >> Hence it should be automated. You say that the file was generated from
>>>>> >> manifests of JARs, so is it hard to just tie the same tool into DRLVM
>>>>> >> build process?
>>>>> >>
>>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>> >>  a. breaks the compatibility of classlib (you change
>>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>> >>  b. treated in DRLVM classloader only.
>>>>> >>
>>>>> >> Of course eventually this feature might be used by others, but IMO we
>>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>> >> rather wait for some incubation on DRLVM side first.
>>>>> >>
>>>>> >> Thanks,
>>>>> >> Aleksey.
>>>>> >>
>>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>> >>> granularity. Instead, it only needs know package info provided in the
>>>>> >>> manifest file.  When class is added to a library, do we need change
>>>>> >>> the manifest as well?
>>>>> >>>
>>>>> >>> btw, I guess there is a mis-understanding: my modulelibrarymapping
>>>>> >>> file only records package info provided by manfiest in each module. It
>>>>> >>> doesn't relate to each class.
>>>>> >>>
>>>>> >>> thx,
>>>>> >>> Wenlong
>>>>> >>>
>>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <pm...@gmail.com>
>>>>> wrote:
>>>>> >>>> Classes are added to class library from time to time. I'm not sure how
>>>>> >>>> it can be possible to track these changes manually.
>>>>> >>>>
>>>>> >>>> WBR,
>>>>> >>>>    Pavel.
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>> wrote:
>>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>> >>>>> specific file, why we could not make a vm specific file manually?
>>>>> Just
>>>>> >>>>> curious to know the possible reason. :)
>>>>> >>>>>
>>>>> >>>>> thx,
>>>>> >>>>> Wenlong
>>>>> >>>>>
>>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <pm...@gmail.com>
>>>>> wrote:
>>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>> file...
>>>>> >>>>>>
>>>>> >>>>>> WBR,
>>>>> >>>>>>    Pavel.
>>>>> >>>>>>
>>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <we...@gmail.com>
>>>>> wrote:
>>>>> >>>>>>> btw, because adding new module is rare case, manually modifying the
>>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>> >>>>>>>
>>>>> >>>>>>> If so, can I conclude adding another property file with same update
>>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>> >>>>>>>
>>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>> >>>>>>>
>>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <pm...@gmail.com>
>>>>> wrote:
>>>>> >>>>>>>> Wenlong,
>>>>> >>>>>>>>
>>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on adding new
>>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>> >>>>>>>>
>>>>> >>>>>>>> WBR,
>>>>> >>>>>>>>    Pavel.
>>>>> >>>>>>>>
>>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <we...@gmail.com>
>>>>> wrote:
>>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>> >>>>>>>>>
>>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>> bootclasspath.properties
>>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>> >>>>>>>>>
>>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>> specific
>>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation time. So
>>>>> that
>>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>> potential
>>>>> >>>>>>>>> modularity and compatibility problem.
>>>>> >>>>>>>>>
>>>>> >>>>>>>>> thx,
>>>>> >>>>>>>>> Wenlong
>>>>> >>>>>>>>>
>>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>> >>>>>>>>>> Hi, Wenlong.
>>>>> >>>>>>>>>>
>>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <we...@gmail.com>
>>>>> wrote:
>>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a need
>>>>> to
>>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance gain in
>>>>> Linux
>>>>> >>>>>>>>>>> when using your methodology. For windows test, I will double
>>>>> check the
>>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>> >>>>>>>>>>
>>>>> >>>>>>>>>> My opinion was already expressed after I had finished the tests
>>>>> from
>>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions, so
>>>>> whether
>>>>> >>>>>>>>>> it's worth including into Harmony really depends on how much
>>>>> mess the
>>>>> >>>>>>>>>> patch would introduce besides the "performance boost". From what
>>>>> I
>>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the correct
>>>>> mapping
>>>>> >>>>>>>>>> between jars and Java packages. This new feature is also spread
>>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific. In
>>>>> this
>>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>> >>>>>>>>>>
>>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch with
>>>>> two
>>>>> >>>>>>>>>> serious modifications:
>>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>> Classlib,
>>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise it
>>>>> might
>>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>> process?)
>>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>> >>>>>>>>>>
>>>>> >>>>>>>>>> Thanks,
>>>>> >>>>>>>>>> Aleksey.
>>>>> >>>>>>>>>>
>>>>> >>>>>>>>>
>>>>> >>>>>>>>
>>>>> >>>>>>>
>>>>> >>>>>>
>>>>> >>>>>
>>>>> >>>>
>>>>> >>>
>>>>> >>
>>>>> >
>>>>> >
>>>>> >
>>>>> > --
>>>>> > С уважением,
>>>>> > Алексей Федотов,
>>>>> > ЗАО «Телеком Экспресс»
>>>>> >
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> http://xiao-feng.blogspot.com
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> С уважением,
>>>> Алексей Федотов,
>>>> ЗАО «Телеком Экспресс»
>>>>
>>>
>>>
>>>
>>> --
>>> http://xiao-feng.blogspot.com
>>>
>>
>>
>>
>> --
>> С уважением,
>> Алексей Федотов,
>> ЗАО «Телеком Экспресс»
>>
>
>
>
> --
> http://xiao-feng.blogspot.com
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Aleksey Shipilev <al...@gmail.com>.

Hi Wenlong!

I assumed that you're talking about H6038_2.patch.

1. On the performance side: the result of performance assessment on
HelloWorld is below. The test system is:
 - Pentium D 820 2.8 Ghz / 2 Gb DDR2-667
 - WD 3200KS, 320 Gb, 16 Mb cache
 - Gentoo Linux x86, 2.6.23
 - Harmony r734360, release build

"cold-start" (invalidate caches):
clean: (2.49 +- 0.13) secs
ondemand: (2.31 +- 0.10) secs

"warm-start" (don't invalidate caches);
clean: (0.42 +- 0.01) secs
ondemand: (0.40 +- 0.01) secs

Ok, we might presume the patch finally gives a small-small boost :)

2. On the conformance side: we need to work out the patch a bit:
 a. externalize magic numbers
 b. externalize inlined string literals
("modulelibrarymapping.properties", etc.)

All other things look ok for me.

Thanks,
Aleksey.

On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
> All,
>
> I submit a new patch for on-demand class loading and parsing. All
> codes are put in VM side, and the mapping info is automatically
> produced.
>
> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>
> Comments are welcome.
>
> Thx, Wenlong
> Managed Runtime Technology Center, Intel
>
> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>> All,
>> At this moment, I move all updates in classlib to VM side such that
>> there is no modularity issue. Next step is to produce the mapping
>> between module and library efficiently and accurately.
>>
>> Comments are welcome.
>>
>> Thx, Wenlong
>> Managed Runtime Technology Center, Intel
>>
>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>> Thx :)
>>>
>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>> <al...@gmail.com> wrote:
>>>> Sure.
>>>>
>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>> splitting the given classpath into pieces. You already know the new piece
>>>> you add and may skip splitting step.
>>>>
>>>> 2. If I understand you code correctly, the case "pdest >
>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>> assrtion would speed up bug discovery.
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>
>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>> this module will not be loaded.
>>>>>
>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>> further.
>>>>>
>>>>> Thx, Wenlong
>>>>> Managed Runtime Technology Center, Intel
>>>>>
>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>> > <al...@gmail.com> wrote:
>>>>> >> Xiao Feng,
>>>>> >> Thank you for explaining.
>>>>> >>
>>>>> >> I get more minor comments on more commented code, ineffective
>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>> >> still remains. But now I'm happy with the design.
>>>>> >
>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>> > speaking.)
>>>>> >
>>>>> > Thanks,
>>>>> > xiaofeng
>>>>> >
>>>>> >> Sorry for being slow.
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>> >>> <al...@gmail.com> wrote:
>>>>> >>>> Xiao-Feng,
>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>> where
>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>> >>>> constructed dynamically depending on application.
>>>>> >>>
>>>>> >>> Alexei, here is the patch code I found:
>>>>> >>>
>>>>> >>> line 245:
>>>>> >>> +            // Find which jar exports this package
>>>>> >>> +            if (pkgName != NULL) {
>>>>> >>> +                char *boot_class_path =
>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>> >>> +                char *pendingClassPath = NULL;
>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>> >>> +                    if (pdest != NULL) {
>>>>> >>> +                        pendingClassPath =
>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>> >>> +                                               +
>>>>> strlen((*it).first->bytes) + 1);
>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>> >>> +                        // Open this found jar, and read all classes
>>>>> >>> contained in this jar
>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>> tmp_pool);
>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>> >>> as it has been parsed
>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>> >>> +                    } else {
>>>>> >>>
>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>> >>>
>>>>> >>> Thanks,
>>>>> >>> xiaofeng
>>>>> >>>
>>>>> >>>> Thanks.
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> >>>>
>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>> >>>>> <al...@gmail.com> wrote:
>>>>> >>>>> > Aleksey,
>>>>> >>>>> > I like your conclusion.
>>>>> >>>>> >
>>>>> >>>>> > Wenlong,
>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>> startup
>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>> I
>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>> "no"
>>>>> >>>>> > to this question.
>>>>> >>>>>
>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>> expected.
>>>>> >>>>>
>>>>> >>>>> Thanks,
>>>>> >>>>> xiaofeng
>>>>> >>>>>
>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>> are
>>>>> >>>>> > improved by this patch.
>>>>> >>>>> > Thanks!
>>>>> >>>>>
>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>> >>>>> >> Ok, here's the catch.
>>>>> >>>>> >>
>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>> enumerates
>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>> is
>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>> the
>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>> new
>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>> >>>>> >>
>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>> and
>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>> in
>>>>> >>>>> >> case the mapping is wrong?
>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>> split
>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>> with
>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>> could
>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>> be
>>>>> >>>>> >> used by them then ;)
>>>>> >>>>> >>
>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>> which
>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>> once.
>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>> from
>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>> DRLVM
>>>>> >>>>> >> build process?
>>>>> >>>>> >>
>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>> >>>>> >>
>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>> we
>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>> >>>>> >>
>>>>> >>>>> >> Thanks,
>>>>> >>>>> >> Aleksey.
>>>>> >>>>> >>
>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>> wrote:
>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>> the
>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>> change
>>>>> >>>>> >>> the manifest as well?
>>>>> >>>>> >>>
>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>> modulelibrarymapping
>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>> module. It
>>>>> >>>>> >>> doesn't relate to each class.
>>>>> >>>>> >>>
>>>>> >>>>> >>> thx,
>>>>> >>>>> >>> Wenlong
>>>>> >>>>> >>>
>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>> sure how
>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>> >>>>> >>>>
>>>>> >>>>> >>>> WBR,
>>>>> >>>>> >>>>    Pavel.
>>>>> >>>>> >>>>
>>>>> >>>>> >>>>
>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>> manually?
>>>>> >>>>> Just
>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>> thx,
>>>>> >>>>> >>>>> Wenlong
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>> >>>>> file...
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>> WBR,
>>>>> >>>>> >>>>>>    Pavel.
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>> modifying the
>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>> update
>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>> Wenlong,
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>> adding new
>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> WBR,
>>>>> >>>>> >>>>>>>>    Pavel.
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>> >>>>> bootclasspath.properties
>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>> >>>>> specific
>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>> time. So
>>>>> >>>>> that
>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>> >>>>> potential
>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> thx,
>>>>> >>>>> >>>>>>>>> Wenlong
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>> need
>>>>> >>>>> to
>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>> gain in
>>>>> >>>>> Linux
>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>> double
>>>>> >>>>> check the
>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>> tests
>>>>> >>>>> from
>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>> so
>>>>> >>>>> whether
>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>> much
>>>>> >>>>> mess the
>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>> From what
>>>>> >>>>> I
>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>> correct
>>>>> >>>>> mapping
>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>> spread
>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>> In
>>>>> >>>>> this
>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>> with
>>>>> >>>>> two
>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>> >>>>> Classlib,
>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>> it
>>>>> >>>>> might
>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>> >>>>> process?)
>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>
>>>>> >>>>> >>>
>>>>> >>>>> >>
>>>>> >>>>> >
>>>>> >>>>> >
>>>>> >>>>> >
>>>>> >>>>> > --
>>>>> >>>>> > С уважением,
>>>>> >>>>> > Алексей Федотов,
>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>> >>>>> >
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>> --
>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>> >>>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> --
>>>>> >>>> С уважением,
>>>>> >>>> Алексей Федотов,
>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>> >>>>
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>> --
>>>>> >>> http://xiao-feng.blogspot.com
>>>>> >>>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> --
>>>>> >> С уважением,
>>>>> >> Алексей Федотов,
>>>>> >> ЗАО «Телеком Экспресс»
>>>>> >>
>>>>> >
>>>>> >
>>>>> >
>>>>> > --
>>>>> > http://xiao-feng.blogspot.com
>>>>> >
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> С уважением,
>>>> Алексей Федотов,
>>>> ЗАО «Телеком Экспресс»
>>>>
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Alexei Fedotov <al...@gmail.com>.

Wenlong,
It would be very nice of you to explicitely outline in the last JIRA
comment and/or the cover letter  which patches are intended to
reviewed (and committed). The only patch which is intended for commit
is H6039.patch_2, isn't it?

[1] http://issues.apache.org/jira/secure/attachment/12397873/H6039.patch_2

On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
> All,
>
> I submit a new patch for on-demand class loading and parsing. All
> codes are put in VM side, and the mapping info is automatically
> produced.
>
> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>
> Comments are welcome.
>
> Thx, Wenlong
> Managed Runtime Technology Center, Intel
>
> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>> All,
>> At this moment, I move all updates in classlib to VM side such that
>> there is no modularity issue. Next step is to produce the mapping
>> between module and library efficiently and accurately.
>>
>> Comments are welcome.
>>
>> Thx, Wenlong
>> Managed Runtime Technology Center, Intel
>>
>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>> Thx :)
>>>
>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>> <al...@gmail.com> wrote:
>>>> Sure.
>>>>
>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>> splitting the given classpath into pieces. You already know the new piece
>>>> you add and may skip splitting step.
>>>>
>>>> 2. If I understand you code correctly, the case "pdest >
>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>> assrtion would speed up bug discovery.
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>
>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>> this module will not be loaded.
>>>>>
>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>> further.
>>>>>
>>>>> Thx, Wenlong
>>>>> Managed Runtime Technology Center, Intel
>>>>>
>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>> > <al...@gmail.com> wrote:
>>>>> >> Xiao Feng,
>>>>> >> Thank you for explaining.
>>>>> >>
>>>>> >> I get more minor comments on more commented code, ineffective
>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>> >> still remains. But now I'm happy with the design.
>>>>> >
>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>> > speaking.)
>>>>> >
>>>>> > Thanks,
>>>>> > xiaofeng
>>>>> >
>>>>> >> Sorry for being slow.
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>> >>> <al...@gmail.com> wrote:
>>>>> >>>> Xiao-Feng,
>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>> where
>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>> >>>> constructed dynamically depending on application.
>>>>> >>>
>>>>> >>> Alexei, here is the patch code I found:
>>>>> >>>
>>>>> >>> line 245:
>>>>> >>> +            // Find which jar exports this package
>>>>> >>> +            if (pkgName != NULL) {
>>>>> >>> +                char *boot_class_path =
>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>> >>> +                char *pendingClassPath = NULL;
>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>> >>> +                    if (pdest != NULL) {
>>>>> >>> +                        pendingClassPath =
>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>> >>> +                                               +
>>>>> strlen((*it).first->bytes) + 1);
>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>> >>> +                        // Open this found jar, and read all classes
>>>>> >>> contained in this jar
>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>> tmp_pool);
>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>> >>> as it has been parsed
>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>> >>> +                    } else {
>>>>> >>>
>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>> >>>
>>>>> >>> Thanks,
>>>>> >>> xiaofeng
>>>>> >>>
>>>>> >>>> Thanks.
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> >>>>
>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>> >>>>> <al...@gmail.com> wrote:
>>>>> >>>>> > Aleksey,
>>>>> >>>>> > I like your conclusion.
>>>>> >>>>> >
>>>>> >>>>> > Wenlong,
>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>> startup
>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>> I
>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>> "no"
>>>>> >>>>> > to this question.
>>>>> >>>>>
>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>> expected.
>>>>> >>>>>
>>>>> >>>>> Thanks,
>>>>> >>>>> xiaofeng
>>>>> >>>>>
>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>> are
>>>>> >>>>> > improved by this patch.
>>>>> >>>>> > Thanks!
>>>>> >>>>>
>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>> >>>>> >> Ok, here's the catch.
>>>>> >>>>> >>
>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>> enumerates
>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>> is
>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>> the
>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>> new
>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>> >>>>> >>
>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>> and
>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>> in
>>>>> >>>>> >> case the mapping is wrong?
>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>> split
>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>> with
>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>> could
>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>> be
>>>>> >>>>> >> used by them then ;)
>>>>> >>>>> >>
>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>> which
>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>> once.
>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>> from
>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>> DRLVM
>>>>> >>>>> >> build process?
>>>>> >>>>> >>
>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>> >>>>> >>
>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>> we
>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>> >>>>> >>
>>>>> >>>>> >> Thanks,
>>>>> >>>>> >> Aleksey.
>>>>> >>>>> >>
>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>> wrote:
>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>> the
>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>> change
>>>>> >>>>> >>> the manifest as well?
>>>>> >>>>> >>>
>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>> modulelibrarymapping
>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>> module. It
>>>>> >>>>> >>> doesn't relate to each class.
>>>>> >>>>> >>>
>>>>> >>>>> >>> thx,
>>>>> >>>>> >>> Wenlong
>>>>> >>>>> >>>
>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>> sure how
>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>> >>>>> >>>>
>>>>> >>>>> >>>> WBR,
>>>>> >>>>> >>>>    Pavel.
>>>>> >>>>> >>>>
>>>>> >>>>> >>>>
>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>> manually?
>>>>> >>>>> Just
>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>> thx,
>>>>> >>>>> >>>>> Wenlong
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>> >>>>> file...
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>> WBR,
>>>>> >>>>> >>>>>>    Pavel.
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>> modifying the
>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>> update
>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>> Wenlong,
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>> adding new
>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> WBR,
>>>>> >>>>> >>>>>>>>    Pavel.
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>> >>>>> bootclasspath.properties
>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>> >>>>> specific
>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>> time. So
>>>>> >>>>> that
>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>> >>>>> potential
>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> thx,
>>>>> >>>>> >>>>>>>>> Wenlong
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>> need
>>>>> >>>>> to
>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>> gain in
>>>>> >>>>> Linux
>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>> double
>>>>> >>>>> check the
>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>> tests
>>>>> >>>>> from
>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>> so
>>>>> >>>>> whether
>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>> much
>>>>> >>>>> mess the
>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>> From what
>>>>> >>>>> I
>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>> correct
>>>>> >>>>> mapping
>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>> spread
>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>> In
>>>>> >>>>> this
>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>> with
>>>>> >>>>> two
>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>> >>>>> Classlib,
>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>> it
>>>>> >>>>> might
>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>> >>>>> process?)
>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>
>>>>> >>>>> >>>
>>>>> >>>>> >>
>>>>> >>>>> >
>>>>> >>>>> >
>>>>> >>>>> >
>>>>> >>>>> > --
>>>>> >>>>> > С уважением,
>>>>> >>>>> > Алексей Федотов,
>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>> >>>>> >
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>> --
>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>> >>>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> --
>>>>> >>>> С уважением,
>>>>> >>>> Алексей Федотов,
>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>> >>>>
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>> --
>>>>> >>> http://xiao-feng.blogspot.com
>>>>> >>>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> --
>>>>> >> С уважением,
>>>>> >> Алексей Федотов,
>>>>> >> ЗАО «Телеком Экспресс»
>>>>> >>
>>>>> >
>>>>> >
>>>>> >
>>>>> > --
>>>>> > http://xiao-feng.blogspot.com
>>>>> >
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> С уважением,
>>>> Алексей Федотов,
>>>> ЗАО «Телеком Экспресс»
>>>>
>>>
>>
>



-- 
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»

Re: [VM] On-demand class library parsing is ready to commit

Posted by Wenlong Li <we...@gmail.com>.

I once used VTune, but it doesn't help too much for this case in
identifying the bottleneck. later I manually insert instrumentation
code to collect execution time, and found class loading in VM_init1
and VM_init2 is time consuming, where jar loading & parsing one of the
hot spots. Just FYI

On Wed, Jan 14, 2009 at 10:25 PM, Pavel Pervov <pm...@gmail.com> wrote:
> BTW, had anyone a chance to profile JVM_CreateJavaVM with a tool like
> VTune or gprof? It'd be interesting to see where most of time is being
> spent during VM creation - it may be something completely different
> than jar files processing.
>
> Pavel.
>
> On Wed, Jan 14, 2009 at 4:41 PM, Wenlong Li <we...@gmail.com> wrote:
>> Btw, Aleksey, to avoid confusion, I expect this patch help VM creation
>> time or at least help the memory footprint since some modules are
>> never used after their loading and parsing. :)
>>
>> On Wed, Jan 14, 2009 at 9:38 PM, Wenlong Li <we...@gmail.com> wrote:
>>> Alexei,
>>>
>>> Sorry for confusing. The patch for review is H6039.patch_2. Please
>>> kindly provide your comment.
>>>
>>> Aleksey,
>>>
>>> I have not measured the performance before completing the code review.
>>> I will do that later.
>>>
>>> thx, Wenlong
>>>
>>> On Wed, Jan 14, 2009 at 9:14 PM, Wenlong Li <we...@gmail.com> wrote:
>>>> Pavel,
>>>>
>>>> Pls see my comments in the JIRA.
>>>>
>>>> thx, Wenlong
>>>>
>>>> On Wed, Jan 14, 2009 at 8:44 PM, Pavel Pervov <pm...@gmail.com> wrote:
>>>>> Please, also, check that jar entry caches still work correctly after your patch.
>>>>>
>>>>> Pavel.
>>>>>
>>>>> On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>>> All,
>>>>>>
>>>>>> I submit a new patch for on-demand class loading and parsing. All
>>>>>> codes are put in VM side, and the mapping info is automatically
>>>>>> produced.
>>>>>>
>>>>>> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>>>>>>
>>>>>> Comments are welcome.
>>>>>>
>>>>>> Thx, Wenlong
>>>>>> Managed Runtime Technology Center, Intel
>>>>>>
>>>>>> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>>>> All,
>>>>>>> At this moment, I move all updates in classlib to VM side such that
>>>>>>> there is no modularity issue. Next step is to produce the mapping
>>>>>>> between module and library efficiently and accurately.
>>>>>>>
>>>>>>> Comments are welcome.
>>>>>>>
>>>>>>> Thx, Wenlong
>>>>>>> Managed Runtime Technology Center, Intel
>>>>>>>
>>>>>>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>>>>> Thx :)
>>>>>>>>
>>>>>>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>>>>>>> <al...@gmail.com> wrote:
>>>>>>>>> Sure.
>>>>>>>>>
>>>>>>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>>>>>>> splitting the given classpath into pieces. You already know the new piece
>>>>>>>>> you add and may skip splitting step.
>>>>>>>>>
>>>>>>>>> 2. If I understand you code correctly, the case "pdest >
>>>>>>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>>>>>>> assrtion would speed up bug discovery.
>>>>>>>>>
>>>>>>>>> Thanks.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>>>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>>>>>>> this module will not be loaded.
>>>>>>>>>>
>>>>>>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>>>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>>>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>>>>>>> further.
>>>>>>>>>>
>>>>>>>>>> Thx, Wenlong
>>>>>>>>>> Managed Runtime Technology Center, Intel
>>>>>>>>>>
>>>>>>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>>>>>>> > <al...@gmail.com> wrote:
>>>>>>>>>> >> Xiao Feng,
>>>>>>>>>> >> Thank you for explaining.
>>>>>>>>>> >>
>>>>>>>>>> >> I get more minor comments on more commented code, ineffective
>>>>>>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>>>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>>>>>>> >> still remains. But now I'm happy with the design.
>>>>>>>>>> >
>>>>>>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>>>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>>>>>>> > speaking.)
>>>>>>>>>> >
>>>>>>>>>> > Thanks,
>>>>>>>>>> > xiaofeng
>>>>>>>>>> >
>>>>>>>>>> >> Sorry for being slow.
>>>>>>>>>> >>
>>>>>>>>>> >>
>>>>>>>>>> >>
>>>>>>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>>>>>>> >>> <al...@gmail.com> wrote:
>>>>>>>>>> >>>> Xiao-Feng,
>>>>>>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>>>>>>> where
>>>>>>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>>>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>>>>>>> >>>> constructed dynamically depending on application.
>>>>>>>>>> >>>
>>>>>>>>>> >>> Alexei, here is the patch code I found:
>>>>>>>>>> >>>
>>>>>>>>>> >>> line 245:
>>>>>>>>>> >>> +            // Find which jar exports this package
>>>>>>>>>> >>> +            if (pkgName != NULL) {
>>>>>>>>>> >>> +                char *boot_class_path =
>>>>>>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>>>>>>> >>> +                char *pendingClassPath = NULL;
>>>>>>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>>>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>>>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>>>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>>>>>>> >>> +                    if (pdest != NULL) {
>>>>>>>>>> >>> +                        pendingClassPath =
>>>>>>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>>>>>>> >>> +                                               +
>>>>>>>>>> strlen((*it).first->bytes) + 1);
>>>>>>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>>>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>>>>>>> >>> +                        // Open this found jar, and read all classes
>>>>>>>>>> >>> contained in this jar
>>>>>>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>>>>>>> tmp_pool);
>>>>>>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>>>>>>> >>> as it has been parsed
>>>>>>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>>>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>>>>>>> >>> +                    } else {
>>>>>>>>>> >>>
>>>>>>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>>>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>>>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>>>>>>> >>>
>>>>>>>>>> >>> Thanks,
>>>>>>>>>> >>> xiaofeng
>>>>>>>>>> >>>
>>>>>>>>>> >>>> Thanks.
>>>>>>>>>> >>>>
>>>>>>>>>> >>>>
>>>>>>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>> >>>>
>>>>>>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>>>>>>> >>>>> <al...@gmail.com> wrote:
>>>>>>>>>> >>>>> > Aleksey,
>>>>>>>>>> >>>>> > I like your conclusion.
>>>>>>>>>> >>>>> >
>>>>>>>>>> >>>>> > Wenlong,
>>>>>>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>>>>>>> startup
>>>>>>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>>>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>>>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>>>>>>> I
>>>>>>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>>>>>>> "no"
>>>>>>>>>> >>>>> > to this question.
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>>>>>>> expected.
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>> Thanks,
>>>>>>>>>> >>>>> xiaofeng
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>>>>>>> are
>>>>>>>>>> >>>>> > improved by this patch.
>>>>>>>>>> >>>>> > Thanks!
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>>>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>>>>>>> >>>>> >> Ok, here's the catch.
>>>>>>>>>> >>>>> >>
>>>>>>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>>>>>>> enumerates
>>>>>>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>>>>>>> is
>>>>>>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>>>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>>>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>>>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>>>>>>> the
>>>>>>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>>>>>>> new
>>>>>>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>>>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>>>>>>> >>>>> >>
>>>>>>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>>>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>>>>>>> and
>>>>>>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>>>>>>> in
>>>>>>>>>> >>>>> >> case the mapping is wrong?
>>>>>>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>>>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>>>>>>> split
>>>>>>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>>>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>>>>>>> with
>>>>>>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>>>>>>> could
>>>>>>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>>>>>>> be
>>>>>>>>>> >>>>> >> used by them then ;)
>>>>>>>>>> >>>>> >>
>>>>>>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>>>>>>> which
>>>>>>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>>>>>>> once.
>>>>>>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>>>>>>> from
>>>>>>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>>>>>>> DRLVM
>>>>>>>>>> >>>>> >> build process?
>>>>>>>>>> >>>>> >>
>>>>>>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>>>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>>>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>>>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>>>>>>> >>>>> >>
>>>>>>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>>>>>>> we
>>>>>>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>>>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>>>>>>> >>>>> >>
>>>>>>>>>> >>>>> >> Thanks,
>>>>>>>>>> >>>>> >> Aleksey.
>>>>>>>>>> >>>>> >>
>>>>>>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>>>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>>>>>>> the
>>>>>>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>>>>>>> change
>>>>>>>>>> >>>>> >>> the manifest as well?
>>>>>>>>>> >>>>> >>>
>>>>>>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>>>>>>> modulelibrarymapping
>>>>>>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>>>>>>> module. It
>>>>>>>>>> >>>>> >>> doesn't relate to each class.
>>>>>>>>>> >>>>> >>>
>>>>>>>>>> >>>>> >>> thx,
>>>>>>>>>> >>>>> >>> Wenlong
>>>>>>>>>> >>>>> >>>
>>>>>>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>>>>>>> pmcfirst@gmail.com>
>>>>>>>>>> >>>>> wrote:
>>>>>>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>>>>>>> sure how
>>>>>>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>>>>>>> >>>>> >>>>
>>>>>>>>>> >>>>> >>>> WBR,
>>>>>>>>>> >>>>> >>>>    Pavel.
>>>>>>>>>> >>>>> >>>>
>>>>>>>>>> >>>>> >>>>
>>>>>>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>>>>>>> >>>>> wrote:
>>>>>>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>>>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>>>>>>> manually?
>>>>>>>>>> >>>>> Just
>>>>>>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>>>>>>> >>>>> >>>>>
>>>>>>>>>> >>>>> >>>>> thx,
>>>>>>>>>> >>>>> >>>>> Wenlong
>>>>>>>>>> >>>>> >>>>>
>>>>>>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>>>>>>> pmcfirst@gmail.com>
>>>>>>>>>> >>>>> wrote:
>>>>>>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>>>>>>> >>>>> file...
>>>>>>>>>> >>>>> >>>>>>
>>>>>>>>>> >>>>> >>>>>> WBR,
>>>>>>>>>> >>>>> >>>>>>    Pavel.
>>>>>>>>>> >>>>> >>>>>>
>>>>>>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>>>>>>> wenlong@gmail.com>
>>>>>>>>>> >>>>> wrote:
>>>>>>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>>>>>>> modifying the
>>>>>>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>>>>>>> >>>>> >>>>>>>
>>>>>>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>>>>>>> update
>>>>>>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>>>>>>> >>>>> >>>>>>>
>>>>>>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>>>>>>> >>>>> >>>>>>>
>>>>>>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>>>>>>> pmcfirst@gmail.com>
>>>>>>>>>> >>>>> wrote:
>>>>>>>>>> >>>>> >>>>>>>> Wenlong,
>>>>>>>>>> >>>>> >>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>>>>>>> adding new
>>>>>>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>>>>>>> >>>>> >>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>> WBR,
>>>>>>>>>> >>>>> >>>>>>>>    Pavel.
>>>>>>>>>> >>>>> >>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>>>>>>> wenlong@gmail.com>
>>>>>>>>>> >>>>> wrote:
>>>>>>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>>>>>>> >>>>> bootclasspath.properties
>>>>>>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>>>>>>> >>>>> specific
>>>>>>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>>>>>>> time. So
>>>>>>>>>> >>>>> that
>>>>>>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>>>>>>> >>>>> potential
>>>>>>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>> thx,
>>>>>>>>>> >>>>> >>>>>>>>> Wenlong
>>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>>>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>>>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>>>>>>> wenlong@gmail.com>
>>>>>>>>>> >>>>> wrote:
>>>>>>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>>>>>>> need
>>>>>>>>>> >>>>> to
>>>>>>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>>>>>>> gain in
>>>>>>>>>> >>>>> Linux
>>>>>>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>>>>>>> double
>>>>>>>>>> >>>>> check the
>>>>>>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>>>>>>> tests
>>>>>>>>>> >>>>> from
>>>>>>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>>>>>>> so
>>>>>>>>>> >>>>> whether
>>>>>>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>>>>>>> much
>>>>>>>>>> >>>>> mess the
>>>>>>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>>>>>>> From what
>>>>>>>>>> >>>>> I
>>>>>>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>>>>>>> correct
>>>>>>>>>> >>>>> mapping
>>>>>>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>>>>>>> spread
>>>>>>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>>>>>>> In
>>>>>>>>>> >>>>> this
>>>>>>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>>>>>>> with
>>>>>>>>>> >>>>> two
>>>>>>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>>>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>>>>>>> >>>>> Classlib,
>>>>>>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>>>>>>> it
>>>>>>>>>> >>>>> might
>>>>>>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>>>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>>>>>>> >>>>> process?)
>>>>>>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>>>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>>
>>>>>>>>>> >>>>> >>>>>>>
>>>>>>>>>> >>>>> >>>>>>
>>>>>>>>>> >>>>> >>>>>
>>>>>>>>>> >>>>> >>>>
>>>>>>>>>> >>>>> >>>
>>>>>>>>>> >>>>> >>
>>>>>>>>>> >>>>> >
>>>>>>>>>> >>>>> >
>>>>>>>>>> >>>>> >
>>>>>>>>>> >>>>> > --
>>>>>>>>>> >>>>> > С уважением,
>>>>>>>>>> >>>>> > Алексей Федотов,
>>>>>>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>>>>>>> >>>>> >
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>> --
>>>>>>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>
>>>>>>>>>> >>>>
>>>>>>>>>> >>>>
>>>>>>>>>> >>>> --
>>>>>>>>>> >>>> С уважением,
>>>>>>>>>> >>>> Алексей Федотов,
>>>>>>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>>>>>>> >>>>
>>>>>>>>>> >>>
>>>>>>>>>> >>>
>>>>>>>>>> >>>
>>>>>>>>>> >>> --
>>>>>>>>>> >>> http://xiao-feng.blogspot.com
>>>>>>>>>> >>>
>>>>>>>>>> >>
>>>>>>>>>> >>
>>>>>>>>>> >>
>>>>>>>>>> >> --
>>>>>>>>>> >> С уважением,
>>>>>>>>>> >> Алексей Федотов,
>>>>>>>>>> >> ЗАО «Телеком Экспресс»
>>>>>>>>>> >>
>>>>>>>>>> >
>>>>>>>>>> >
>>>>>>>>>> >
>>>>>>>>>> > --
>>>>>>>>>> > http://xiao-feng.blogspot.com
>>>>>>>>>> >
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> С уважением,
>>>>>>>>> Алексей Федотов,
>>>>>>>>> ЗАО «Телеком Экспресс»
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Pavel Pervov <pm...@gmail.com>.

BTW, had anyone a chance to profile JVM_CreateJavaVM with a tool like
VTune or gprof? It'd be interesting to see where most of time is being
spent during VM creation - it may be something completely different
than jar files processing.

Pavel.

On Wed, Jan 14, 2009 at 4:41 PM, Wenlong Li <we...@gmail.com> wrote:
> Btw, Aleksey, to avoid confusion, I expect this patch help VM creation
> time or at least help the memory footprint since some modules are
> never used after their loading and parsing. :)
>
> On Wed, Jan 14, 2009 at 9:38 PM, Wenlong Li <we...@gmail.com> wrote:
>> Alexei,
>>
>> Sorry for confusing. The patch for review is H6039.patch_2. Please
>> kindly provide your comment.
>>
>> Aleksey,
>>
>> I have not measured the performance before completing the code review.
>> I will do that later.
>>
>> thx, Wenlong
>>
>> On Wed, Jan 14, 2009 at 9:14 PM, Wenlong Li <we...@gmail.com> wrote:
>>> Pavel,
>>>
>>> Pls see my comments in the JIRA.
>>>
>>> thx, Wenlong
>>>
>>> On Wed, Jan 14, 2009 at 8:44 PM, Pavel Pervov <pm...@gmail.com> wrote:
>>>> Please, also, check that jar entry caches still work correctly after your patch.
>>>>
>>>> Pavel.
>>>>
>>>> On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>> All,
>>>>>
>>>>> I submit a new patch for on-demand class loading and parsing. All
>>>>> codes are put in VM side, and the mapping info is automatically
>>>>> produced.
>>>>>
>>>>> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>>>>>
>>>>> Comments are welcome.
>>>>>
>>>>> Thx, Wenlong
>>>>> Managed Runtime Technology Center, Intel
>>>>>
>>>>> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>>> All,
>>>>>> At this moment, I move all updates in classlib to VM side such that
>>>>>> there is no modularity issue. Next step is to produce the mapping
>>>>>> between module and library efficiently and accurately.
>>>>>>
>>>>>> Comments are welcome.
>>>>>>
>>>>>> Thx, Wenlong
>>>>>> Managed Runtime Technology Center, Intel
>>>>>>
>>>>>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>>>> Thx :)
>>>>>>>
>>>>>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>>>>>> <al...@gmail.com> wrote:
>>>>>>>> Sure.
>>>>>>>>
>>>>>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>>>>>> splitting the given classpath into pieces. You already know the new piece
>>>>>>>> you add and may skip splitting step.
>>>>>>>>
>>>>>>>> 2. If I understand you code correctly, the case "pdest >
>>>>>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>>>>>> assrtion would speed up bug discovery.
>>>>>>>>
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>>>>>
>>>>>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>>>>>> this module will not be loaded.
>>>>>>>>>
>>>>>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>>>>>> further.
>>>>>>>>>
>>>>>>>>> Thx, Wenlong
>>>>>>>>> Managed Runtime Technology Center, Intel
>>>>>>>>>
>>>>>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>>>>>> > <al...@gmail.com> wrote:
>>>>>>>>> >> Xiao Feng,
>>>>>>>>> >> Thank you for explaining.
>>>>>>>>> >>
>>>>>>>>> >> I get more minor comments on more commented code, ineffective
>>>>>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>>>>>> >> still remains. But now I'm happy with the design.
>>>>>>>>> >
>>>>>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>>>>>> > speaking.)
>>>>>>>>> >
>>>>>>>>> > Thanks,
>>>>>>>>> > xiaofeng
>>>>>>>>> >
>>>>>>>>> >> Sorry for being slow.
>>>>>>>>> >>
>>>>>>>>> >>
>>>>>>>>> >>
>>>>>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>>>>>> >>> <al...@gmail.com> wrote:
>>>>>>>>> >>>> Xiao-Feng,
>>>>>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>>>>>> where
>>>>>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>>>>>> >>>> constructed dynamically depending on application.
>>>>>>>>> >>>
>>>>>>>>> >>> Alexei, here is the patch code I found:
>>>>>>>>> >>>
>>>>>>>>> >>> line 245:
>>>>>>>>> >>> +            // Find which jar exports this package
>>>>>>>>> >>> +            if (pkgName != NULL) {
>>>>>>>>> >>> +                char *boot_class_path =
>>>>>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>>>>>> >>> +                char *pendingClassPath = NULL;
>>>>>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>>>>>> >>> +                    if (pdest != NULL) {
>>>>>>>>> >>> +                        pendingClassPath =
>>>>>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>>>>>> >>> +                                               +
>>>>>>>>> strlen((*it).first->bytes) + 1);
>>>>>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>>>>>> >>> +                        // Open this found jar, and read all classes
>>>>>>>>> >>> contained in this jar
>>>>>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>>>>>> tmp_pool);
>>>>>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>>>>>> >>> as it has been parsed
>>>>>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>>>>>> >>> +                    } else {
>>>>>>>>> >>>
>>>>>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>>>>>> >>>
>>>>>>>>> >>> Thanks,
>>>>>>>>> >>> xiaofeng
>>>>>>>>> >>>
>>>>>>>>> >>>> Thanks.
>>>>>>>>> >>>>
>>>>>>>>> >>>>
>>>>>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> >>>>
>>>>>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>>>>>> >>>>> <al...@gmail.com> wrote:
>>>>>>>>> >>>>> > Aleksey,
>>>>>>>>> >>>>> > I like your conclusion.
>>>>>>>>> >>>>> >
>>>>>>>>> >>>>> > Wenlong,
>>>>>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>>>>>> startup
>>>>>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>>>>>> I
>>>>>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>>>>>> "no"
>>>>>>>>> >>>>> > to this question.
>>>>>>>>> >>>>>
>>>>>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>>>>>> expected.
>>>>>>>>> >>>>>
>>>>>>>>> >>>>> Thanks,
>>>>>>>>> >>>>> xiaofeng
>>>>>>>>> >>>>>
>>>>>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>>>>>> are
>>>>>>>>> >>>>> > improved by this patch.
>>>>>>>>> >>>>> > Thanks!
>>>>>>>>> >>>>>
>>>>>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>>>>>> >>>>> >> Ok, here's the catch.
>>>>>>>>> >>>>> >>
>>>>>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>>>>>> enumerates
>>>>>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>>>>>> is
>>>>>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>>>>>> the
>>>>>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>>>>>> new
>>>>>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>>>>>> >>>>> >>
>>>>>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>>>>>> and
>>>>>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>>>>>> in
>>>>>>>>> >>>>> >> case the mapping is wrong?
>>>>>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>>>>>> split
>>>>>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>>>>>> with
>>>>>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>>>>>> could
>>>>>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>>>>>> be
>>>>>>>>> >>>>> >> used by them then ;)
>>>>>>>>> >>>>> >>
>>>>>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>>>>>> which
>>>>>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>>>>>> once.
>>>>>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>>>>>> from
>>>>>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>>>>>> DRLVM
>>>>>>>>> >>>>> >> build process?
>>>>>>>>> >>>>> >>
>>>>>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>>>>>> >>>>> >>
>>>>>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>>>>>> we
>>>>>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>>>>>> >>>>> >>
>>>>>>>>> >>>>> >> Thanks,
>>>>>>>>> >>>>> >> Aleksey.
>>>>>>>>> >>>>> >>
>>>>>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>>>>>> the
>>>>>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>>>>>> change
>>>>>>>>> >>>>> >>> the manifest as well?
>>>>>>>>> >>>>> >>>
>>>>>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>>>>>> modulelibrarymapping
>>>>>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>>>>>> module. It
>>>>>>>>> >>>>> >>> doesn't relate to each class.
>>>>>>>>> >>>>> >>>
>>>>>>>>> >>>>> >>> thx,
>>>>>>>>> >>>>> >>> Wenlong
>>>>>>>>> >>>>> >>>
>>>>>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>>>>>> pmcfirst@gmail.com>
>>>>>>>>> >>>>> wrote:
>>>>>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>>>>>> sure how
>>>>>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>>>>>> >>>>> >>>>
>>>>>>>>> >>>>> >>>> WBR,
>>>>>>>>> >>>>> >>>>    Pavel.
>>>>>>>>> >>>>> >>>>
>>>>>>>>> >>>>> >>>>
>>>>>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>>>>>> >>>>> wrote:
>>>>>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>>>>>> manually?
>>>>>>>>> >>>>> Just
>>>>>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>>>>>> >>>>> >>>>>
>>>>>>>>> >>>>> >>>>> thx,
>>>>>>>>> >>>>> >>>>> Wenlong
>>>>>>>>> >>>>> >>>>>
>>>>>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>>>>>> pmcfirst@gmail.com>
>>>>>>>>> >>>>> wrote:
>>>>>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>>>>>> >>>>> file...
>>>>>>>>> >>>>> >>>>>>
>>>>>>>>> >>>>> >>>>>> WBR,
>>>>>>>>> >>>>> >>>>>>    Pavel.
>>>>>>>>> >>>>> >>>>>>
>>>>>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>>>>>> wenlong@gmail.com>
>>>>>>>>> >>>>> wrote:
>>>>>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>>>>>> modifying the
>>>>>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>>>>>> >>>>> >>>>>>>
>>>>>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>>>>>> update
>>>>>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>>>>>> >>>>> >>>>>>>
>>>>>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>>>>>> >>>>> >>>>>>>
>>>>>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>>>>>> pmcfirst@gmail.com>
>>>>>>>>> >>>>> wrote:
>>>>>>>>> >>>>> >>>>>>>> Wenlong,
>>>>>>>>> >>>>> >>>>>>>>
>>>>>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>>>>>> adding new
>>>>>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>>>>>> >>>>> >>>>>>>>
>>>>>>>>> >>>>> >>>>>>>> WBR,
>>>>>>>>> >>>>> >>>>>>>>    Pavel.
>>>>>>>>> >>>>> >>>>>>>>
>>>>>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>>>>>> wenlong@gmail.com>
>>>>>>>>> >>>>> wrote:
>>>>>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>>>>>> >>>>> bootclasspath.properties
>>>>>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>>>>>> >>>>> specific
>>>>>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>>>>>> time. So
>>>>>>>>> >>>>> that
>>>>>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>>>>>> >>>>> potential
>>>>>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>> thx,
>>>>>>>>> >>>>> >>>>>>>>> Wenlong
>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>>>>>> wenlong@gmail.com>
>>>>>>>>> >>>>> wrote:
>>>>>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>>>>>> need
>>>>>>>>> >>>>> to
>>>>>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>>>>>> gain in
>>>>>>>>> >>>>> Linux
>>>>>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>>>>>> double
>>>>>>>>> >>>>> check the
>>>>>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>>>>>> tests
>>>>>>>>> >>>>> from
>>>>>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>>>>>> so
>>>>>>>>> >>>>> whether
>>>>>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>>>>>> much
>>>>>>>>> >>>>> mess the
>>>>>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>>>>>> From what
>>>>>>>>> >>>>> I
>>>>>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>>>>>> correct
>>>>>>>>> >>>>> mapping
>>>>>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>>>>>> spread
>>>>>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>>>>>> In
>>>>>>>>> >>>>> this
>>>>>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>>>>>> with
>>>>>>>>> >>>>> two
>>>>>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>>>>>> >>>>> Classlib,
>>>>>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>>>>>> it
>>>>>>>>> >>>>> might
>>>>>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>>>>>> >>>>> process?)
>>>>>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>>> >>>>> >>>>>>>>
>>>>>>>>> >>>>> >>>>>>>
>>>>>>>>> >>>>> >>>>>>
>>>>>>>>> >>>>> >>>>>
>>>>>>>>> >>>>> >>>>
>>>>>>>>> >>>>> >>>
>>>>>>>>> >>>>> >>
>>>>>>>>> >>>>> >
>>>>>>>>> >>>>> >
>>>>>>>>> >>>>> >
>>>>>>>>> >>>>> > --
>>>>>>>>> >>>>> > С уважением,
>>>>>>>>> >>>>> > Алексей Федотов,
>>>>>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>>>>>> >>>>> >
>>>>>>>>> >>>>>
>>>>>>>>> >>>>>
>>>>>>>>> >>>>>
>>>>>>>>> >>>>> --
>>>>>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>>>>>> >>>>>
>>>>>>>>> >>>>
>>>>>>>>> >>>>
>>>>>>>>> >>>>
>>>>>>>>> >>>> --
>>>>>>>>> >>>> С уважением,
>>>>>>>>> >>>> Алексей Федотов,
>>>>>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>>>>>> >>>>
>>>>>>>>> >>>
>>>>>>>>> >>>
>>>>>>>>> >>>
>>>>>>>>> >>> --
>>>>>>>>> >>> http://xiao-feng.blogspot.com
>>>>>>>>> >>>
>>>>>>>>> >>
>>>>>>>>> >>
>>>>>>>>> >>
>>>>>>>>> >> --
>>>>>>>>> >> С уважением,
>>>>>>>>> >> Алексей Федотов,
>>>>>>>>> >> ЗАО «Телеком Экспресс»
>>>>>>>>> >>
>>>>>>>>> >
>>>>>>>>> >
>>>>>>>>> >
>>>>>>>>> > --
>>>>>>>>> > http://xiao-feng.blogspot.com
>>>>>>>>> >
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> С уважением,
>>>>>>>> Алексей Федотов,
>>>>>>>> ЗАО «Телеком Экспресс»
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Wenlong Li <we...@gmail.com>.

Btw, Aleksey, to avoid confusion, I expect this patch help VM creation
time or at least help the memory footprint since some modules are
never used after their loading and parsing. :)

On Wed, Jan 14, 2009 at 9:38 PM, Wenlong Li <we...@gmail.com> wrote:
> Alexei,
>
> Sorry for confusing. The patch for review is H6039.patch_2. Please
> kindly provide your comment.
>
> Aleksey,
>
> I have not measured the performance before completing the code review.
> I will do that later.
>
> thx, Wenlong
>
> On Wed, Jan 14, 2009 at 9:14 PM, Wenlong Li <we...@gmail.com> wrote:
>> Pavel,
>>
>> Pls see my comments in the JIRA.
>>
>> thx, Wenlong
>>
>> On Wed, Jan 14, 2009 at 8:44 PM, Pavel Pervov <pm...@gmail.com> wrote:
>>> Please, also, check that jar entry caches still work correctly after your patch.
>>>
>>> Pavel.
>>>
>>> On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
>>>> All,
>>>>
>>>> I submit a new patch for on-demand class loading and parsing. All
>>>> codes are put in VM side, and the mapping info is automatically
>>>> produced.
>>>>
>>>> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>>>>
>>>> Comments are welcome.
>>>>
>>>> Thx, Wenlong
>>>> Managed Runtime Technology Center, Intel
>>>>
>>>> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>> All,
>>>>> At this moment, I move all updates in classlib to VM side such that
>>>>> there is no modularity issue. Next step is to produce the mapping
>>>>> between module and library efficiently and accurately.
>>>>>
>>>>> Comments are welcome.
>>>>>
>>>>> Thx, Wenlong
>>>>> Managed Runtime Technology Center, Intel
>>>>>
>>>>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>>> Thx :)
>>>>>>
>>>>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>>>>> <al...@gmail.com> wrote:
>>>>>>> Sure.
>>>>>>>
>>>>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>>>>> splitting the given classpath into pieces. You already know the new piece
>>>>>>> you add and may skip splitting step.
>>>>>>>
>>>>>>> 2. If I understand you code correctly, the case "pdest >
>>>>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>>>>> assrtion would speed up bug discovery.
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>>>>
>>>>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>>>>> this module will not be loaded.
>>>>>>>>
>>>>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>>>>> further.
>>>>>>>>
>>>>>>>> Thx, Wenlong
>>>>>>>> Managed Runtime Technology Center, Intel
>>>>>>>>
>>>>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>> wrote:
>>>>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>>>>> > <al...@gmail.com> wrote:
>>>>>>>> >> Xiao Feng,
>>>>>>>> >> Thank you for explaining.
>>>>>>>> >>
>>>>>>>> >> I get more minor comments on more commented code, ineffective
>>>>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>>>>> >> still remains. But now I'm happy with the design.
>>>>>>>> >
>>>>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>>>>> > speaking.)
>>>>>>>> >
>>>>>>>> > Thanks,
>>>>>>>> > xiaofeng
>>>>>>>> >
>>>>>>>> >> Sorry for being slow.
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>> wrote:
>>>>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>>>>> >>> <al...@gmail.com> wrote:
>>>>>>>> >>>> Xiao-Feng,
>>>>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>>>>> where
>>>>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>>>>> >>>> constructed dynamically depending on application.
>>>>>>>> >>>
>>>>>>>> >>> Alexei, here is the patch code I found:
>>>>>>>> >>>
>>>>>>>> >>> line 245:
>>>>>>>> >>> +            // Find which jar exports this package
>>>>>>>> >>> +            if (pkgName != NULL) {
>>>>>>>> >>> +                char *boot_class_path =
>>>>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>>>>> >>> +                char *pendingClassPath = NULL;
>>>>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>>>>> >>> +                    if (pdest != NULL) {
>>>>>>>> >>> +                        pendingClassPath =
>>>>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>>>>> >>> +                                               +
>>>>>>>> strlen((*it).first->bytes) + 1);
>>>>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>>>>> >>> +                        // Open this found jar, and read all classes
>>>>>>>> >>> contained in this jar
>>>>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>>>>> tmp_pool);
>>>>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>>>>> >>> as it has been parsed
>>>>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>>>>> >>> +                    } else {
>>>>>>>> >>>
>>>>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>>>>> >>>
>>>>>>>> >>> Thanks,
>>>>>>>> >>> xiaofeng
>>>>>>>> >>>
>>>>>>>> >>>> Thanks.
>>>>>>>> >>>>
>>>>>>>> >>>>
>>>>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>>> wrote:
>>>>>>>> >>>>
>>>>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>>>>> >>>>> <al...@gmail.com> wrote:
>>>>>>>> >>>>> > Aleksey,
>>>>>>>> >>>>> > I like your conclusion.
>>>>>>>> >>>>> >
>>>>>>>> >>>>> > Wenlong,
>>>>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>>>>> startup
>>>>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>>>>> I
>>>>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>>>>> "no"
>>>>>>>> >>>>> > to this question.
>>>>>>>> >>>>>
>>>>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>>>>> expected.
>>>>>>>> >>>>>
>>>>>>>> >>>>> Thanks,
>>>>>>>> >>>>> xiaofeng
>>>>>>>> >>>>>
>>>>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>>>>> are
>>>>>>>> >>>>> > improved by this patch.
>>>>>>>> >>>>> > Thanks!
>>>>>>>> >>>>>
>>>>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>>>>> >>>>> >> Ok, here's the catch.
>>>>>>>> >>>>> >>
>>>>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>>>>> enumerates
>>>>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>>>>> is
>>>>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>>>>> the
>>>>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>>>>> new
>>>>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>>>>> >>>>> >>
>>>>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>>>>> and
>>>>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>>>>> in
>>>>>>>> >>>>> >> case the mapping is wrong?
>>>>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>>>>> split
>>>>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>>>>> with
>>>>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>>>>> could
>>>>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>>>>> be
>>>>>>>> >>>>> >> used by them then ;)
>>>>>>>> >>>>> >>
>>>>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>>>>> which
>>>>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>>>>> once.
>>>>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>>>>> from
>>>>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>>>>> DRLVM
>>>>>>>> >>>>> >> build process?
>>>>>>>> >>>>> >>
>>>>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>>>>> >>>>> >>
>>>>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>>>>> we
>>>>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>>>>> >>>>> >>
>>>>>>>> >>>>> >> Thanks,
>>>>>>>> >>>>> >> Aleksey.
>>>>>>>> >>>>> >>
>>>>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>>>>> wrote:
>>>>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>>>>> the
>>>>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>>>>> change
>>>>>>>> >>>>> >>> the manifest as well?
>>>>>>>> >>>>> >>>
>>>>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>>>>> modulelibrarymapping
>>>>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>>>>> module. It
>>>>>>>> >>>>> >>> doesn't relate to each class.
>>>>>>>> >>>>> >>>
>>>>>>>> >>>>> >>> thx,
>>>>>>>> >>>>> >>> Wenlong
>>>>>>>> >>>>> >>>
>>>>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>>>>> pmcfirst@gmail.com>
>>>>>>>> >>>>> wrote:
>>>>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>>>>> sure how
>>>>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>>>>> >>>>> >>>>
>>>>>>>> >>>>> >>>> WBR,
>>>>>>>> >>>>> >>>>    Pavel.
>>>>>>>> >>>>> >>>>
>>>>>>>> >>>>> >>>>
>>>>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>>>>> >>>>> wrote:
>>>>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>>>>> manually?
>>>>>>>> >>>>> Just
>>>>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>>>>> >>>>> >>>>>
>>>>>>>> >>>>> >>>>> thx,
>>>>>>>> >>>>> >>>>> Wenlong
>>>>>>>> >>>>> >>>>>
>>>>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>>>>> pmcfirst@gmail.com>
>>>>>>>> >>>>> wrote:
>>>>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>>>>> >>>>> file...
>>>>>>>> >>>>> >>>>>>
>>>>>>>> >>>>> >>>>>> WBR,
>>>>>>>> >>>>> >>>>>>    Pavel.
>>>>>>>> >>>>> >>>>>>
>>>>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>>>>> wenlong@gmail.com>
>>>>>>>> >>>>> wrote:
>>>>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>>>>> modifying the
>>>>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>>>>> >>>>> >>>>>>>
>>>>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>>>>> update
>>>>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>>>>> >>>>> >>>>>>>
>>>>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>>>>> >>>>> >>>>>>>
>>>>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>>>>> pmcfirst@gmail.com>
>>>>>>>> >>>>> wrote:
>>>>>>>> >>>>> >>>>>>>> Wenlong,
>>>>>>>> >>>>> >>>>>>>>
>>>>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>>>>> adding new
>>>>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>>>>> >>>>> >>>>>>>>
>>>>>>>> >>>>> >>>>>>>> WBR,
>>>>>>>> >>>>> >>>>>>>>    Pavel.
>>>>>>>> >>>>> >>>>>>>>
>>>>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>>>>> wenlong@gmail.com>
>>>>>>>> >>>>> wrote:
>>>>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>>>>> >>>>> bootclasspath.properties
>>>>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>>>>> >>>>> specific
>>>>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>>>>> time. So
>>>>>>>> >>>>> that
>>>>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>>>>> >>>>> potential
>>>>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>> thx,
>>>>>>>> >>>>> >>>>>>>>> Wenlong
>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>>>>> wenlong@gmail.com>
>>>>>>>> >>>>> wrote:
>>>>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>>>>> need
>>>>>>>> >>>>> to
>>>>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>>>>> gain in
>>>>>>>> >>>>> Linux
>>>>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>>>>> double
>>>>>>>> >>>>> check the
>>>>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>>>>> tests
>>>>>>>> >>>>> from
>>>>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>>>>> so
>>>>>>>> >>>>> whether
>>>>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>>>>> much
>>>>>>>> >>>>> mess the
>>>>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>>>>> From what
>>>>>>>> >>>>> I
>>>>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>>>>> correct
>>>>>>>> >>>>> mapping
>>>>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>>>>> spread
>>>>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>>>>> In
>>>>>>>> >>>>> this
>>>>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>>>>> with
>>>>>>>> >>>>> two
>>>>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>>>>> >>>>> Classlib,
>>>>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>>>>> it
>>>>>>>> >>>>> might
>>>>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>>>>> >>>>> process?)
>>>>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>>>>> >>>>> >>>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>>
>>>>>>>> >>>>> >>>>>>>>
>>>>>>>> >>>>> >>>>>>>
>>>>>>>> >>>>> >>>>>>
>>>>>>>> >>>>> >>>>>
>>>>>>>> >>>>> >>>>
>>>>>>>> >>>>> >>>
>>>>>>>> >>>>> >>
>>>>>>>> >>>>> >
>>>>>>>> >>>>> >
>>>>>>>> >>>>> >
>>>>>>>> >>>>> > --
>>>>>>>> >>>>> > С уважением,
>>>>>>>> >>>>> > Алексей Федотов,
>>>>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>>>>> >>>>> >
>>>>>>>> >>>>>
>>>>>>>> >>>>>
>>>>>>>> >>>>>
>>>>>>>> >>>>> --
>>>>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>>>>> >>>>>
>>>>>>>> >>>>
>>>>>>>> >>>>
>>>>>>>> >>>>
>>>>>>>> >>>> --
>>>>>>>> >>>> С уважением,
>>>>>>>> >>>> Алексей Федотов,
>>>>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>>>>> >>>>
>>>>>>>> >>>
>>>>>>>> >>>
>>>>>>>> >>>
>>>>>>>> >>> --
>>>>>>>> >>> http://xiao-feng.blogspot.com
>>>>>>>> >>>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >>
>>>>>>>> >> --
>>>>>>>> >> С уважением,
>>>>>>>> >> Алексей Федотов,
>>>>>>>> >> ЗАО «Телеком Экспресс»
>>>>>>>> >>
>>>>>>>> >
>>>>>>>> >
>>>>>>>> >
>>>>>>>> > --
>>>>>>>> > http://xiao-feng.blogspot.com
>>>>>>>> >
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> С уважением,
>>>>>>> Алексей Федотов,
>>>>>>> ЗАО «Телеком Экспресс»
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Alexei Fedotov <al...@gmail.com>.

Here review continues and completes,
https://issues.apache.org/jira/browse/HARMONY-6039?focusedCommentId=12664090#action_12664090

Overall, review comments about restructuring the code, avoiding
cut&paste and separating new changes are also pleas of making the
review of these parts more productive.

Wenlong, thanks for the patch!


On Thu, Jan 15, 2009 at 11:49 AM, Alexei Fedotov
<al...@gmail.com> wrote:
> Hello Wenlong,
>
> The intention of the following review is to improve the readability of
> the code. Please find my comments preceded with patch line numbers and
> fix anything you find worthy to fix.
>
> 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-
> cannot understand where the signature file comes from - I cannot find
> apr_file_write for it
> the explanatory comment is welcome
>
> if both mapping and signature files are things introduced by this
> patch why don't we use one file instead of two
>
> 200
> can this be replaced with assert(luni_path)?
>
> 213
> +1 to Aleksey's comment on literals
>
> [have to go, will continue later]
>
> 434
> commented code
>
> Thanks.
>
>
> On Wed, Jan 14, 2009 at 4:38 PM, Wenlong Li <we...@gmail.com> wrote:
>>
>> Alexei,
>>
>> Sorry for confusing. The patch for review is H6039.patch_2. Please
>> kindly provide your comment.
>>
>> Aleksey,
>>
>> I have not measured the performance before completing the code review.
>> I will do that later.
>>
>> thx, Wenlong
>>
>> On Wed, Jan 14, 2009 at 9:14 PM, Wenlong Li <we...@gmail.com> wrote:
>> > Pavel,
>> >
>> > Pls see my comments in the JIRA.
>> >
>> > thx, Wenlong
>> >
>> > On Wed, Jan 14, 2009 at 8:44 PM, Pavel Pervov <pm...@gmail.com> wrote:
>> >> Please, also, check that jar entry caches still work correctly after your patch.
>> >>
>> >> Pavel.
>> >>
>> >> On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
>> >>> All,
>> >>>
>> >>> I submit a new patch for on-demand class loading and parsing. All
>> >>> codes are put in VM side, and the mapping info is automatically
>> >>> produced.
>> >>>
>> >>> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>> >>>
>> >>> Comments are welcome.
>> >>>
>> >>> Thx, Wenlong
>> >>> Managed Runtime Technology Center, Intel
>> >>>
>> >>> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>> >>>> All,
>> >>>> At this moment, I move all updates in classlib to VM side such that
>> >>>> there is no modularity issue. Next step is to produce the mapping
>> >>>> between module and library efficiently and accurately.
>> >>>>
>> >>>> Comments are welcome.
>> >>>>
>> >>>> Thx, Wenlong
>> >>>> Managed Runtime Technology Center, Intel
>> >>>>
>> >>>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>> >>>>> Thx :)
>> >>>>>
>> >>>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>> >>>>> <al...@gmail.com> wrote:
>> >>>>>> Sure.
>> >>>>>>
>> >>>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>> >>>>>> splitting the given classpath into pieces. You already know the new piece
>> >>>>>> you add and may skip splitting step.
>> >>>>>>
>> >>>>>> 2. If I understand you code correctly, the case "pdest >
>> >>>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>> >>>>>> assrtion would speed up bug discovery.
>> >>>>>>
>> >>>>>> Thanks.
>> >>>>>>
>> >>>>>>
>> >>>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>> >>>>>>
>> >>>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>> >>>>>>> modules on demand. If no class in swing.jar is not requested, then
>> >>>>>>> this module will not be loaded.
>> >>>>>>>
>> >>>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>> >>>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>> >>>>>>> them? I just reused some code in Harmony, and didn't optimize them
>> >>>>>>> further.
>> >>>>>>>
>> >>>>>>> Thx, Wenlong
>> >>>>>>> Managed Runtime Technology Center, Intel
>> >>>>>>>
>> >>>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>> >>>>>>> wrote:
>> >>>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>> >>>>>>> > <al...@gmail.com> wrote:
>> >>>>>>> >> Xiao Feng,
>> >>>>>>> >> Thank you for explaining.
>> >>>>>>> >>
>> >>>>>>> >> I get more minor comments on more commented code, ineffective
>> >>>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>> >>>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>> >>>>>>> >> still remains. But now I'm happy with the design.
>> >>>>>>> >
>> >>>>>>> > Alexei, yes, I agree with your comments. These parts should be
>> >>>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>> >>>>>>> > speaking.)
>> >>>>>>> >
>> >>>>>>> > Thanks,
>> >>>>>>> > xiaofeng
>> >>>>>>> >
>> >>>>>>> >> Sorry for being slow.
>> >>>>>>> >>
>> >>>>>>> >>
>> >>>>>>> >>
>> >>>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>> >>>>>>> wrote:
>> >>>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>> >>>>>>> >>> <al...@gmail.com> wrote:
>> >>>>>>> >>>> Xiao-Feng,
>> >>>>>>> >>>> Continuing with the server example could you please give me a hint
>> >>>>>>> where
>> >>>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>> >>>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>> >>>>>>> >>>> constructed dynamically depending on application.
>> >>>>>>> >>>
>> >>>>>>> >>> Alexei, here is the patch code I found:
>> >>>>>>> >>>
>> >>>>>>> >>> line 245:
>> >>>>>>> >>> +            // Find which jar exports this package
>> >>>>>>> >>> +            if (pkgName != NULL) {
>> >>>>>>> >>> +                char *boot_class_path =
>> >>>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>> >>>>>>> >>> +                char *pendingClassPath = NULL;
>> >>>>>>> >>> +                apr_pool_t *tmp_pool;
>> >>>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>> >>>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>> >>>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>> >>>>>>> >>> +                    if (pdest != NULL) {
>> >>>>>>> >>> +                        pendingClassPath =
>> >>>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>> >>>>>>> >>> +                                               +
>> >>>>>>> strlen((*it).first->bytes) + 1);
>> >>>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>> >>>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>> >>>>>>> >>> +                        // Open this found jar, and read all classes
>> >>>>>>> >>> contained in this jar
>> >>>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>> >>>>>>> tmp_pool);
>> >>>>>>> >>> +                        // Erase the found jar from pending jar list
>> >>>>>>> >>> as it has been parsed
>> >>>>>>> >>> +                        env->pending_jar_set.erase(it++);
>> >>>>>>> >>> +                        STD_FREE(pendingClassPath);
>> >>>>>>> >>> +                    } else {
>> >>>>>>> >>>
>> >>>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>> >>>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>> >>>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>> >>>>>>> >>>
>> >>>>>>> >>> Thanks,
>> >>>>>>> >>> xiaofeng
>> >>>>>>> >>>
>> >>>>>>> >>>> Thanks.
>> >>>>>>> >>>>
>> >>>>>>> >>>>
>> >>>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>> >>>>>>> wrote:
>> >>>>>>> >>>>
>> >>>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>> >>>>>>> >>>>> <al...@gmail.com> wrote:
>> >>>>>>> >>>>> > Aleksey,
>> >>>>>>> >>>>> > I like your conclusion.
>> >>>>>>> >>>>> >
>> >>>>>>> >>>>> > Wenlong,
>> >>>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>> >>>>>>> startup
>> >>>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>> >>>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>> >>>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>> >>>>>>> I
>> >>>>>>> >>>>> > believe that the proper design of delayed loading should answer
>> >>>>>>> "no"
>> >>>>>>> >>>>> > to this question.
>> >>>>>>> >>>>>
>> >>>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>> >>>>>>> expected.
>> >>>>>>> >>>>>
>> >>>>>>> >>>>> Thanks,
>> >>>>>>> >>>>> xiaofeng
>> >>>>>>> >>>>>
>> >>>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>> >>>>>>> are
>> >>>>>>> >>>>> > improved by this patch.
>> >>>>>>> >>>>> > Thanks!
>> >>>>>>> >>>>>
>> >>>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>> >>>>>>> >>>>> > <al...@gmail.com> wrote:
>> >>>>>>> >>>>> >> Ok, here's the catch.
>> >>>>>>> >>>>> >>
>> >>>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>> >>>>>>> enumerates
>> >>>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>> >>>>>>> is
>> >>>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>> >>>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>> >>>>>>> >>>>> >> should be updated when new JAR file arrives.
>> >>>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>> >>>>>>> the
>> >>>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>> >>>>>>> new
>> >>>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>> >>>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>> >>>>>>> >>>>> >>
>> >>>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>> >>>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>> >>>>>>> and
>> >>>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>> >>>>>>> in
>> >>>>>>> >>>>> >> case the mapping is wrong?
>> >>>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>> >>>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>> >>>>>>> split
>> >>>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>> >>>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>> >>>>>>> with
>> >>>>>>> >>>>> >> different package layouts and their impact on performance. They
>> >>>>>>> could
>> >>>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>> >>>>>>> be
>> >>>>>>> >>>>> >> used by them then ;)
>> >>>>>>> >>>>> >>
>> >>>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>> >>>>>>> which
>> >>>>>>> >>>>> >> could be done more than once, eventually would be done more than
>> >>>>>>> once.
>> >>>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>> >>>>>>> from
>> >>>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>> >>>>>>> DRLVM
>> >>>>>>> >>>>> >> build process?
>> >>>>>>> >>>>> >>
>> >>>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>> >>>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>> >>>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>> >>>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>> >>>>>>> >>>>> >>
>> >>>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>> >>>>>>> we
>> >>>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>> >>>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>> >>>>>>> >>>>> >>
>> >>>>>>> >>>>> >> Thanks,
>> >>>>>>> >>>>> >> Aleksey.
>> >>>>>>> >>>>> >>
>> >>>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>> >>>>>>> wrote:
>> >>>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>> >>>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>> >>>>>>> the
>> >>>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>> >>>>>>> change
>> >>>>>>> >>>>> >>> the manifest as well?
>> >>>>>>> >>>>> >>>
>> >>>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>> >>>>>>> modulelibrarymapping
>> >>>>>>> >>>>> >>> file only records package info provided by manfiest in each
>> >>>>>>> module. It
>> >>>>>>> >>>>> >>> doesn't relate to each class.
>> >>>>>>> >>>>> >>>
>> >>>>>>> >>>>> >>> thx,
>> >>>>>>> >>>>> >>> Wenlong
>> >>>>>>> >>>>> >>>
>> >>>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>> >>>>>>> pmcfirst@gmail.com>
>> >>>>>>> >>>>> wrote:
>> >>>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>> >>>>>>> sure how
>> >>>>>>> >>>>> >>>> it can be possible to track these changes manually.
>> >>>>>>> >>>>> >>>>
>> >>>>>>> >>>>> >>>> WBR,
>> >>>>>>> >>>>> >>>>    Pavel.
>> >>>>>>> >>>>> >>>>
>> >>>>>>> >>>>> >>>>
>> >>>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>> >>>>>>> >>>>> wrote:
>> >>>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>> >>>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>> >>>>>>> manually?
>> >>>>>>> >>>>> Just
>> >>>>>>> >>>>> >>>>> curious to know the possible reason. :)
>> >>>>>>> >>>>> >>>>>
>> >>>>>>> >>>>> >>>>> thx,
>> >>>>>>> >>>>> >>>>> Wenlong
>> >>>>>>> >>>>> >>>>>
>> >>>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>> >>>>>>> pmcfirst@gmail.com>
>> >>>>>>> >>>>> wrote:
>> >>>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>> >>>>>>> >>>>> file...
>> >>>>>>> >>>>> >>>>>>
>> >>>>>>> >>>>> >>>>>> WBR,
>> >>>>>>> >>>>> >>>>>>    Pavel.
>> >>>>>>> >>>>> >>>>>>
>> >>>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>> >>>>>>> wenlong@gmail.com>
>> >>>>>>> >>>>> wrote:
>> >>>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>> >>>>>>> modifying the
>> >>>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>> >>>>>>> >>>>> >>>>>>>
>> >>>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>> >>>>>>> update
>> >>>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>> >>>>>>> >>>>> >>>>>>>
>> >>>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>> >>>>>>> >>>>> >>>>>>>
>> >>>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>> >>>>>>> pmcfirst@gmail.com>
>> >>>>>>> >>>>> wrote:
>> >>>>>>> >>>>> >>>>>>>> Wenlong,
>> >>>>>>> >>>>> >>>>>>>>
>> >>>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>> >>>>>>> adding new
>> >>>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>> >>>>>>> >>>>> >>>>>>>>
>> >>>>>>> >>>>> >>>>>>>> WBR,
>> >>>>>>> >>>>> >>>>>>>>    Pavel.
>> >>>>>>> >>>>> >>>>>>>>
>> >>>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>> >>>>>>> wenlong@gmail.com>
>> >>>>>>> >>>>> wrote:
>> >>>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>> >>>>>>> >>>>> >>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>> >>>>>>> >>>>> bootclasspath.properties
>> >>>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>> >>>>>>> >>>>> >>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>> >>>>>>> >>>>> specific
>> >>>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>> >>>>>>> time. So
>> >>>>>>> >>>>> that
>> >>>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>> >>>>>>> >>>>> potential
>> >>>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>> >>>>>>> >>>>> >>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>> thx,
>> >>>>>>> >>>>> >>>>>>>>> Wenlong
>> >>>>>>> >>>>> >>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>> >>>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>> >>>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>> >>>>>>> >>>>> >>>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>> >>>>>>> wenlong@gmail.com>
>> >>>>>>> >>>>> wrote:
>> >>>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>> >>>>>>> need
>> >>>>>>> >>>>> to
>> >>>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>> >>>>>>> gain in
>> >>>>>>> >>>>> Linux
>> >>>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>> >>>>>>> double
>> >>>>>>> >>>>> check the
>> >>>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>> >>>>>>> >>>>> >>>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>> >>>>>>> tests
>> >>>>>>> >>>>> from
>> >>>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>> >>>>>>> so
>> >>>>>>> >>>>> whether
>> >>>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>> >>>>>>> much
>> >>>>>>> >>>>> mess the
>> >>>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>> >>>>>>> From what
>> >>>>>>> >>>>> I
>> >>>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>> >>>>>>> correct
>> >>>>>>> >>>>> mapping
>> >>>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>> >>>>>>> spread
>> >>>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>> >>>>>>> In
>> >>>>>>> >>>>> this
>> >>>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>> >>>>>>> >>>>> >>>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>> >>>>>>> with
>> >>>>>>> >>>>> two
>> >>>>>>> >>>>> >>>>>>>>>> serious modifications:
>> >>>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>> >>>>>>> >>>>> Classlib,
>> >>>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>> >>>>>>> it
>> >>>>>>> >>>>> might
>> >>>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>> >>>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>> >>>>>>> >>>>> process?)
>> >>>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>> >>>>>>> >>>>> >>>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>>> Thanks,
>> >>>>>>> >>>>> >>>>>>>>>> Aleksey.
>> >>>>>>> >>>>> >>>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>>
>> >>>>>>> >>>>> >>>>>>>>
>> >>>>>>> >>>>> >>>>>>>
>> >>>>>>> >>>>> >>>>>>
>> >>>>>>> >>>>> >>>>>
>> >>>>>>> >>>>> >>>>
>> >>>>>>> >>>>> >>>
>> >>>>>>> >>>>> >>
>> >>>>>>> >>>>> >
>> >>>>>>> >>>>> >
>> >>>>>>> >>>>> >
>> >>>>>>> >>>>> > --
>> >>>>>>> >>>>> > С уважением,
>> >>>>>>> >>>>> > Алексей Федотов,
>> >>>>>>> >>>>> > ЗАО «Телеком Экспресс»
>> >>>>>>> >>>>> >
>> >>>>>>> >>>>>
>> >>>>>>> >>>>>
>> >>>>>>> >>>>>
>> >>>>>>> >>>>> --
>> >>>>>>> >>>>> http://xiao-feng.blogspot.com
>> >>>>>>> >>>>>
>> >>>>>>> >>>>
>> >>>>>>> >>>>
>> >>>>>>> >>>>
>> >>>>>>> >>>> --
>> >>>>>>> >>>> С уважением,
>> >>>>>>> >>>> Алексей Федотов,
>> >>>>>>> >>>> ЗАО «Телеком Экспресс»
>> >>>>>>> >>>>
>> >>>>>>> >>>
>> >>>>>>> >>>
>> >>>>>>> >>>
>> >>>>>>> >>> --
>> >>>>>>> >>> http://xiao-feng.blogspot.com
>> >>>>>>> >>>
>> >>>>>>> >>
>> >>>>>>> >>
>> >>>>>>> >>
>> >>>>>>> >> --
>> >>>>>>> >> С уважением,
>> >>>>>>> >> Алексей Федотов,
>> >>>>>>> >> ЗАО «Телеком Экспресс»
>> >>>>>>> >>
>> >>>>>>> >
>> >>>>>>> >
>> >>>>>>> >
>> >>>>>>> > --
>> >>>>>>> > http://xiao-feng.blogspot.com
>> >>>>>>> >
>> >>>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>> --
>> >>>>>> С уважением,
>> >>>>>> Алексей Федотов,
>> >>>>>> ЗАО «Телеком Экспресс»
>> >>>>>>
>> >>>>>
>> >>>>
>> >>>
>> >>
>> >
>
>
>
> --
> С уважением,
> Алексей Федотов,
> ЗАО «Телеком Экспресс»
>



-- 
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»

Re: [VM] On-demand class library parsing is ready to commit

Posted by Alexei Fedotov <al...@gmail.com>.

Hello Wenlong,

The intention of the following review is to improve the readability of
the code. Please find my comments preceded with patch line numbers and
fix anything you find worthy to fix.

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-
cannot understand where the signature file comes from - I cannot find
apr_file_write for it
the explanatory comment is welcome

if both mapping and signature files are things introduced by this
patch why don't we use one file instead of two

200
can this be replaced with assert(luni_path)?

213
+1 to Aleksey's comment on literals

[have to go, will continue later]

434
commented code

Thanks.


On Wed, Jan 14, 2009 at 4:38 PM, Wenlong Li <we...@gmail.com> wrote:
>
> Alexei,
>
> Sorry for confusing. The patch for review is H6039.patch_2. Please
> kindly provide your comment.
>
> Aleksey,
>
> I have not measured the performance before completing the code review.
> I will do that later.
>
> thx, Wenlong
>
> On Wed, Jan 14, 2009 at 9:14 PM, Wenlong Li <we...@gmail.com> wrote:
> > Pavel,
> >
> > Pls see my comments in the JIRA.
> >
> > thx, Wenlong
> >
> > On Wed, Jan 14, 2009 at 8:44 PM, Pavel Pervov <pm...@gmail.com> wrote:
> >> Please, also, check that jar entry caches still work correctly after your patch.
> >>
> >> Pavel.
> >>
> >> On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
> >>> All,
> >>>
> >>> I submit a new patch for on-demand class loading and parsing. All
> >>> codes are put in VM side, and the mapping info is automatically
> >>> produced.
> >>>
> >>> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
> >>>
> >>> Comments are welcome.
> >>>
> >>> Thx, Wenlong
> >>> Managed Runtime Technology Center, Intel
> >>>
> >>> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
> >>>> All,
> >>>> At this moment, I move all updates in classlib to VM side such that
> >>>> there is no modularity issue. Next step is to produce the mapping
> >>>> between module and library efficiently and accurately.
> >>>>
> >>>> Comments are welcome.
> >>>>
> >>>> Thx, Wenlong
> >>>> Managed Runtime Technology Center, Intel
> >>>>
> >>>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
> >>>>> Thx :)
> >>>>>
> >>>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
> >>>>> <al...@gmail.com> wrote:
> >>>>>> Sure.
> >>>>>>
> >>>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
> >>>>>> splitting the given classpath into pieces. You already know the new piece
> >>>>>> you add and may skip splitting step.
> >>>>>>
> >>>>>> 2. If I understand you code correctly, the case "pdest >
> >>>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
> >>>>>> assrtion would speed up bug discovery.
> >>>>>>
> >>>>>> Thanks.
> >>>>>>
> >>>>>>
> >>>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
> >>>>>>
> >>>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
> >>>>>>> modules on demand. If no class in swing.jar is not requested, then
> >>>>>>> this module will not be loaded.
> >>>>>>>
> >>>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
> >>>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
> >>>>>>> them? I just reused some code in Harmony, and didn't optimize them
> >>>>>>> further.
> >>>>>>>
> >>>>>>> Thx, Wenlong
> >>>>>>> Managed Runtime Technology Center, Intel
> >>>>>>>
> >>>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
> >>>>>>> wrote:
> >>>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
> >>>>>>> > <al...@gmail.com> wrote:
> >>>>>>> >> Xiao Feng,
> >>>>>>> >> Thank you for explaining.
> >>>>>>> >>
> >>>>>>> >> I get more minor comments on more commented code, ineffective
> >>>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
> >>>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
> >>>>>>> >> still remains. But now I'm happy with the design.
> >>>>>>> >
> >>>>>>> > Alexei, yes, I agree with your comments. These parts should be
> >>>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
> >>>>>>> > speaking.)
> >>>>>>> >
> >>>>>>> > Thanks,
> >>>>>>> > xiaofeng
> >>>>>>> >
> >>>>>>> >> Sorry for being slow.
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
> >>>>>>> wrote:
> >>>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
> >>>>>>> >>> <al...@gmail.com> wrote:
> >>>>>>> >>>> Xiao-Feng,
> >>>>>>> >>>> Continuing with the server example could you please give me a hint
> >>>>>>> where
> >>>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
> >>>>>>> >>>> perception was that the list of what to load was hardcoded and was not
> >>>>>>> >>>> constructed dynamically depending on application.
> >>>>>>> >>>
> >>>>>>> >>> Alexei, here is the patch code I found:
> >>>>>>> >>>
> >>>>>>> >>> line 245:
> >>>>>>> >>> +            // Find which jar exports this package
> >>>>>>> >>> +            if (pkgName != NULL) {
> >>>>>>> >>> +                char *boot_class_path =
> >>>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
> >>>>>>> >>> +                char *pendingClassPath = NULL;
> >>>>>>> >>> +                apr_pool_t *tmp_pool;
> >>>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
> >>>>>>> >>> +                while (it != env->pending_jar_set.end()) {
> >>>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
> >>>>>>> >>> +                    if (pdest != NULL) {
> >>>>>>> >>> +                        pendingClassPath =
> >>>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
> >>>>>>> >>> +                                               +
> >>>>>>> strlen((*it).first->bytes) + 1);
> >>>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
> >>>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
> >>>>>>> >>> +                        // Open this found jar, and read all classes
> >>>>>>> >>> contained in this jar
> >>>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
> >>>>>>> tmp_pool);
> >>>>>>> >>> +                        // Erase the found jar from pending jar list
> >>>>>>> >>> as it has been parsed
> >>>>>>> >>> +                        env->pending_jar_set.erase(it++);
> >>>>>>> >>> +                        STD_FREE(pendingClassPath);
> >>>>>>> >>> +                    } else {
> >>>>>>> >>>
> >>>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
> >>>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
> >>>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
> >>>>>>> >>>
> >>>>>>> >>> Thanks,
> >>>>>>> >>> xiaofeng
> >>>>>>> >>>
> >>>>>>> >>>> Thanks.
> >>>>>>> >>>>
> >>>>>>> >>>>
> >>>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
> >>>>>>> wrote:
> >>>>>>> >>>>
> >>>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
> >>>>>>> >>>>> <al...@gmail.com> wrote:
> >>>>>>> >>>>> > Aleksey,
> >>>>>>> >>>>> > I like your conclusion.
> >>>>>>> >>>>> >
> >>>>>>> >>>>> > Wenlong,
> >>>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
> >>>>>>> startup
> >>>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
> >>>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
> >>>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
> >>>>>>> I
> >>>>>>> >>>>> > believe that the proper design of delayed loading should answer
> >>>>>>> "no"
> >>>>>>> >>>>> > to this question.
> >>>>>>> >>>>>
> >>>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
> >>>>>>> expected.
> >>>>>>> >>>>>
> >>>>>>> >>>>> Thanks,
> >>>>>>> >>>>> xiaofeng
> >>>>>>> >>>>>
> >>>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
> >>>>>>> are
> >>>>>>> >>>>> > improved by this patch.
> >>>>>>> >>>>> > Thanks!
> >>>>>>> >>>>>
> >>>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
> >>>>>>> >>>>> > <al...@gmail.com> wrote:
> >>>>>>> >>>>> >> Ok, here's the catch.
> >>>>>>> >>>>> >>
> >>>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
> >>>>>>> enumerates
> >>>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
> >>>>>>> is
> >>>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
> >>>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
> >>>>>>> >>>>> >> should be updated when new JAR file arrives.
> >>>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
> >>>>>>> the
> >>>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
> >>>>>>> new
> >>>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
> >>>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
> >>>>>>> >>>>> >>
> >>>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
> >>>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
> >>>>>>> and
> >>>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
> >>>>>>> in
> >>>>>>> >>>>> >> case the mapping is wrong?
> >>>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
> >>>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
> >>>>>>> split
> >>>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
> >>>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
> >>>>>>> with
> >>>>>>> >>>>> >> different package layouts and their impact on performance. They
> >>>>>>> could
> >>>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
> >>>>>>> be
> >>>>>>> >>>>> >> used by them then ;)
> >>>>>>> >>>>> >>
> >>>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
> >>>>>>> which
> >>>>>>> >>>>> >> could be done more than once, eventually would be done more than
> >>>>>>> once.
> >>>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
> >>>>>>> from
> >>>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
> >>>>>>> DRLVM
> >>>>>>> >>>>> >> build process?
> >>>>>>> >>>>> >>
> >>>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
> >>>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
> >>>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
> >>>>>>> >>>>> >>  b. treated in DRLVM classloader only.
> >>>>>>> >>>>> >>
> >>>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
> >>>>>>> we
> >>>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
> >>>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
> >>>>>>> >>>>> >>
> >>>>>>> >>>>> >> Thanks,
> >>>>>>> >>>>> >> Aleksey.
> >>>>>>> >>>>> >>
> >>>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
> >>>>>>> wrote:
> >>>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
> >>>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
> >>>>>>> the
> >>>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
> >>>>>>> change
> >>>>>>> >>>>> >>> the manifest as well?
> >>>>>>> >>>>> >>>
> >>>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
> >>>>>>> modulelibrarymapping
> >>>>>>> >>>>> >>> file only records package info provided by manfiest in each
> >>>>>>> module. It
> >>>>>>> >>>>> >>> doesn't relate to each class.
> >>>>>>> >>>>> >>>
> >>>>>>> >>>>> >>> thx,
> >>>>>>> >>>>> >>> Wenlong
> >>>>>>> >>>>> >>>
> >>>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
> >>>>>>> pmcfirst@gmail.com>
> >>>>>>> >>>>> wrote:
> >>>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
> >>>>>>> sure how
> >>>>>>> >>>>> >>>> it can be possible to track these changes manually.
> >>>>>>> >>>>> >>>>
> >>>>>>> >>>>> >>>> WBR,
> >>>>>>> >>>>> >>>>    Pavel.
> >>>>>>> >>>>> >>>>
> >>>>>>> >>>>> >>>>
> >>>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
> >>>>>>> >>>>> wrote:
> >>>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
> >>>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
> >>>>>>> manually?
> >>>>>>> >>>>> Just
> >>>>>>> >>>>> >>>>> curious to know the possible reason. :)
> >>>>>>> >>>>> >>>>>
> >>>>>>> >>>>> >>>>> thx,
> >>>>>>> >>>>> >>>>> Wenlong
> >>>>>>> >>>>> >>>>>
> >>>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
> >>>>>>> pmcfirst@gmail.com>
> >>>>>>> >>>>> wrote:
> >>>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
> >>>>>>> >>>>> file...
> >>>>>>> >>>>> >>>>>>
> >>>>>>> >>>>> >>>>>> WBR,
> >>>>>>> >>>>> >>>>>>    Pavel.
> >>>>>>> >>>>> >>>>>>
> >>>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
> >>>>>>> wenlong@gmail.com>
> >>>>>>> >>>>> wrote:
> >>>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
> >>>>>>> modifying the
> >>>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
> >>>>>>> >>>>> >>>>>>>
> >>>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
> >>>>>>> update
> >>>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
> >>>>>>> >>>>> >>>>>>>
> >>>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
> >>>>>>> >>>>> >>>>>>>
> >>>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
> >>>>>>> pmcfirst@gmail.com>
> >>>>>>> >>>>> wrote:
> >>>>>>> >>>>> >>>>>>>> Wenlong,
> >>>>>>> >>>>> >>>>>>>>
> >>>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
> >>>>>>> adding new
> >>>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
> >>>>>>> >>>>> >>>>>>>>
> >>>>>>> >>>>> >>>>>>>> WBR,
> >>>>>>> >>>>> >>>>>>>>    Pavel.
> >>>>>>> >>>>> >>>>>>>>
> >>>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
> >>>>>>> wenlong@gmail.com>
> >>>>>>> >>>>> wrote:
> >>>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
> >>>>>>> >>>>> >>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
> >>>>>>> >>>>> bootclasspath.properties
> >>>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
> >>>>>>> >>>>> >>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
> >>>>>>> >>>>> specific
> >>>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
> >>>>>>> time. So
> >>>>>>> >>>>> that
> >>>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
> >>>>>>> >>>>> potential
> >>>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
> >>>>>>> >>>>> >>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>> thx,
> >>>>>>> >>>>> >>>>>>>>> Wenlong
> >>>>>>> >>>>> >>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
> >>>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
> >>>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
> >>>>>>> >>>>> >>>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
> >>>>>>> wenlong@gmail.com>
> >>>>>>> >>>>> wrote:
> >>>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
> >>>>>>> need
> >>>>>>> >>>>> to
> >>>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
> >>>>>>> gain in
> >>>>>>> >>>>> Linux
> >>>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
> >>>>>>> double
> >>>>>>> >>>>> check the
> >>>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
> >>>>>>> >>>>> >>>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
> >>>>>>> tests
> >>>>>>> >>>>> from
> >>>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
> >>>>>>> so
> >>>>>>> >>>>> whether
> >>>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
> >>>>>>> much
> >>>>>>> >>>>> mess the
> >>>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
> >>>>>>> From what
> >>>>>>> >>>>> I
> >>>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
> >>>>>>> correct
> >>>>>>> >>>>> mapping
> >>>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
> >>>>>>> spread
> >>>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
> >>>>>>> In
> >>>>>>> >>>>> this
> >>>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
> >>>>>>> >>>>> >>>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
> >>>>>>> with
> >>>>>>> >>>>> two
> >>>>>>> >>>>> >>>>>>>>>> serious modifications:
> >>>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
> >>>>>>> >>>>> Classlib,
> >>>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
> >>>>>>> it
> >>>>>>> >>>>> might
> >>>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
> >>>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
> >>>>>>> >>>>> process?)
> >>>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
> >>>>>>> >>>>> >>>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>>> Thanks,
> >>>>>>> >>>>> >>>>>>>>>> Aleksey.
> >>>>>>> >>>>> >>>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>>
> >>>>>>> >>>>> >>>>>>>>
> >>>>>>> >>>>> >>>>>>>
> >>>>>>> >>>>> >>>>>>
> >>>>>>> >>>>> >>>>>
> >>>>>>> >>>>> >>>>
> >>>>>>> >>>>> >>>
> >>>>>>> >>>>> >>
> >>>>>>> >>>>> >
> >>>>>>> >>>>> >
> >>>>>>> >>>>> >
> >>>>>>> >>>>> > --
> >>>>>>> >>>>> > С уважением,
> >>>>>>> >>>>> > Алексей Федотов,
> >>>>>>> >>>>> > ЗАО «Телеком Экспресс»
> >>>>>>> >>>>> >
> >>>>>>> >>>>>
> >>>>>>> >>>>>
> >>>>>>> >>>>>
> >>>>>>> >>>>> --
> >>>>>>> >>>>> http://xiao-feng.blogspot.com
> >>>>>>> >>>>>
> >>>>>>> >>>>
> >>>>>>> >>>>
> >>>>>>> >>>>
> >>>>>>> >>>> --
> >>>>>>> >>>> С уважением,
> >>>>>>> >>>> Алексей Федотов,
> >>>>>>> >>>> ЗАО «Телеком Экспресс»
> >>>>>>> >>>>
> >>>>>>> >>>
> >>>>>>> >>>
> >>>>>>> >>>
> >>>>>>> >>> --
> >>>>>>> >>> http://xiao-feng.blogspot.com
> >>>>>>> >>>
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >> --
> >>>>>>> >> С уважением,
> >>>>>>> >> Алексей Федотов,
> >>>>>>> >> ЗАО «Телеком Экспресс»
> >>>>>>> >>
> >>>>>>> >
> >>>>>>> >
> >>>>>>> >
> >>>>>>> > --
> >>>>>>> > http://xiao-feng.blogspot.com
> >>>>>>> >
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> С уважением,
> >>>>>> Алексей Федотов,
> >>>>>> ЗАО «Телеком Экспресс»
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >



--
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»

Re: [VM] On-demand class library parsing is ready to commit

Posted by Wenlong Li <we...@gmail.com>.

Alexei,

Sorry for confusing. The patch for review is H6039.patch_2. Please
kindly provide your comment.

Aleksey,

I have not measured the performance before completing the code review.
I will do that later.

thx, Wenlong

On Wed, Jan 14, 2009 at 9:14 PM, Wenlong Li <we...@gmail.com> wrote:
> Pavel,
>
> Pls see my comments in the JIRA.
>
> thx, Wenlong
>
> On Wed, Jan 14, 2009 at 8:44 PM, Pavel Pervov <pm...@gmail.com> wrote:
>> Please, also, check that jar entry caches still work correctly after your patch.
>>
>> Pavel.
>>
>> On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
>>> All,
>>>
>>> I submit a new patch for on-demand class loading and parsing. All
>>> codes are put in VM side, and the mapping info is automatically
>>> produced.
>>>
>>> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>>>
>>> Comments are welcome.
>>>
>>> Thx, Wenlong
>>> Managed Runtime Technology Center, Intel
>>>
>>> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>> All,
>>>> At this moment, I move all updates in classlib to VM side such that
>>>> there is no modularity issue. Next step is to produce the mapping
>>>> between module and library efficiently and accurately.
>>>>
>>>> Comments are welcome.
>>>>
>>>> Thx, Wenlong
>>>> Managed Runtime Technology Center, Intel
>>>>
>>>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>>> Thx :)
>>>>>
>>>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>>>> <al...@gmail.com> wrote:
>>>>>> Sure.
>>>>>>
>>>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>>>> splitting the given classpath into pieces. You already know the new piece
>>>>>> you add and may skip splitting step.
>>>>>>
>>>>>> 2. If I understand you code correctly, the case "pdest >
>>>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>>>> assrtion would speed up bug discovery.
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>>
>>>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>>>
>>>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>>>> this module will not be loaded.
>>>>>>>
>>>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>>>> further.
>>>>>>>
>>>>>>> Thx, Wenlong
>>>>>>> Managed Runtime Technology Center, Intel
>>>>>>>
>>>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>> wrote:
>>>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>>>> > <al...@gmail.com> wrote:
>>>>>>> >> Xiao Feng,
>>>>>>> >> Thank you for explaining.
>>>>>>> >>
>>>>>>> >> I get more minor comments on more commented code, ineffective
>>>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>>>> >> still remains. But now I'm happy with the design.
>>>>>>> >
>>>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>>>> > speaking.)
>>>>>>> >
>>>>>>> > Thanks,
>>>>>>> > xiaofeng
>>>>>>> >
>>>>>>> >> Sorry for being slow.
>>>>>>> >>
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>> wrote:
>>>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>>>> >>> <al...@gmail.com> wrote:
>>>>>>> >>>> Xiao-Feng,
>>>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>>>> where
>>>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>>>> >>>> constructed dynamically depending on application.
>>>>>>> >>>
>>>>>>> >>> Alexei, here is the patch code I found:
>>>>>>> >>>
>>>>>>> >>> line 245:
>>>>>>> >>> +            // Find which jar exports this package
>>>>>>> >>> +            if (pkgName != NULL) {
>>>>>>> >>> +                char *boot_class_path =
>>>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>>>> >>> +                char *pendingClassPath = NULL;
>>>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>>>> >>> +                    if (pdest != NULL) {
>>>>>>> >>> +                        pendingClassPath =
>>>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>>>> >>> +                                               +
>>>>>>> strlen((*it).first->bytes) + 1);
>>>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>>>> >>> +                        // Open this found jar, and read all classes
>>>>>>> >>> contained in this jar
>>>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>>>> tmp_pool);
>>>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>>>> >>> as it has been parsed
>>>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>>>> >>> +                    } else {
>>>>>>> >>>
>>>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>>>> >>>
>>>>>>> >>> Thanks,
>>>>>>> >>> xiaofeng
>>>>>>> >>>
>>>>>>> >>>> Thanks.
>>>>>>> >>>>
>>>>>>> >>>>
>>>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>>> wrote:
>>>>>>> >>>>
>>>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>>>> >>>>> <al...@gmail.com> wrote:
>>>>>>> >>>>> > Aleksey,
>>>>>>> >>>>> > I like your conclusion.
>>>>>>> >>>>> >
>>>>>>> >>>>> > Wenlong,
>>>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>>>> startup
>>>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>>>> I
>>>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>>>> "no"
>>>>>>> >>>>> > to this question.
>>>>>>> >>>>>
>>>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>>>> expected.
>>>>>>> >>>>>
>>>>>>> >>>>> Thanks,
>>>>>>> >>>>> xiaofeng
>>>>>>> >>>>>
>>>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>>>> are
>>>>>>> >>>>> > improved by this patch.
>>>>>>> >>>>> > Thanks!
>>>>>>> >>>>>
>>>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>>>> >>>>> >> Ok, here's the catch.
>>>>>>> >>>>> >>
>>>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>>>> enumerates
>>>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>>>> is
>>>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>>>> the
>>>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>>>> new
>>>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>>>> >>>>> >>
>>>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>>>> and
>>>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>>>> in
>>>>>>> >>>>> >> case the mapping is wrong?
>>>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>>>> split
>>>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>>>> with
>>>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>>>> could
>>>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>>>> be
>>>>>>> >>>>> >> used by them then ;)
>>>>>>> >>>>> >>
>>>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>>>> which
>>>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>>>> once.
>>>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>>>> from
>>>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>>>> DRLVM
>>>>>>> >>>>> >> build process?
>>>>>>> >>>>> >>
>>>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>>>> >>>>> >>
>>>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>>>> we
>>>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>>>> >>>>> >>
>>>>>>> >>>>> >> Thanks,
>>>>>>> >>>>> >> Aleksey.
>>>>>>> >>>>> >>
>>>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>>>> wrote:
>>>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>>>> the
>>>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>>>> change
>>>>>>> >>>>> >>> the manifest as well?
>>>>>>> >>>>> >>>
>>>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>>>> modulelibrarymapping
>>>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>>>> module. It
>>>>>>> >>>>> >>> doesn't relate to each class.
>>>>>>> >>>>> >>>
>>>>>>> >>>>> >>> thx,
>>>>>>> >>>>> >>> Wenlong
>>>>>>> >>>>> >>>
>>>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>>>> pmcfirst@gmail.com>
>>>>>>> >>>>> wrote:
>>>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>>>> sure how
>>>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>>>> >>>>> >>>>
>>>>>>> >>>>> >>>> WBR,
>>>>>>> >>>>> >>>>    Pavel.
>>>>>>> >>>>> >>>>
>>>>>>> >>>>> >>>>
>>>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>>>> >>>>> wrote:
>>>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>>>> manually?
>>>>>>> >>>>> Just
>>>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>>>> >>>>> >>>>>
>>>>>>> >>>>> >>>>> thx,
>>>>>>> >>>>> >>>>> Wenlong
>>>>>>> >>>>> >>>>>
>>>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>>>> pmcfirst@gmail.com>
>>>>>>> >>>>> wrote:
>>>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>>>> >>>>> file...
>>>>>>> >>>>> >>>>>>
>>>>>>> >>>>> >>>>>> WBR,
>>>>>>> >>>>> >>>>>>    Pavel.
>>>>>>> >>>>> >>>>>>
>>>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>>>> wenlong@gmail.com>
>>>>>>> >>>>> wrote:
>>>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>>>> modifying the
>>>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>>>> >>>>> >>>>>>>
>>>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>>>> update
>>>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>>>> >>>>> >>>>>>>
>>>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>>>> >>>>> >>>>>>>
>>>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>>>> pmcfirst@gmail.com>
>>>>>>> >>>>> wrote:
>>>>>>> >>>>> >>>>>>>> Wenlong,
>>>>>>> >>>>> >>>>>>>>
>>>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>>>> adding new
>>>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>>>> >>>>> >>>>>>>>
>>>>>>> >>>>> >>>>>>>> WBR,
>>>>>>> >>>>> >>>>>>>>    Pavel.
>>>>>>> >>>>> >>>>>>>>
>>>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>>>> wenlong@gmail.com>
>>>>>>> >>>>> wrote:
>>>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>>>> >>>>> >>>>>>>>>
>>>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>>>> >>>>> bootclasspath.properties
>>>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>>>> >>>>> >>>>>>>>>
>>>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>>>> >>>>> specific
>>>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>>>> time. So
>>>>>>> >>>>> that
>>>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>>>> >>>>> potential
>>>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>>>> >>>>> >>>>>>>>>
>>>>>>> >>>>> >>>>>>>>> thx,
>>>>>>> >>>>> >>>>>>>>> Wenlong
>>>>>>> >>>>> >>>>>>>>>
>>>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>>>> >>>>> >>>>>>>>>>
>>>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>>>> wenlong@gmail.com>
>>>>>>> >>>>> wrote:
>>>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>>>> need
>>>>>>> >>>>> to
>>>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>>>> gain in
>>>>>>> >>>>> Linux
>>>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>>>> double
>>>>>>> >>>>> check the
>>>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>>>> >>>>> >>>>>>>>>>
>>>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>>>> tests
>>>>>>> >>>>> from
>>>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>>>> so
>>>>>>> >>>>> whether
>>>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>>>> much
>>>>>>> >>>>> mess the
>>>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>>>> From what
>>>>>>> >>>>> I
>>>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>>>> correct
>>>>>>> >>>>> mapping
>>>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>>>> spread
>>>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>>>> In
>>>>>>> >>>>> this
>>>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>>>> >>>>> >>>>>>>>>>
>>>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>>>> with
>>>>>>> >>>>> two
>>>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>>>> >>>>> Classlib,
>>>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>>>> it
>>>>>>> >>>>> might
>>>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>>>> >>>>> process?)
>>>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>>>> >>>>> >>>>>>>>>>
>>>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>>>> >>>>> >>>>>>>>>>
>>>>>>> >>>>> >>>>>>>>>
>>>>>>> >>>>> >>>>>>>>
>>>>>>> >>>>> >>>>>>>
>>>>>>> >>>>> >>>>>>
>>>>>>> >>>>> >>>>>
>>>>>>> >>>>> >>>>
>>>>>>> >>>>> >>>
>>>>>>> >>>>> >>
>>>>>>> >>>>> >
>>>>>>> >>>>> >
>>>>>>> >>>>> >
>>>>>>> >>>>> > --
>>>>>>> >>>>> > С уважением,
>>>>>>> >>>>> > Алексей Федотов,
>>>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>>>> >>>>> >
>>>>>>> >>>>>
>>>>>>> >>>>>
>>>>>>> >>>>>
>>>>>>> >>>>> --
>>>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>>>> >>>>>
>>>>>>> >>>>
>>>>>>> >>>>
>>>>>>> >>>>
>>>>>>> >>>> --
>>>>>>> >>>> С уважением,
>>>>>>> >>>> Алексей Федотов,
>>>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>>>> >>>>
>>>>>>> >>>
>>>>>>> >>>
>>>>>>> >>>
>>>>>>> >>> --
>>>>>>> >>> http://xiao-feng.blogspot.com
>>>>>>> >>>
>>>>>>> >>
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> --
>>>>>>> >> С уважением,
>>>>>>> >> Алексей Федотов,
>>>>>>> >> ЗАО «Телеком Экспресс»
>>>>>>> >>
>>>>>>> >
>>>>>>> >
>>>>>>> >
>>>>>>> > --
>>>>>>> > http://xiao-feng.blogspot.com
>>>>>>> >
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> С уважением,
>>>>>> Алексей Федотов,
>>>>>> ЗАО «Телеком Экспресс»
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Wenlong Li <we...@gmail.com>.

Pavel,

Pls see my comments in the JIRA.

thx, Wenlong

On Wed, Jan 14, 2009 at 8:44 PM, Pavel Pervov <pm...@gmail.com> wrote:
> Please, also, check that jar entry caches still work correctly after your patch.
>
> Pavel.
>
> On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
>> All,
>>
>> I submit a new patch for on-demand class loading and parsing. All
>> codes are put in VM side, and the mapping info is automatically
>> produced.
>>
>> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>>
>> Comments are welcome.
>>
>> Thx, Wenlong
>> Managed Runtime Technology Center, Intel
>>
>> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>> All,
>>> At this moment, I move all updates in classlib to VM side such that
>>> there is no modularity issue. Next step is to produce the mapping
>>> between module and library efficiently and accurately.
>>>
>>> Comments are welcome.
>>>
>>> Thx, Wenlong
>>> Managed Runtime Technology Center, Intel
>>>
>>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>>> Thx :)
>>>>
>>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>>> <al...@gmail.com> wrote:
>>>>> Sure.
>>>>>
>>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>>> splitting the given classpath into pieces. You already know the new piece
>>>>> you add and may skip splitting step.
>>>>>
>>>>> 2. If I understand you code correctly, the case "pdest >
>>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>>> assrtion would speed up bug discovery.
>>>>>
>>>>> Thanks.
>>>>>
>>>>>
>>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>>
>>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>>> this module will not be loaded.
>>>>>>
>>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>>> further.
>>>>>>
>>>>>> Thx, Wenlong
>>>>>> Managed Runtime Technology Center, Intel
>>>>>>
>>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>>> wrote:
>>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>>> > <al...@gmail.com> wrote:
>>>>>> >> Xiao Feng,
>>>>>> >> Thank you for explaining.
>>>>>> >>
>>>>>> >> I get more minor comments on more commented code, ineffective
>>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>>> >> still remains. But now I'm happy with the design.
>>>>>> >
>>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>>> > speaking.)
>>>>>> >
>>>>>> > Thanks,
>>>>>> > xiaofeng
>>>>>> >
>>>>>> >> Sorry for being slow.
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>> wrote:
>>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>>> >>> <al...@gmail.com> wrote:
>>>>>> >>>> Xiao-Feng,
>>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>>> where
>>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>>> >>>> constructed dynamically depending on application.
>>>>>> >>>
>>>>>> >>> Alexei, here is the patch code I found:
>>>>>> >>>
>>>>>> >>> line 245:
>>>>>> >>> +            // Find which jar exports this package
>>>>>> >>> +            if (pkgName != NULL) {
>>>>>> >>> +                char *boot_class_path =
>>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>>> >>> +                char *pendingClassPath = NULL;
>>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>>> >>> +                    if (pdest != NULL) {
>>>>>> >>> +                        pendingClassPath =
>>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>>> >>> +                                               +
>>>>>> strlen((*it).first->bytes) + 1);
>>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>>> >>> +                        // Open this found jar, and read all classes
>>>>>> >>> contained in this jar
>>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>>> tmp_pool);
>>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>>> >>> as it has been parsed
>>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>>> >>> +                    } else {
>>>>>> >>>
>>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>>> >>>
>>>>>> >>> Thanks,
>>>>>> >>> xiaofeng
>>>>>> >>>
>>>>>> >>>> Thanks.
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>>> wrote:
>>>>>> >>>>
>>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>>> >>>>> <al...@gmail.com> wrote:
>>>>>> >>>>> > Aleksey,
>>>>>> >>>>> > I like your conclusion.
>>>>>> >>>>> >
>>>>>> >>>>> > Wenlong,
>>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>>> startup
>>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>>> I
>>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>>> "no"
>>>>>> >>>>> > to this question.
>>>>>> >>>>>
>>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>>> expected.
>>>>>> >>>>>
>>>>>> >>>>> Thanks,
>>>>>> >>>>> xiaofeng
>>>>>> >>>>>
>>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>>> are
>>>>>> >>>>> > improved by this patch.
>>>>>> >>>>> > Thanks!
>>>>>> >>>>>
>>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>>> >>>>> >> Ok, here's the catch.
>>>>>> >>>>> >>
>>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>>> enumerates
>>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>>> is
>>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>>> the
>>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>>> new
>>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>>> >>>>> >>
>>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>>> and
>>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>>> in
>>>>>> >>>>> >> case the mapping is wrong?
>>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>>> split
>>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>>> with
>>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>>> could
>>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>>> be
>>>>>> >>>>> >> used by them then ;)
>>>>>> >>>>> >>
>>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>>> which
>>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>>> once.
>>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>>> from
>>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>>> DRLVM
>>>>>> >>>>> >> build process?
>>>>>> >>>>> >>
>>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>>> >>>>> >>
>>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>>> we
>>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>>> >>>>> >>
>>>>>> >>>>> >> Thanks,
>>>>>> >>>>> >> Aleksey.
>>>>>> >>>>> >>
>>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>>> wrote:
>>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>>> the
>>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>>> change
>>>>>> >>>>> >>> the manifest as well?
>>>>>> >>>>> >>>
>>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>>> modulelibrarymapping
>>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>>> module. It
>>>>>> >>>>> >>> doesn't relate to each class.
>>>>>> >>>>> >>>
>>>>>> >>>>> >>> thx,
>>>>>> >>>>> >>> Wenlong
>>>>>> >>>>> >>>
>>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>>> pmcfirst@gmail.com>
>>>>>> >>>>> wrote:
>>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>>> sure how
>>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>>> >>>>> >>>>
>>>>>> >>>>> >>>> WBR,
>>>>>> >>>>> >>>>    Pavel.
>>>>>> >>>>> >>>>
>>>>>> >>>>> >>>>
>>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>>> >>>>> wrote:
>>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>>> manually?
>>>>>> >>>>> Just
>>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>>> >>>>> >>>>>
>>>>>> >>>>> >>>>> thx,
>>>>>> >>>>> >>>>> Wenlong
>>>>>> >>>>> >>>>>
>>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>>> pmcfirst@gmail.com>
>>>>>> >>>>> wrote:
>>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>>> >>>>> file...
>>>>>> >>>>> >>>>>>
>>>>>> >>>>> >>>>>> WBR,
>>>>>> >>>>> >>>>>>    Pavel.
>>>>>> >>>>> >>>>>>
>>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>>> wenlong@gmail.com>
>>>>>> >>>>> wrote:
>>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>>> modifying the
>>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>>> >>>>> >>>>>>>
>>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>>> update
>>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>>> >>>>> >>>>>>>
>>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>>> >>>>> >>>>>>>
>>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>>> pmcfirst@gmail.com>
>>>>>> >>>>> wrote:
>>>>>> >>>>> >>>>>>>> Wenlong,
>>>>>> >>>>> >>>>>>>>
>>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>>> adding new
>>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>>> >>>>> >>>>>>>>
>>>>>> >>>>> >>>>>>>> WBR,
>>>>>> >>>>> >>>>>>>>    Pavel.
>>>>>> >>>>> >>>>>>>>
>>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>>> wenlong@gmail.com>
>>>>>> >>>>> wrote:
>>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>>> >>>>> >>>>>>>>>
>>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>>> >>>>> bootclasspath.properties
>>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>>> >>>>> >>>>>>>>>
>>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>>> >>>>> specific
>>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>>> time. So
>>>>>> >>>>> that
>>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>>> >>>>> potential
>>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>>> >>>>> >>>>>>>>>
>>>>>> >>>>> >>>>>>>>> thx,
>>>>>> >>>>> >>>>>>>>> Wenlong
>>>>>> >>>>> >>>>>>>>>
>>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>>> >>>>> >>>>>>>>>>
>>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>>> wenlong@gmail.com>
>>>>>> >>>>> wrote:
>>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>>> need
>>>>>> >>>>> to
>>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>>> gain in
>>>>>> >>>>> Linux
>>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>>> double
>>>>>> >>>>> check the
>>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>>> >>>>> >>>>>>>>>>
>>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>>> tests
>>>>>> >>>>> from
>>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>>> so
>>>>>> >>>>> whether
>>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>>> much
>>>>>> >>>>> mess the
>>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>>> From what
>>>>>> >>>>> I
>>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>>> correct
>>>>>> >>>>> mapping
>>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>>> spread
>>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>>> In
>>>>>> >>>>> this
>>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>>> >>>>> >>>>>>>>>>
>>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>>> with
>>>>>> >>>>> two
>>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>>> >>>>> Classlib,
>>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>>> it
>>>>>> >>>>> might
>>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>>> >>>>> process?)
>>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>>> >>>>> >>>>>>>>>>
>>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>>> >>>>> >>>>>>>>>>
>>>>>> >>>>> >>>>>>>>>
>>>>>> >>>>> >>>>>>>>
>>>>>> >>>>> >>>>>>>
>>>>>> >>>>> >>>>>>
>>>>>> >>>>> >>>>>
>>>>>> >>>>> >>>>
>>>>>> >>>>> >>>
>>>>>> >>>>> >>
>>>>>> >>>>> >
>>>>>> >>>>> >
>>>>>> >>>>> >
>>>>>> >>>>> > --
>>>>>> >>>>> > С уважением,
>>>>>> >>>>> > Алексей Федотов,
>>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>>> >>>>> >
>>>>>> >>>>>
>>>>>> >>>>>
>>>>>> >>>>>
>>>>>> >>>>> --
>>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>>> >>>>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> --
>>>>>> >>>> С уважением,
>>>>>> >>>> Алексей Федотов,
>>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>>> >>>>
>>>>>> >>>
>>>>>> >>>
>>>>>> >>>
>>>>>> >>> --
>>>>>> >>> http://xiao-feng.blogspot.com
>>>>>> >>>
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >> --
>>>>>> >> С уважением,
>>>>>> >> Алексей Федотов,
>>>>>> >> ЗАО «Телеком Экспресс»
>>>>>> >>
>>>>>> >
>>>>>> >
>>>>>> >
>>>>>> > --
>>>>>> > http://xiao-feng.blogspot.com
>>>>>> >
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> С уважением,
>>>>> Алексей Федотов,
>>>>> ЗАО «Телеком Экспресс»
>>>>>
>>>>
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Pavel Pervov <pm...@gmail.com>.

Please, also, check that jar entry caches still work correctly after your patch.

Pavel.

On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
> All,
>
> I submit a new patch for on-demand class loading and parsing. All
> codes are put in VM side, and the mapping info is automatically
> produced.
>
> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>
> Comments are welcome.
>
> Thx, Wenlong
> Managed Runtime Technology Center, Intel
>
> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>> All,
>> At this moment, I move all updates in classlib to VM side such that
>> there is no modularity issue. Next step is to produce the mapping
>> between module and library efficiently and accurately.
>>
>> Comments are welcome.
>>
>> Thx, Wenlong
>> Managed Runtime Technology Center, Intel
>>
>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>> Thx :)
>>>
>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>> <al...@gmail.com> wrote:
>>>> Sure.
>>>>
>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>> splitting the given classpath into pieces. You already know the new piece
>>>> you add and may skip splitting step.
>>>>
>>>> 2. If I understand you code correctly, the case "pdest >
>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>> assrtion would speed up bug discovery.
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>
>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>> this module will not be loaded.
>>>>>
>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>> further.
>>>>>
>>>>> Thx, Wenlong
>>>>> Managed Runtime Technology Center, Intel
>>>>>
>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>> > <al...@gmail.com> wrote:
>>>>> >> Xiao Feng,
>>>>> >> Thank you for explaining.
>>>>> >>
>>>>> >> I get more minor comments on more commented code, ineffective
>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>> >> still remains. But now I'm happy with the design.
>>>>> >
>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>> > speaking.)
>>>>> >
>>>>> > Thanks,
>>>>> > xiaofeng
>>>>> >
>>>>> >> Sorry for being slow.
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>> >>> <al...@gmail.com> wrote:
>>>>> >>>> Xiao-Feng,
>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>> where
>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>> >>>> constructed dynamically depending on application.
>>>>> >>>
>>>>> >>> Alexei, here is the patch code I found:
>>>>> >>>
>>>>> >>> line 245:
>>>>> >>> +            // Find which jar exports this package
>>>>> >>> +            if (pkgName != NULL) {
>>>>> >>> +                char *boot_class_path =
>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>> >>> +                char *pendingClassPath = NULL;
>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>> >>> +                    if (pdest != NULL) {
>>>>> >>> +                        pendingClassPath =
>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>> >>> +                                               +
>>>>> strlen((*it).first->bytes) + 1);
>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>> >>> +                        // Open this found jar, and read all classes
>>>>> >>> contained in this jar
>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>> tmp_pool);
>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>> >>> as it has been parsed
>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>> >>> +                    } else {
>>>>> >>>
>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>> >>>
>>>>> >>> Thanks,
>>>>> >>> xiaofeng
>>>>> >>>
>>>>> >>>> Thanks.
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> >>>>
>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>> >>>>> <al...@gmail.com> wrote:
>>>>> >>>>> > Aleksey,
>>>>> >>>>> > I like your conclusion.
>>>>> >>>>> >
>>>>> >>>>> > Wenlong,
>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>> startup
>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>> I
>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>> "no"
>>>>> >>>>> > to this question.
>>>>> >>>>>
>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>> expected.
>>>>> >>>>>
>>>>> >>>>> Thanks,
>>>>> >>>>> xiaofeng
>>>>> >>>>>
>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>> are
>>>>> >>>>> > improved by this patch.
>>>>> >>>>> > Thanks!
>>>>> >>>>>
>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>> >>>>> >> Ok, here's the catch.
>>>>> >>>>> >>
>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>> enumerates
>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>> is
>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>> the
>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>> new
>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>> >>>>> >>
>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>> and
>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>> in
>>>>> >>>>> >> case the mapping is wrong?
>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>> split
>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>> with
>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>> could
>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>> be
>>>>> >>>>> >> used by them then ;)
>>>>> >>>>> >>
>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>> which
>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>> once.
>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>> from
>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>> DRLVM
>>>>> >>>>> >> build process?
>>>>> >>>>> >>
>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>> >>>>> >>
>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>> we
>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>> >>>>> >>
>>>>> >>>>> >> Thanks,
>>>>> >>>>> >> Aleksey.
>>>>> >>>>> >>
>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>> wrote:
>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>> the
>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>> change
>>>>> >>>>> >>> the manifest as well?
>>>>> >>>>> >>>
>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>> modulelibrarymapping
>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>> module. It
>>>>> >>>>> >>> doesn't relate to each class.
>>>>> >>>>> >>>
>>>>> >>>>> >>> thx,
>>>>> >>>>> >>> Wenlong
>>>>> >>>>> >>>
>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>> sure how
>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>> >>>>> >>>>
>>>>> >>>>> >>>> WBR,
>>>>> >>>>> >>>>    Pavel.
>>>>> >>>>> >>>>
>>>>> >>>>> >>>>
>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>> manually?
>>>>> >>>>> Just
>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>> thx,
>>>>> >>>>> >>>>> Wenlong
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>> >>>>> file...
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>> WBR,
>>>>> >>>>> >>>>>>    Pavel.
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>> modifying the
>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>> update
>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>> Wenlong,
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>> adding new
>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> WBR,
>>>>> >>>>> >>>>>>>>    Pavel.
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>> >>>>> bootclasspath.properties
>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>> >>>>> specific
>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>> time. So
>>>>> >>>>> that
>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>> >>>>> potential
>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> thx,
>>>>> >>>>> >>>>>>>>> Wenlong
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>> need
>>>>> >>>>> to
>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>> gain in
>>>>> >>>>> Linux
>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>> double
>>>>> >>>>> check the
>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>> tests
>>>>> >>>>> from
>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>> so
>>>>> >>>>> whether
>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>> much
>>>>> >>>>> mess the
>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>> From what
>>>>> >>>>> I
>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>> correct
>>>>> >>>>> mapping
>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>> spread
>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>> In
>>>>> >>>>> this
>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>> with
>>>>> >>>>> two
>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>> >>>>> Classlib,
>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>> it
>>>>> >>>>> might
>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>> >>>>> process?)
>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>
>>>>> >>>>> >>>
>>>>> >>>>> >>
>>>>> >>>>> >
>>>>> >>>>> >
>>>>> >>>>> >
>>>>> >>>>> > --
>>>>> >>>>> > С уважением,
>>>>> >>>>> > Алексей Федотов,
>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>> >>>>> >
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>> --
>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>> >>>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> --
>>>>> >>>> С уважением,
>>>>> >>>> Алексей Федотов,
>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>> >>>>
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>> --
>>>>> >>> http://xiao-feng.blogspot.com
>>>>> >>>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> --
>>>>> >> С уважением,
>>>>> >> Алексей Федотов,
>>>>> >> ЗАО «Телеком Экспресс»
>>>>> >>
>>>>> >
>>>>> >
>>>>> >
>>>>> > --
>>>>> > http://xiao-feng.blogspot.com
>>>>> >
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> С уважением,
>>>> Алексей Федотов,
>>>> ЗАО «Телеком Экспресс»
>>>>
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Pavel Pervov <pm...@gmail.com>.

Wenlong,

Please, see my comments in the JIRA.

WBR,
    Pavel.

On Wed, Jan 14, 2009 at 12:33 PM, Wenlong Li <we...@gmail.com> wrote:
> All,
>
> I submit a new patch for on-demand class loading and parsing. All
> codes are put in VM side, and the mapping info is automatically
> produced.
>
> Pls see https://issues.apache.org/jira/browse/HARMONY-6039
>
> Comments are welcome.
>
> Thx, Wenlong
> Managed Runtime Technology Center, Intel
>
> On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
>> All,
>> At this moment, I move all updates in classlib to VM side such that
>> there is no modularity issue. Next step is to produce the mapping
>> between module and library efficiently and accurately.
>>
>> Comments are welcome.
>>
>> Thx, Wenlong
>> Managed Runtime Technology Center, Intel
>>
>> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>>> Thx :)
>>>
>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>>> <al...@gmail.com> wrote:
>>>> Sure.
>>>>
>>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>>> splitting the given classpath into pieces. You already know the new piece
>>>> you add and may skip splitting step.
>>>>
>>>> 2. If I understand you code correctly, the case "pdest >
>>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>>> assrtion would speed up bug discovery.
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>>
>>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>>> modules on demand. If no class in swing.jar is not requested, then
>>>>> this module will not be loaded.
>>>>>
>>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>>> further.
>>>>>
>>>>> Thx, Wenlong
>>>>> Managed Runtime Technology Center, Intel
>>>>>
>>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>>> > <al...@gmail.com> wrote:
>>>>> >> Xiao Feng,
>>>>> >> Thank you for explaining.
>>>>> >>
>>>>> >> I get more minor comments on more commented code, ineffective
>>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>>> >> still remains. But now I'm happy with the design.
>>>>> >
>>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>>> > speaking.)
>>>>> >
>>>>> > Thanks,
>>>>> > xiaofeng
>>>>> >
>>>>> >> Sorry for being slow.
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>>> >>> <al...@gmail.com> wrote:
>>>>> >>>> Xiao-Feng,
>>>>> >>>> Continuing with the server example could you please give me a hint
>>>>> where
>>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>>> >>>> constructed dynamically depending on application.
>>>>> >>>
>>>>> >>> Alexei, here is the patch code I found:
>>>>> >>>
>>>>> >>> line 245:
>>>>> >>> +            // Find which jar exports this package
>>>>> >>> +            if (pkgName != NULL) {
>>>>> >>> +                char *boot_class_path =
>>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>>> >>> +                char *pendingClassPath = NULL;
>>>>> >>> +                apr_pool_t *tmp_pool;
>>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>>> >>> +                    if (pdest != NULL) {
>>>>> >>> +                        pendingClassPath =
>>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>>> >>> +                                               +
>>>>> strlen((*it).first->bytes) + 1);
>>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>>> >>> +                        // Open this found jar, and read all classes
>>>>> >>> contained in this jar
>>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>>> tmp_pool);
>>>>> >>> +                        // Erase the found jar from pending jar list
>>>>> >>> as it has been parsed
>>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>>> >>> +                        STD_FREE(pendingClassPath);
>>>>> >>> +                    } else {
>>>>> >>>
>>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>>> >>>
>>>>> >>> Thanks,
>>>>> >>> xiaofeng
>>>>> >>>
>>>>> >>>> Thanks.
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>>> wrote:
>>>>> >>>>
>>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>>> >>>>> <al...@gmail.com> wrote:
>>>>> >>>>> > Aleksey,
>>>>> >>>>> > I like your conclusion.
>>>>> >>>>> >
>>>>> >>>>> > Wenlong,
>>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>>> startup
>>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>>> I
>>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>>> "no"
>>>>> >>>>> > to this question.
>>>>> >>>>>
>>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>>> expected.
>>>>> >>>>>
>>>>> >>>>> Thanks,
>>>>> >>>>> xiaofeng
>>>>> >>>>>
>>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>>> are
>>>>> >>>>> > improved by this patch.
>>>>> >>>>> > Thanks!
>>>>> >>>>>
>>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>>> >>>>> > <al...@gmail.com> wrote:
>>>>> >>>>> >> Ok, here's the catch.
>>>>> >>>>> >>
>>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>>> enumerates
>>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>>> is
>>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>>> >>>>> >> should be updated when new JAR file arrives.
>>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>>> the
>>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>>> new
>>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>>> >>>>> >>
>>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>>> and
>>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>>> in
>>>>> >>>>> >> case the mapping is wrong?
>>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>>> split
>>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>>> with
>>>>> >>>>> >> different package layouts and their impact on performance. They
>>>>> could
>>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>>> be
>>>>> >>>>> >> used by them then ;)
>>>>> >>>>> >>
>>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>>> which
>>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>>> once.
>>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>>> from
>>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>>> DRLVM
>>>>> >>>>> >> build process?
>>>>> >>>>> >>
>>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>>> >>>>> >>
>>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>>> we
>>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>>> >>>>> >>
>>>>> >>>>> >> Thanks,
>>>>> >>>>> >> Aleksey.
>>>>> >>>>> >>
>>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>>> wrote:
>>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>>> the
>>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>>> change
>>>>> >>>>> >>> the manifest as well?
>>>>> >>>>> >>>
>>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>>> modulelibrarymapping
>>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>>> module. It
>>>>> >>>>> >>> doesn't relate to each class.
>>>>> >>>>> >>>
>>>>> >>>>> >>> thx,
>>>>> >>>>> >>> Wenlong
>>>>> >>>>> >>>
>>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>>> sure how
>>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>>> >>>>> >>>>
>>>>> >>>>> >>>> WBR,
>>>>> >>>>> >>>>    Pavel.
>>>>> >>>>> >>>>
>>>>> >>>>> >>>>
>>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>>> manually?
>>>>> >>>>> Just
>>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>> thx,
>>>>> >>>>> >>>>> Wenlong
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>>> >>>>> file...
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>> WBR,
>>>>> >>>>> >>>>>>    Pavel.
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>>> modifying the
>>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>>> update
>>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>>> pmcfirst@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>> Wenlong,
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>>> adding new
>>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> WBR,
>>>>> >>>>> >>>>>>>>    Pavel.
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>>> >>>>> bootclasspath.properties
>>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>>> >>>>> specific
>>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>>> time. So
>>>>> >>>>> that
>>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>>> >>>>> potential
>>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> thx,
>>>>> >>>>> >>>>>>>>> Wenlong
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>>> wenlong@gmail.com>
>>>>> >>>>> wrote:
>>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>>> need
>>>>> >>>>> to
>>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>>> gain in
>>>>> >>>>> Linux
>>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>>> double
>>>>> >>>>> check the
>>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>>> tests
>>>>> >>>>> from
>>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>>> so
>>>>> >>>>> whether
>>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>>> much
>>>>> >>>>> mess the
>>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>>> From what
>>>>> >>>>> I
>>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>>> correct
>>>>> >>>>> mapping
>>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>>> spread
>>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>>> In
>>>>> >>>>> this
>>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>>> with
>>>>> >>>>> two
>>>>> >>>>> >>>>>>>>>> serious modifications:
>>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>>> >>>>> Classlib,
>>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>>> it
>>>>> >>>>> might
>>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>>> >>>>> process?)
>>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>> Thanks,
>>>>> >>>>> >>>>>>>>>> Aleksey.
>>>>> >>>>> >>>>>>>>>>
>>>>> >>>>> >>>>>>>>>
>>>>> >>>>> >>>>>>>>
>>>>> >>>>> >>>>>>>
>>>>> >>>>> >>>>>>
>>>>> >>>>> >>>>>
>>>>> >>>>> >>>>
>>>>> >>>>> >>>
>>>>> >>>>> >>
>>>>> >>>>> >
>>>>> >>>>> >
>>>>> >>>>> >
>>>>> >>>>> > --
>>>>> >>>>> > С уважением,
>>>>> >>>>> > Алексей Федотов,
>>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>>> >>>>> >
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>>
>>>>> >>>>> --
>>>>> >>>>> http://xiao-feng.blogspot.com
>>>>> >>>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> --
>>>>> >>>> С уважением,
>>>>> >>>> Алексей Федотов,
>>>>> >>>> ЗАО «Телеком Экспресс»
>>>>> >>>>
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>> --
>>>>> >>> http://xiao-feng.blogspot.com
>>>>> >>>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> --
>>>>> >> С уважением,
>>>>> >> Алексей Федотов,
>>>>> >> ЗАО «Телеком Экспресс»
>>>>> >>
>>>>> >
>>>>> >
>>>>> >
>>>>> > --
>>>>> > http://xiao-feng.blogspot.com
>>>>> >
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> С уважением,
>>>> Алексей Федотов,
>>>> ЗАО «Телеком Экспресс»
>>>>
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Wenlong Li <we...@gmail.com>.

All,

I submit a new patch for on-demand class loading and parsing. All
codes are put in VM side, and the mapping info is automatically
produced.

Pls see https://issues.apache.org/jira/browse/HARMONY-6039

Comments are welcome.

Thx, Wenlong
Managed Runtime Technology Center, Intel

On Wed, Jan 7, 2009 at 12:08 PM, Wenlong Li <we...@gmail.com> wrote:
> All,
> At this moment, I move all updates in classlib to VM side such that
> there is no modularity issue. Next step is to produce the mapping
> between module and library efficiently and accurately.
>
> Comments are welcome.
>
> Thx, Wenlong
> Managed Runtime Technology Center, Intel
>
> On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
>> Thx :)
>>
>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>> <al...@gmail.com> wrote:
>>> Sure.
>>>
>>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>>> splitting the given classpath into pieces. You already know the new piece
>>> you add and may skip splitting step.
>>>
>>> 2. If I understand you code correctly, the case "pdest >
>>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>>> assrtion would speed up bug discovery.
>>>
>>> Thanks.
>>>
>>>
>>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>>
>>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>>> modules on demand. If no class in swing.jar is not requested, then
>>>> this module will not be loaded.
>>>>
>>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>>> them? I just reused some code in Harmony, and didn't optimize them
>>>> further.
>>>>
>>>> Thx, Wenlong
>>>> Managed Runtime Technology Center, Intel
>>>>
>>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>>> wrote:
>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>>> > <al...@gmail.com> wrote:
>>>> >> Xiao Feng,
>>>> >> Thank you for explaining.
>>>> >>
>>>> >> I get more minor comments on more commented code, ineffective
>>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>>> >> still remains. But now I'm happy with the design.
>>>> >
>>>> > Alexei, yes, I agree with your comments. These parts should be
>>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>>> > speaking.)
>>>> >
>>>> > Thanks,
>>>> > xiaofeng
>>>> >
>>>> >> Sorry for being slow.
>>>> >>
>>>> >>
>>>> >>
>>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>>> wrote:
>>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>>> >>> <al...@gmail.com> wrote:
>>>> >>>> Xiao-Feng,
>>>> >>>> Continuing with the server example could you please give me a hint
>>>> where
>>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>>> >>>> perception was that the list of what to load was hardcoded and was not
>>>> >>>> constructed dynamically depending on application.
>>>> >>>
>>>> >>> Alexei, here is the patch code I found:
>>>> >>>
>>>> >>> line 245:
>>>> >>> +            // Find which jar exports this package
>>>> >>> +            if (pkgName != NULL) {
>>>> >>> +                char *boot_class_path =
>>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>>> >>> +                char *pendingClassPath = NULL;
>>>> >>> +                apr_pool_t *tmp_pool;
>>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>>> >>> +                while (it != env->pending_jar_set.end()) {
>>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>>> >>> +                    if (pdest != NULL) {
>>>> >>> +                        pendingClassPath =
>>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>>> >>> +                                               +
>>>> strlen((*it).first->bytes) + 1);
>>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>>> >>> +                        // Open this found jar, and read all classes
>>>> >>> contained in this jar
>>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>>> tmp_pool);
>>>> >>> +                        // Erase the found jar from pending jar list
>>>> >>> as it has been parsed
>>>> >>> +                        env->pending_jar_set.erase(it++);
>>>> >>> +                        STD_FREE(pendingClassPath);
>>>> >>> +                    } else {
>>>> >>>
>>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>>> >>>
>>>> >>> Thanks,
>>>> >>> xiaofeng
>>>> >>>
>>>> >>>> Thanks.
>>>> >>>>
>>>> >>>>
>>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>>> wrote:
>>>> >>>>
>>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>>> >>>>> <al...@gmail.com> wrote:
>>>> >>>>> > Aleksey,
>>>> >>>>> > I like your conclusion.
>>>> >>>>> >
>>>> >>>>> > Wenlong,
>>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>>> startup
>>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>>> I
>>>> >>>>> > believe that the proper design of delayed loading should answer
>>>> "no"
>>>> >>>>> > to this question.
>>>> >>>>>
>>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>>> expected.
>>>> >>>>>
>>>> >>>>> Thanks,
>>>> >>>>> xiaofeng
>>>> >>>>>
>>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>>> are
>>>> >>>>> > improved by this patch.
>>>> >>>>> > Thanks!
>>>> >>>>>
>>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>>> >>>>> > <al...@gmail.com> wrote:
>>>> >>>>> >> Ok, here's the catch.
>>>> >>>>> >>
>>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>>> enumerates
>>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>>> is
>>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>>> >>>>> >> should be updated when new JAR file arrives.
>>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>>> the
>>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>>> new
>>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>>> >>>>> >>
>>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>>> and
>>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>>> in
>>>> >>>>> >> case the mapping is wrong?
>>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>>> split
>>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>>> with
>>>> >>>>> >> different package layouts and their impact on performance. They
>>>> could
>>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>>> be
>>>> >>>>> >> used by them then ;)
>>>> >>>>> >>
>>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>>> which
>>>> >>>>> >> could be done more than once, eventually would be done more than
>>>> once.
>>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>>> from
>>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>>> DRLVM
>>>> >>>>> >> build process?
>>>> >>>>> >>
>>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>>> >>>>> >>  b. treated in DRLVM classloader only.
>>>> >>>>> >>
>>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>>> we
>>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>>> >>>>> >>
>>>> >>>>> >> Thanks,
>>>> >>>>> >> Aleksey.
>>>> >>>>> >>
>>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>>> wrote:
>>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>>> the
>>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>>> change
>>>> >>>>> >>> the manifest as well?
>>>> >>>>> >>>
>>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>>> modulelibrarymapping
>>>> >>>>> >>> file only records package info provided by manfiest in each
>>>> module. It
>>>> >>>>> >>> doesn't relate to each class.
>>>> >>>>> >>>
>>>> >>>>> >>> thx,
>>>> >>>>> >>> Wenlong
>>>> >>>>> >>>
>>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>>> pmcfirst@gmail.com>
>>>> >>>>> wrote:
>>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>>> sure how
>>>> >>>>> >>>> it can be possible to track these changes manually.
>>>> >>>>> >>>>
>>>> >>>>> >>>> WBR,
>>>> >>>>> >>>>    Pavel.
>>>> >>>>> >>>>
>>>> >>>>> >>>>
>>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>>> >>>>> wrote:
>>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>>> manually?
>>>> >>>>> Just
>>>> >>>>> >>>>> curious to know the possible reason. :)
>>>> >>>>> >>>>>
>>>> >>>>> >>>>> thx,
>>>> >>>>> >>>>> Wenlong
>>>> >>>>> >>>>>
>>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>>> pmcfirst@gmail.com>
>>>> >>>>> wrote:
>>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>>> >>>>> file...
>>>> >>>>> >>>>>>
>>>> >>>>> >>>>>> WBR,
>>>> >>>>> >>>>>>    Pavel.
>>>> >>>>> >>>>>>
>>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>>> wenlong@gmail.com>
>>>> >>>>> wrote:
>>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>>> modifying the
>>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>>> >>>>> >>>>>>>
>>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>>> update
>>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>>> >>>>> >>>>>>>
>>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>>> >>>>> >>>>>>>
>>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>>> pmcfirst@gmail.com>
>>>> >>>>> wrote:
>>>> >>>>> >>>>>>>> Wenlong,
>>>> >>>>> >>>>>>>>
>>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>>> adding new
>>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>>> >>>>> >>>>>>>>
>>>> >>>>> >>>>>>>> WBR,
>>>> >>>>> >>>>>>>>    Pavel.
>>>> >>>>> >>>>>>>>
>>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>>> wenlong@gmail.com>
>>>> >>>>> wrote:
>>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>>> >>>>> >>>>>>>>>
>>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>>> >>>>> bootclasspath.properties
>>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>>> >>>>> >>>>>>>>>
>>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>>> >>>>> specific
>>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>>> time. So
>>>> >>>>> that
>>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>>> >>>>> potential
>>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>>> >>>>> >>>>>>>>>
>>>> >>>>> >>>>>>>>> thx,
>>>> >>>>> >>>>>>>>> Wenlong
>>>> >>>>> >>>>>>>>>
>>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>>> >>>>> >>>>>>>>>>
>>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>>> wenlong@gmail.com>
>>>> >>>>> wrote:
>>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>>> need
>>>> >>>>> to
>>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>>> gain in
>>>> >>>>> Linux
>>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>>> double
>>>> >>>>> check the
>>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>>> >>>>> >>>>>>>>>>
>>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>>> tests
>>>> >>>>> from
>>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>>> so
>>>> >>>>> whether
>>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>>> much
>>>> >>>>> mess the
>>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>>> From what
>>>> >>>>> I
>>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>>> correct
>>>> >>>>> mapping
>>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>>> spread
>>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>>> In
>>>> >>>>> this
>>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>>> >>>>> >>>>>>>>>>
>>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>>> with
>>>> >>>>> two
>>>> >>>>> >>>>>>>>>> serious modifications:
>>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>>> >>>>> Classlib,
>>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>>> it
>>>> >>>>> might
>>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>>> >>>>> process?)
>>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>>> >>>>> >>>>>>>>>>
>>>> >>>>> >>>>>>>>>> Thanks,
>>>> >>>>> >>>>>>>>>> Aleksey.
>>>> >>>>> >>>>>>>>>>
>>>> >>>>> >>>>>>>>>
>>>> >>>>> >>>>>>>>
>>>> >>>>> >>>>>>>
>>>> >>>>> >>>>>>
>>>> >>>>> >>>>>
>>>> >>>>> >>>>
>>>> >>>>> >>>
>>>> >>>>> >>
>>>> >>>>> >
>>>> >>>>> >
>>>> >>>>> >
>>>> >>>>> > --
>>>> >>>>> > С уважением,
>>>> >>>>> > Алексей Федотов,
>>>> >>>>> > ЗАО «Телеком Экспресс»
>>>> >>>>> >
>>>> >>>>>
>>>> >>>>>
>>>> >>>>>
>>>> >>>>> --
>>>> >>>>> http://xiao-feng.blogspot.com
>>>> >>>>>
>>>> >>>>
>>>> >>>>
>>>> >>>>
>>>> >>>> --
>>>> >>>> С уважением,
>>>> >>>> Алексей Федотов,
>>>> >>>> ЗАО «Телеком Экспресс»
>>>> >>>>
>>>> >>>
>>>> >>>
>>>> >>>
>>>> >>> --
>>>> >>> http://xiao-feng.blogspot.com
>>>> >>>
>>>> >>
>>>> >>
>>>> >>
>>>> >> --
>>>> >> С уважением,
>>>> >> Алексей Федотов,
>>>> >> ЗАО «Телеком Экспресс»
>>>> >>
>>>> >
>>>> >
>>>> >
>>>> > --
>>>> > http://xiao-feng.blogspot.com
>>>> >
>>>>
>>>
>>>
>>>
>>> --
>>> С уважением,
>>> Алексей Федотов,
>>> ЗАО «Телеком Экспресс»
>>>
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Wenlong Li <we...@gmail.com>.

All,
At this moment, I move all updates in classlib to VM side such that
there is no modularity issue. Next step is to produce the mapping
between module and library efficiently and accurately.

Comments are welcome.

Thx, Wenlong
Managed Runtime Technology Center, Intel

On Tue, Jan 6, 2009 at 11:08 PM, Wenlong Li <we...@gmail.com> wrote:
> Thx :)
>
> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
> <al...@gmail.com> wrote:
>> Sure.
>>
>> 1. If you dig into SetClasspathFromString, you will see that it starts from
>> splitting the given classpath into pieces. You already know the new piece
>> you add and may skip splitting step.
>>
>> 2. If I understand you code correctly, the case "pdest >
>> (*it).second->bytes" might be a subject of a negative assertion. Adding this
>> assrtion would speed up bug discovery.
>>
>> Thanks.
>>
>>
>> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>>
>>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>>> modules on demand. If no class in swing.jar is not requested, then
>>> this module will not be loaded.
>>>
>>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>>> (*it).second->bytes" are not efficient. Can you share more comments on
>>> them? I just reused some code in Harmony, and didn't optimize them
>>> further.
>>>
>>> Thx, Wenlong
>>> Managed Runtime Technology Center, Intel
>>>
>>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>>> wrote:
>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>>> > <al...@gmail.com> wrote:
>>> >> Xiao Feng,
>>> >> Thank you for explaining.
>>> >>
>>> >> I get more minor comments on more commented code, ineffective
>>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>>> >> still remains. But now I'm happy with the design.
>>> >
>>> > Alexei, yes, I agree with your comments. These parts should be
>>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>>> > speaking.)
>>> >
>>> > Thanks,
>>> > xiaofeng
>>> >
>>> >> Sorry for being slow.
>>> >>
>>> >>
>>> >>
>>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>>> wrote:
>>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>>> >>> <al...@gmail.com> wrote:
>>> >>>> Xiao-Feng,
>>> >>>> Continuing with the server example could you please give me a hint
>>> where
>>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>>> >>>> perception was that the list of what to load was hardcoded and was not
>>> >>>> constructed dynamically depending on application.
>>> >>>
>>> >>> Alexei, here is the patch code I found:
>>> >>>
>>> >>> line 245:
>>> >>> +            // Find which jar exports this package
>>> >>> +            if (pkgName != NULL) {
>>> >>> +                char *boot_class_path =
>>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>>> >>> +                char *pendingClassPath = NULL;
>>> >>> +                apr_pool_t *tmp_pool;
>>> >>> +                apr_pool_create(&tmp_pool, NULL);
>>> >>> +                while (it != env->pending_jar_set.end()) {
>>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>>> >>> +                    if (pdest != NULL) {
>>> >>> +                        pendingClassPath =
>>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>>> >>> +                                               +
>>> strlen((*it).first->bytes) + 1);
>>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>>> >>> +                        // Open this found jar, and read all classes
>>> >>> contained in this jar
>>> >>> +                        SetClasspathFromString(pendingClassPath,
>>> tmp_pool);
>>> >>> +                        // Erase the found jar from pending jar list
>>> >>> as it has been parsed
>>> >>> +                        env->pending_jar_set.erase(it++);
>>> >>> +                        STD_FREE(pendingClassPath);
>>> >>> +                    } else {
>>> >>>
>>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>>> >>> am not sure if this is what you were asking. (Btw, this is only my
>>> >>> understanding of his patch. I am not speaking for Wenlong.)
>>> >>>
>>> >>> Thanks,
>>> >>> xiaofeng
>>> >>>
>>> >>>> Thanks.
>>> >>>>
>>> >>>>
>>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>>> wrote:
>>> >>>>
>>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>>> >>>>> <al...@gmail.com> wrote:
>>> >>>>> > Aleksey,
>>> >>>>> > I like your conclusion.
>>> >>>>> >
>>> >>>>> > Wenlong,
>>> >>>>> > I'm trying to understand the real life value of the "abstract"
>>> startup
>>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>>> >>>>> > swing.jar for a server application? Do I understand that loading
>>> >>>>> > happens, though it happens later compared to VM without your patch?
>>> I
>>> >>>>> > believe that the proper design of delayed loading should answer
>>> "no"
>>> >>>>> > to this question.
>>> >>>>>
>>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>>> expected.
>>> >>>>>
>>> >>>>> Thanks,
>>> >>>>> xiaofeng
>>> >>>>>
>>> >>>>> > In other words, I appreciate if you describe which real use cases
>>> are
>>> >>>>> > improved by this patch.
>>> >>>>> > Thanks!
>>> >>>>>
>>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>>> >>>>> > <al...@gmail.com> wrote:
>>> >>>>> >> Ok, here's the catch.
>>> >>>>> >>
>>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>>> enumerates
>>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>>> is
>>> >>>>> >> really stable because modular decomposition of classlib is stable.
>>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>>> >>>>> >> should be updated when new JAR file arrives.
>>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>>> the
>>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>>> new
>>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>>> >>>>> >>
>>> >>>>> >> Automatic generation of this property file gives two advantages:
>>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>>> and
>>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>>> in
>>> >>>>> >> case the mapping is wrong?
>>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>>> split
>>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>>> >>>>> >> generation would enable them to quickly roll-in and experiment
>>> with
>>> >>>>> >> different package layouts and their impact on performance. They
>>> could
>>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>>> be
>>> >>>>> >> used by them then ;)
>>> >>>>> >>
>>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>>> which
>>> >>>>> >> could be done more than once, eventually would be done more than
>>> once.
>>> >>>>> >> Hence it should be automated. You say that the file was generated
>>> from
>>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>>> DRLVM
>>> >>>>> >> build process?
>>> >>>>> >>
>>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>>> >>>>> >>  a. breaks the compatibility of classlib (you change
>>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>>> >>>>> >>  b. treated in DRLVM classloader only.
>>> >>>>> >>
>>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>>> we
>>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>>> >>>>> >> rather wait for some incubation on DRLVM side first.
>>> >>>>> >>
>>> >>>>> >> Thanks,
>>> >>>>> >> Aleksey.
>>> >>>>> >>
>>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>>> wrote:
>>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>>> the
>>> >>>>> >>> manifest file.  When class is added to a library, do we need
>>> change
>>> >>>>> >>> the manifest as well?
>>> >>>>> >>>
>>> >>>>> >>> btw, I guess there is a mis-understanding: my
>>> modulelibrarymapping
>>> >>>>> >>> file only records package info provided by manfiest in each
>>> module. It
>>> >>>>> >>> doesn't relate to each class.
>>> >>>>> >>>
>>> >>>>> >>> thx,
>>> >>>>> >>> Wenlong
>>> >>>>> >>>
>>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>>> pmcfirst@gmail.com>
>>> >>>>> wrote:
>>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>>> sure how
>>> >>>>> >>>> it can be possible to track these changes manually.
>>> >>>>> >>>>
>>> >>>>> >>>> WBR,
>>> >>>>> >>>>    Pavel.
>>> >>>>> >>>>
>>> >>>>> >>>>
>>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>>> >>>>> wrote:
>>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>>> >>>>> >>>>> specific file, why we could not make a vm specific file
>>> manually?
>>> >>>>> Just
>>> >>>>> >>>>> curious to know the possible reason. :)
>>> >>>>> >>>>>
>>> >>>>> >>>>> thx,
>>> >>>>> >>>>> Wenlong
>>> >>>>> >>>>>
>>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>>> pmcfirst@gmail.com>
>>> >>>>> wrote:
>>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>>> >>>>> file...
>>> >>>>> >>>>>>
>>> >>>>> >>>>>> WBR,
>>> >>>>> >>>>>>    Pavel.
>>> >>>>> >>>>>>
>>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>>> wenlong@gmail.com>
>>> >>>>> wrote:
>>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>>> modifying the
>>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>>> >>>>> >>>>>>>
>>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>>> update
>>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>>> >>>>> >>>>>>>
>>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>>> >>>>> >>>>>>>
>>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>>> pmcfirst@gmail.com>
>>> >>>>> wrote:
>>> >>>>> >>>>>>>> Wenlong,
>>> >>>>> >>>>>>>>
>>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>>> adding new
>>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>>> >>>>> >>>>>>>>
>>> >>>>> >>>>>>>> WBR,
>>> >>>>> >>>>>>>>    Pavel.
>>> >>>>> >>>>>>>>
>>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>>> wenlong@gmail.com>
>>> >>>>> wrote:
>>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>>> >>>>> >>>>>>>>>
>>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>>> >>>>> bootclasspath.properties
>>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>>> >>>>> >>>>>>>>>
>>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>>> >>>>> specific
>>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>>> time. So
>>> >>>>> that
>>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>>> >>>>> potential
>>> >>>>> >>>>>>>>> modularity and compatibility problem.
>>> >>>>> >>>>>>>>>
>>> >>>>> >>>>>>>>> thx,
>>> >>>>> >>>>>>>>> Wenlong
>>> >>>>> >>>>>>>>>
>>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>>> >>>>> >>>>>>>>>> Hi, Wenlong.
>>> >>>>> >>>>>>>>>>
>>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>>> wenlong@gmail.com>
>>> >>>>> wrote:
>>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>>> need
>>> >>>>> to
>>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>>> gain in
>>> >>>>> Linux
>>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>>> double
>>> >>>>> check the
>>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>>> >>>>> >>>>>>>>>>
>>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>>> tests
>>> >>>>> from
>>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>>> so
>>> >>>>> whether
>>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>>> much
>>> >>>>> mess the
>>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>>> From what
>>> >>>>> I
>>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>>> correct
>>> >>>>> mapping
>>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>>> spread
>>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>>> In
>>> >>>>> this
>>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>>> >>>>> >>>>>>>>>>
>>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>>> with
>>> >>>>> two
>>> >>>>> >>>>>>>>>> serious modifications:
>>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>>> >>>>> Classlib,
>>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>>> it
>>> >>>>> might
>>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>>> >>>>> process?)
>>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>>> >>>>> >>>>>>>>>>
>>> >>>>> >>>>>>>>>> Thanks,
>>> >>>>> >>>>>>>>>> Aleksey.
>>> >>>>> >>>>>>>>>>
>>> >>>>> >>>>>>>>>
>>> >>>>> >>>>>>>>
>>> >>>>> >>>>>>>
>>> >>>>> >>>>>>
>>> >>>>> >>>>>
>>> >>>>> >>>>
>>> >>>>> >>>
>>> >>>>> >>
>>> >>>>> >
>>> >>>>> >
>>> >>>>> >
>>> >>>>> > --
>>> >>>>> > С уважением,
>>> >>>>> > Алексей Федотов,
>>> >>>>> > ЗАО «Телеком Экспресс»
>>> >>>>> >
>>> >>>>>
>>> >>>>>
>>> >>>>>
>>> >>>>> --
>>> >>>>> http://xiao-feng.blogspot.com
>>> >>>>>
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>> --
>>> >>>> С уважением,
>>> >>>> Алексей Федотов,
>>> >>>> ЗАО «Телеком Экспресс»
>>> >>>>
>>> >>>
>>> >>>
>>> >>>
>>> >>> --
>>> >>> http://xiao-feng.blogspot.com
>>> >>>
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> С уважением,
>>> >> Алексей Федотов,
>>> >> ЗАО «Телеком Экспресс»
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > http://xiao-feng.blogspot.com
>>> >
>>>
>>
>>
>>
>> --
>> С уважением,
>> Алексей Федотов,
>> ЗАО «Телеком Экспресс»
>>
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Wenlong Li <we...@gmail.com>.

Thx :)

On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
<al...@gmail.com> wrote:
> Sure.
>
> 1. If you dig into SetClasspathFromString, you will see that it starts from
> splitting the given classpath into pieces. You already know the new piece
> you add and may skip splitting step.
>
> 2. If I understand you code correctly, the case "pdest >
> (*it).second->bytes" might be a subject of a negative assertion. Adding this
> assrtion would speed up bug discovery.
>
> Thanks.
>
>
> On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:
>
>> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
>> modules on demand. If no class in swing.jar is not requested, then
>> this module will not be loaded.
>>
>> btw, Alexei, you said "SetClasspathFromString" and "pdest >
>> (*it).second->bytes" are not efficient. Can you share more comments on
>> them? I just reused some code in Harmony, and didn't optimize them
>> further.
>>
>> Thx, Wenlong
>> Managed Runtime Technology Center, Intel
>>
>> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
>> wrote:
>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>> > <al...@gmail.com> wrote:
>> >> Xiao Feng,
>> >> Thank you for explaining.
>> >>
>> >> I get more minor comments on more commented code, ineffective
>> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
>> >> (*it).second->bytes. One major comment on crossing vm module boundary
>> >> still remains. But now I'm happy with the design.
>> >
>> > Alexei, yes, I agree with your comments. These parts should be
>> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
>> > speaking.)
>> >
>> > Thanks,
>> > xiaofeng
>> >
>> >> Sorry for being slow.
>> >>
>> >>
>> >>
>> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
>> wrote:
>> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
>> >>> <al...@gmail.com> wrote:
>> >>>> Xiao-Feng,
>> >>>> Continuing with the server example could you please give me a hint
>> where
>> >>>> decision to load swing.jar or not is taken in the patch? My initial
>> >>>> perception was that the list of what to load was hardcoded and was not
>> >>>> constructed dynamically depending on application.
>> >>>
>> >>> Alexei, here is the patch code I found:
>> >>>
>> >>> line 245:
>> >>> +            // Find which jar exports this package
>> >>> +            if (pkgName != NULL) {
>> >>> +                char *boot_class_path =
>> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
>> >>> +                char *pendingClassPath = NULL;
>> >>> +                apr_pool_t *tmp_pool;
>> >>> +                apr_pool_create(&tmp_pool, NULL);
>> >>> +                while (it != env->pending_jar_set.end()) {
>> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
>> >>> +                    if (pdest != NULL) {
>> >>> +                        pendingClassPath =
>> >>> (char*)STD_MALLOC(strlen(boot_class_path)
>> >>> +                                               +
>> strlen((*it).first->bytes) + 1);
>> >>> +                        strcpy(pendingClassPath, boot_class_path);
>> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
>> >>> +                        // Open this found jar, and read all classes
>> >>> contained in this jar
>> >>> +                        SetClasspathFromString(pendingClassPath,
>> tmp_pool);
>> >>> +                        // Erase the found jar from pending jar list
>> >>> as it has been parsed
>> >>> +                        env->pending_jar_set.erase(it++);
>> >>> +                        STD_FREE(pendingClassPath);
>> >>> +                    } else {
>> >>>
>> >>> It checks if a JAR has the requested package, then loads it if yes. I
>> >>> am not sure if this is what you were asking. (Btw, this is only my
>> >>> understanding of his patch. I am not speaking for Wenlong.)
>> >>>
>> >>> Thanks,
>> >>> xiaofeng
>> >>>
>> >>>> Thanks.
>> >>>>
>> >>>>
>> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
>> wrote:
>> >>>>
>> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>> >>>>> <al...@gmail.com> wrote:
>> >>>>> > Aleksey,
>> >>>>> > I like your conclusion.
>> >>>>> >
>> >>>>> > Wenlong,
>> >>>>> > I'm trying to understand the real life value of the "abstract"
>> startup
>> >>>>> > time metric you've suggested. Does Harmony with your patch load
>> >>>>> > swing.jar for a server application? Do I understand that loading
>> >>>>> > happens, though it happens later compared to VM without your patch?
>> I
>> >>>>> > believe that the proper design of delayed loading should answer
>> "no"
>> >>>>> > to this question.
>> >>>>>
>> >>>>> I checked the patch, and I found the answer is indeed "No" as you
>> expected.
>> >>>>>
>> >>>>> Thanks,
>> >>>>> xiaofeng
>> >>>>>
>> >>>>> > In other words, I appreciate if you describe which real use cases
>> are
>> >>>>> > improved by this patch.
>> >>>>> > Thanks!
>> >>>>>
>> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
>> >>>>> > <al...@gmail.com> wrote:
>> >>>>> >> Ok, here's the catch.
>> >>>>> >>
>> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
>> enumerates
>> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
>> is
>> >>>>> >> really stable because modular decomposition of classlib is stable.
>> >>>>> >> That's why nobody bothers with automatic generation of it: it only
>> >>>>> >> should be updated when new JAR file arrives.
>> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
>> the
>> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
>> new
>> >>>>> >> *package* is introduced. I'm not talking about java.* packages
>> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
>> >>>>> >>
>> >>>>> >> Automatic generation of this property file gives two advantages:
>> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
>> and
>> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
>> in
>> >>>>> >> case the mapping is wrong?
>> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
>> >>>>> >> modularity of Harmony classlib and eventually they might want to
>> split
>> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
>> >>>>> >> generation would enable them to quickly roll-in and experiment
>> with
>> >>>>> >> different package layouts and their impact on performance. They
>> could
>> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
>> be
>> >>>>> >> used by them then ;)
>> >>>>> >>
>> >>>>> >> That's merely a housekeeping procedure. I believe that anything
>> which
>> >>>>> >> could be done more than once, eventually would be done more than
>> once.
>> >>>>> >> Hence it should be automated. You say that the file was generated
>> from
>> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
>> DRLVM
>> >>>>> >> build process?
>> >>>>> >>
>> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
>> >>>>> >>  a. breaks the compatibility of classlib (you change
>> >>>>> >> bootclasspath.properties, right?) with other VMs.
>> >>>>> >>  b. treated in DRLVM classloader only.
>> >>>>> >>
>> >>>>> >> Of course eventually this feature might be used by others, but IMO
>> we
>> >>>>> >> should be careful about other guys who use the same classlib. I'd
>> >>>>> >> rather wait for some incubation on DRLVM side first.
>> >>>>> >>
>> >>>>> >> Thanks,
>> >>>>> >> Aleksey.
>> >>>>> >>
>> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
>> wrote:
>> >>>>> >>> I see. In fact, my file doesn't need track change at the class
>> >>>>> >>> granularity. Instead, it only needs know package info provided in
>> the
>> >>>>> >>> manifest file.  When class is added to a library, do we need
>> change
>> >>>>> >>> the manifest as well?
>> >>>>> >>>
>> >>>>> >>> btw, I guess there is a mis-understanding: my
>> modulelibrarymapping
>> >>>>> >>> file only records package info provided by manfiest in each
>> module. It
>> >>>>> >>> doesn't relate to each class.
>> >>>>> >>>
>> >>>>> >>> thx,
>> >>>>> >>> Wenlong
>> >>>>> >>>
>> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
>> pmcfirst@gmail.com>
>> >>>>> wrote:
>> >>>>> >>>> Classes are added to class library from time to time. I'm not
>> sure how
>> >>>>> >>>> it can be possible to track these changes manually.
>> >>>>> >>>>
>> >>>>> >>>> WBR,
>> >>>>> >>>>    Pavel.
>> >>>>> >>>>
>> >>>>> >>>>
>> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
>> >>>>> wrote:
>> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
>> >>>>> >>>>> specific file, why we could not make a vm specific file
>> manually?
>> >>>>> Just
>> >>>>> >>>>> curious to know the possible reason. :)
>> >>>>> >>>>>
>> >>>>> >>>>> thx,
>> >>>>> >>>>> Wenlong
>> >>>>> >>>>>
>> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
>> pmcfirst@gmail.com>
>> >>>>> wrote:
>> >>>>> >>>>>> If this would be VM-side automatically produced configuration
>> >>>>> file...
>> >>>>> >>>>>>
>> >>>>> >>>>>> WBR,
>> >>>>> >>>>>>    Pavel.
>> >>>>> >>>>>>
>> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
>> wenlong@gmail.com>
>> >>>>> wrote:
>> >>>>> >>>>>>> btw, because adding new module is rare case, manually
>> modifying the
>> >>>>> >>>>>>> bootclasspath.properties is not an issue?
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> If so, can I conclude adding another property file with same
>> update
>> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
>> >>>>> >>>>>>>
>> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
>> pmcfirst@gmail.com>
>> >>>>> wrote:
>> >>>>> >>>>>>>> Wenlong,
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
>> adding new
>> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>> WBR,
>> >>>>> >>>>>>>>    Pavel.
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
>> wenlong@gmail.com>
>> >>>>> wrote:
>> >>>>> >>>>>>>>> Thx for your advice. Alexey.
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> Here I have one question: do you know how the
>> >>>>> bootclasspath.properties
>> >>>>> >>>>>>>>> is maintained, manually or automatically?
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
>> >>>>> specific
>> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
>> time. So
>> >>>>> that
>> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
>> >>>>> potential
>> >>>>> >>>>>>>>> modularity and compatibility problem.
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> thx,
>> >>>>> >>>>>>>>> Wenlong
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
>> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
>> >>>>> >>>>>>>>>> Hi, Wenlong.
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
>> wenlong@gmail.com>
>> >>>>> wrote:
>> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
>> need
>> >>>>> to
>> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
>> gain in
>> >>>>> Linux
>> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
>> double
>> >>>>> check the
>> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
>> tests
>> >>>>> from
>> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
>> so
>> >>>>> whether
>> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
>> much
>> >>>>> mess the
>> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
>> From what
>> >>>>> I
>> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
>> correct
>> >>>>> mapping
>> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
>> spread
>> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
>> In
>> >>>>> this
>> >>>>> >>>>>>>>>> case I would rather stay without the patch.
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
>> with
>> >>>>> two
>> >>>>> >>>>>>>>>> serious modifications:
>> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
>> >>>>> Classlib,
>> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
>> it
>> >>>>> might
>> >>>>> >>>>>>>>>> break the compatibility with other VMs.
>> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
>> >>>>> process?)
>> >>>>> >>>>>>>>>> to free the burden for maintainers.
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>> Thanks,
>> >>>>> >>>>>>>>>> Aleksey.
>> >>>>> >>>>>>>>>>
>> >>>>> >>>>>>>>>
>> >>>>> >>>>>>>>
>> >>>>> >>>>>>>
>> >>>>> >>>>>>
>> >>>>> >>>>>
>> >>>>> >>>>
>> >>>>> >>>
>> >>>>> >>
>> >>>>> >
>> >>>>> >
>> >>>>> >
>> >>>>> > --
>> >>>>> > С уважением,
>> >>>>> > Алексей Федотов,
>> >>>>> > ЗАО «Телеком Экспресс»
>> >>>>> >
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> --
>> >>>>> http://xiao-feng.blogspot.com
>> >>>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> С уважением,
>> >>>> Алексей Федотов,
>> >>>> ЗАО «Телеком Экспресс»
>> >>>>
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> http://xiao-feng.blogspot.com
>> >>>
>> >>
>> >>
>> >>
>> >> --
>> >> С уважением,
>> >> Алексей Федотов,
>> >> ЗАО «Телеком Экспресс»
>> >>
>> >
>> >
>> >
>> > --
>> > http://xiao-feng.blogspot.com
>> >
>>
>
>
>
> --
> С уважением,
> Алексей Федотов,
> ЗАО «Телеком Экспресс»
>

Re: [VM] On-demand class library parsing is ready to commit

Posted by Alexei Fedotov <al...@gmail.com>.

Sure.

1. If you dig into SetClasspathFromString, you will see that it starts from
splitting the given classpath into pieces. You already know the new piece
you add and may skip splitting step.

2. If I understand you code correctly, the case "pdest >
(*it).second->bytes" might be a subject of a negative assertion. Adding this
assrtion would speed up bug discovery.

Thanks.


On Tue, Jan 6, 2009 at 5:09 AM, Wenlong Li <we...@gmail.com> wrote:

> Yes, Xiao-Feng's understanding is correct. The patch loads and parses
> modules on demand. If no class in swing.jar is not requested, then
> this module will not be loaded.
>
> btw, Alexei, you said "SetClasspathFromString" and "pdest >
> (*it).second->bytes" are not efficient. Can you share more comments on
> them? I just reused some code in Harmony, and didn't optimize them
> further.
>
> Thx, Wenlong
> Managed Runtime Technology Center, Intel
>
> On Fri, Dec 26, 2008 at 5:16 PM, Xiao-Feng Li <xi...@gmail.com>
> wrote:
> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
> > <al...@gmail.com> wrote:
> >> Xiao Feng,
> >> Thank you for explaining.
> >>
> >> I get more minor comments on more commented code, ineffective
> >> SetClasspathFromString usage, non-covered unexpected case when pdest >
> >> (*it).second->bytes. One major comment on crossing vm module boundary
> >> still remains. But now I'm happy with the design.
> >
> > Alexei, yes, I agree with your comments. These parts should be
> > improved. (Still, this is my personal opinion. :)  Let's wait Wenlong
> > speaking.)
> >
> > Thanks,
> > xiaofeng
> >
> >> Sorry for being slow.
> >>
> >>
> >>
> >> On Fri, Dec 26, 2008 at 11:40 AM, Xiao-Feng Li <xi...@gmail.com>
> wrote:
> >>> On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
> >>> <al...@gmail.com> wrote:
> >>>> Xiao-Feng,
> >>>> Continuing with the server example could you please give me a hint
> where
> >>>> decision to load swing.jar or not is taken in the patch? My initial
> >>>> perception was that the list of what to load was hardcoded and was not
> >>>> constructed dynamically depending on application.
> >>>
> >>> Alexei, here is the patch code I found:
> >>>
> >>> line 245:
> >>> +            // Find which jar exports this package
> >>> +            if (pkgName != NULL) {
> >>> +                char *boot_class_path =
> >>> env->JavaProperties()->get(VM_BOOT_CLASS_DIR);
> >>> +                char *pendingClassPath = NULL;
> >>> +                apr_pool_t *tmp_pool;
> >>> +                apr_pool_create(&tmp_pool, NULL);
> >>> +                while (it != env->pending_jar_set.end()) {
> >>> +                    pdest = strstr( (*it).second->bytes, pkgName );
> >>> +                    if (pdest != NULL) {
> >>> +                        pendingClassPath =
> >>> (char*)STD_MALLOC(strlen(boot_class_path)
> >>> +                                               +
> strlen((*it).first->bytes) + 1);
> >>> +                        strcpy(pendingClassPath, boot_class_path);
> >>> +                        strcat(pendingClassPath, (*it).first->bytes);
> >>> +                        // Open this found jar, and read all classes
> >>> contained in this jar
> >>> +                        SetClasspathFromString(pendingClassPath,
> tmp_pool);
> >>> +                        // Erase the found jar from pending jar list
> >>> as it has been parsed
> >>> +                        env->pending_jar_set.erase(it++);
> >>> +                        STD_FREE(pendingClassPath);
> >>> +                    } else {
> >>>
> >>> It checks if a JAR has the requested package, then loads it if yes. I
> >>> am not sure if this is what you were asking. (Btw, this is only my
> >>> understanding of his patch. I am not speaking for Wenlong.)
> >>>
> >>> Thanks,
> >>> xiaofeng
> >>>
> >>>> Thanks.
> >>>>
> >>>>
> >>>> On Fri, Dec 26, 2008 at 4:14 AM, Xiao-Feng Li <xi...@gmail.com>
> wrote:
> >>>>
> >>>>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
> >>>>> <al...@gmail.com> wrote:
> >>>>> > Aleksey,
> >>>>> > I like your conclusion.
> >>>>> >
> >>>>> > Wenlong,
> >>>>> > I'm trying to understand the real life value of the "abstract"
> startup
> >>>>> > time metric you've suggested. Does Harmony with your patch load
> >>>>> > swing.jar for a server application? Do I understand that loading
> >>>>> > happens, though it happens later compared to VM without your patch?
> I
> >>>>> > believe that the proper design of delayed loading should answer
> "no"
> >>>>> > to this question.
> >>>>>
> >>>>> I checked the patch, and I found the answer is indeed "No" as you
> expected.
> >>>>>
> >>>>> Thanks,
> >>>>> xiaofeng
> >>>>>
> >>>>> > In other words, I appreciate if you describe which real use cases
> are
> >>>>> > improved by this patch.
> >>>>> > Thanks!
> >>>>>
> >>>>> > On Thu, Dec 25, 2008 at 7:29 PM, Aleksey Shipilev
> >>>>> > <al...@gmail.com> wrote:
> >>>>> >> Ok, here's the catch.
> >>>>> >>
> >>>>> >> bootclasspath.properties is the SortedSet<JARfile>, which
> enumerates
> >>>>> >> the JARs available for bootclassloader. The set of such the JARs
> is
> >>>>> >> really stable because modular decomposition of classlib is stable.
> >>>>> >> That's why nobody bothers with automatic generation of it: it only
> >>>>> >> should be updated when new JAR file arrives.
> >>>>> >> Modulelibrarymapping.properties is different on this point, it's
> the
> >>>>> >> Map<PackageName,JARfile>, which should be updated each time the
> new
> >>>>> >> *package* is introduced. I'm not talking about java.* packages
> >>>>> >> (they're standardized), rather about org.apache.harmony.*.
> >>>>> >>
> >>>>> >> Automatic generation of this property file gives two advantages:
> >>>>> >>  1. Error-prone. Prevent yourself from hand-messing with mapping
> and
> >>>>> >> getting spurious ClassNotFoundException. BTW, what's the behaviour
> in
> >>>>> >> case the mapping is wrong?
> >>>>> >>  2. "Researchful". There're lot of guys around who enjoys the
> >>>>> >> modularity of Harmony classlib and eventually they might want to
> split
> >>>>> >> the packages even deeper, into smaller pieces. Then automatic
> >>>>> >> generation would enable them to quickly roll-in and experiment
> with
> >>>>> >> different package layouts and their impact on performance. They
> could
> >>>>> >> use ordinary bootclasspath.properties, but your feature wouldn't
> be
> >>>>> >> used by them then ;)
> >>>>> >>
> >>>>> >> That's merely a housekeeping procedure. I believe that anything
> which
> >>>>> >> could be done more than once, eventually would be done more than
> once.
> >>>>> >> Hence it should be automated. You say that the file was generated
> from
> >>>>> >> manifests of JARs, so is it hard to just tie the same tool into
> DRLVM
> >>>>> >> build process?
> >>>>> >>
> >>>>> >> As for DRLVM-specific, my opinion that this is because the patch:
> >>>>> >>  a. breaks the compatibility of classlib (you change
> >>>>> >> bootclasspath.properties, right?) with other VMs.
> >>>>> >>  b. treated in DRLVM classloader only.
> >>>>> >>
> >>>>> >> Of course eventually this feature might be used by others, but IMO
> we
> >>>>> >> should be careful about other guys who use the same classlib. I'd
> >>>>> >> rather wait for some incubation on DRLVM side first.
> >>>>> >>
> >>>>> >> Thanks,
> >>>>> >> Aleksey.
> >>>>> >>
> >>>>> >> On Thu, Dec 25, 2008 at 6:18 PM, Wenlong Li <we...@gmail.com>
> wrote:
> >>>>> >>> I see. In fact, my file doesn't need track change at the class
> >>>>> >>> granularity. Instead, it only needs know package info provided in
> the
> >>>>> >>> manifest file.  When class is added to a library, do we need
> change
> >>>>> >>> the manifest as well?
> >>>>> >>>
> >>>>> >>> btw, I guess there is a mis-understanding: my
> modulelibrarymapping
> >>>>> >>> file only records package info provided by manfiest in each
> module. It
> >>>>> >>> doesn't relate to each class.
> >>>>> >>>
> >>>>> >>> thx,
> >>>>> >>> Wenlong
> >>>>> >>>
> >>>>> >>> On Thu, Dec 25, 2008 at 10:55 PM, Pavel Pervov <
> pmcfirst@gmail.com>
> >>>>> wrote:
> >>>>> >>>> Classes are added to class library from time to time. I'm not
> sure how
> >>>>> >>>> it can be possible to track these changes manually.
> >>>>> >>>>
> >>>>> >>>> WBR,
> >>>>> >>>>    Pavel.
> >>>>> >>>>
> >>>>> >>>>
> >>>>> >>>> On Thu, Dec 25, 2008 at 5:09 PM, Wenlong Li <we...@gmail.com>
> >>>>> wrote:
> >>>>> >>>>> Sorry, one more question: bootclasspath.properties is classlib
> >>>>> >>>>> specific file, why we could not make a vm specific file
> manually?
> >>>>> Just
> >>>>> >>>>> curious to know the possible reason. :)
> >>>>> >>>>>
> >>>>> >>>>> thx,
> >>>>> >>>>> Wenlong
> >>>>> >>>>>
> >>>>> >>>>> On Thu, Dec 25, 2008 at 10:00 PM, Pavel Pervov <
> pmcfirst@gmail.com>
> >>>>> wrote:
> >>>>> >>>>>> If this would be VM-side automatically produced configuration
> >>>>> file...
> >>>>> >>>>>>
> >>>>> >>>>>> WBR,
> >>>>> >>>>>>    Pavel.
> >>>>> >>>>>>
> >>>>> >>>>>> On Thu, Dec 25, 2008 at 4:58 PM, Wenlong Li <
> wenlong@gmail.com>
> >>>>> wrote:
> >>>>> >>>>>>> btw, because adding new module is rare case, manually
> modifying the
> >>>>> >>>>>>> bootclasspath.properties is not an issue?
> >>>>> >>>>>>>
> >>>>> >>>>>>> If so, can I conclude adding another property file with same
> update
> >>>>> >>>>>>> frequency as bootclasspath would be fine as well?
> >>>>> >>>>>>>
> >>>>> >>>>>>> Pls kindly correct me if my understanding is wrong.
> >>>>> >>>>>>>
> >>>>> >>>>>>> On Thu, Dec 25, 2008 at 9:05 PM, Pavel Pervov <
> pmcfirst@gmail.com>
> >>>>> wrote:
> >>>>> >>>>>>>> Wenlong,
> >>>>> >>>>>>>>
> >>>>> >>>>>>>> Note, that bootclasspath.properties is only changed on
> adding new
> >>>>> >>>>>>>> module. This is pretty rare occasion, I believe.
> >>>>> >>>>>>>>
> >>>>> >>>>>>>> WBR,
> >>>>> >>>>>>>>    Pavel.
> >>>>> >>>>>>>>
> >>>>> >>>>>>>> On Thu, Dec 25, 2008 at 3:48 PM, Wenlong Li <
> wenlong@gmail.com>
> >>>>> wrote:
> >>>>> >>>>>>>>> Thx for your advice. Alexey.
> >>>>> >>>>>>>>>
> >>>>> >>>>>>>>> Here I have one question: do you know how the
> >>>>> bootclasspath.properties
> >>>>> >>>>>>>>> is maintained, manually or automatically?
> >>>>> >>>>>>>>>
> >>>>> >>>>>>>>> Another comment is I would like to treat the patch as DRLVM
> >>>>> specific
> >>>>> >>>>>>>>> optimization, e.g., it targets for improving VM creation
> time. So
> >>>>> that
> >>>>> >>>>>>>>> is possible to move all updates to DRLVM part to eliminate
> >>>>> potential
> >>>>> >>>>>>>>> modularity and compatibility problem.
> >>>>> >>>>>>>>>
> >>>>> >>>>>>>>> thx,
> >>>>> >>>>>>>>> Wenlong
> >>>>> >>>>>>>>>
> >>>>> >>>>>>>>> On Thu, Dec 25, 2008 at 5:32 PM, Aleksey Shipilev
> >>>>> >>>>>>>>> <al...@gmail.com> wrote:
> >>>>> >>>>>>>>>> Hi, Wenlong.
> >>>>> >>>>>>>>>>
> >>>>> >>>>>>>>>> On Thu, Dec 25, 2008 at 11:49 AM, Wenlong Li <
> wenlong@gmail.com>
> >>>>> wrote:
> >>>>> >>>>>>>>>>> btw, Alexey, Let's go back to discuss whether there is a
> need
> >>>>> to
> >>>>> >>>>>>>>>>> include this feature in Harmony, given 17% performance
> gain in
> >>>>> Linux
> >>>>> >>>>>>>>>>> when using your methodology. For windows test, I will
> double
> >>>>> check the
> >>>>> >>>>>>>>>>> backgroud process as you pointed out.
> >>>>> >>>>>>>>>>
> >>>>> >>>>>>>>>> My opinion was already expressed after I had finished the
> tests
> >>>>> from
> >>>>> >>>>>>>>>> my side: the boost can be achieved in specific conditions,
> so
> >>>>> whether
> >>>>> >>>>>>>>>> it's worth including into Harmony really depends on how
> much
> >>>>> mess the
> >>>>> >>>>>>>>>> patch would introduce besides the "performance boost".
> From what
> >>>>> I
> >>>>> >>>>>>>>>> see, the patch obliges the maintainer to maintain the
> correct
> >>>>> mapping
> >>>>> >>>>>>>>>> between jars and Java packages. This new feature is also
> spread
> >>>>> >>>>>>>>>> between Classlib and VM, but it seems like DRLVM specific.
> In
> >>>>> this
> >>>>> >>>>>>>>>> case I would rather stay without the patch.
> >>>>> >>>>>>>>>>
> >>>>> >>>>>>>>>> Personally (if I'll be committer) I would accept the patch
> with
> >>>>> two
> >>>>> >>>>>>>>>> serious modifications:
> >>>>> >>>>>>>>>>  1. Stay within DRLVM, do not introduce this feature into
> >>>>> Classlib,
> >>>>> >>>>>>>>>> get the thing tested and evolved on DRLVM side. Otherwise
> it
> >>>>> might
> >>>>> >>>>>>>>>> break the compatibility with other VMs.
> >>>>> >>>>>>>>>>  2. Make the mapping generated automatically (during build
> >>>>> process?)
> >>>>> >>>>>>>>>> to free the burden for maintainers.
> >>>>> >>>>>>>>>>
> >>>>> >>>>>>>>>> Thanks,
> >>>>> >>>>>>>>>> Aleksey.
> >>>>> >>>>>>>>>>
> >>>>> >>>>>>>>>
> >>>>> >>>>>>>>
> >>>>> >>>>>>>
> >>>>> >>>>>>
> >>>>> >>>>>
> >>>>> >>>>
> >>>>> >>>
> >>>>> >>
> >>>>> >
> >>>>> >
> >>>>> >
> >>>>> > --
> >>>>> > С уважением,
> >>>>> > Алексей Федотов,
> >>>>> > ЗАО «Телеком Экспресс»
> >>>>> >
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> http://xiao-feng.blogspot.com
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> С уважением,
> >>>> Алексей Федотов,
> >>>> ЗАО «Телеком Экспресс»
> >>>>
> >>>
> >>>
> >>>
> >>> --
> >>> http://xiao-feng.blogspot.com
> >>>
> >>
> >>
> >>
> >> --
> >> С уважением,
> >> Алексей Федотов,
> >> ЗАО «Телеком Экспресс»
> >>
> >
> >
> >
> > --
> > http://xiao-feng.blogspot.com
> >
>



-- 
С уважением,
Алексей Федотов,
ЗАО «Телеком Экспресс»