harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xiao-Feng Li" <xiaofeng...@gmail.com>
Subject Re: [VM] On-demand class library parsing is ready to commit
Date Fri, 26 Dec 2008 08:40:29 GMT
On Fri, Dec 26, 2008 at 4:03 PM, Alexei Fedotov
<alexei.fedotov@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 <xiaofeng.li@gmail.com> wrote:
>
>> On Fri, Dec 26, 2008 at 12:49 AM, Alexei Fedotov
>> <alexei.fedotov@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
>> > <aleksey.shipilev@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 <wenlong@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 <wenlong@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
>> >>>>>>>>> <aleksey.shipilev@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
Mime
View raw message