harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexei Fedotov" <alexei.fedo...@gmail.com>
Subject Re: [VM] On-demand class library parsing is ready to commit
Date Thu, 15 Jan 2009 11:11:00 GMT
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
<alexei.fedotov@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 <wenlong@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 <wenlong@gmail.com> wrote:
>> > Pavel,
>> >
>> > Pls see my comments in the JIRA.
>> >
>> > thx, Wenlong
>> >
>> > On Wed, Jan 14, 2009 at 8:44 PM, Pavel Pervov <pmcfirst@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 <wenlong@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 <wenlong@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 <wenlong@gmail.com>
wrote:
>> >>>>> Thx :)
>> >>>>>
>> >>>>> On Tue, Jan 6, 2009 at 10:35 PM, Alexei Fedotov
>> >>>>> <alexei.fedotov@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 <wenlong@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 <xiaofeng.li@gmail.com>
>> >>>>>>> wrote:
>> >>>>>>> > On Fri, Dec 26, 2008 at 5:08 PM, Alexei Fedotov
>> >>>>>>> > <alexei.fedotov@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 <xiaofeng.li@gmail.com>
>> >>>>>>> wrote:
>> >>>>>>> >>> 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
>> >>>>>>> >>>
>> >>>>>>> >>
>> >>>>>>> >>
>> >>>>>>> >>
>> >>>>>>> >> --
>> >>>>>>> >> С уважением,
>> >>>>>>> >> Алексей Федотов,
>> >>>>>>> >> ЗАО «Телеком Экспресс»
>> >>>>>>> >>
>> >>>>>>> >
>> >>>>>>> >
>> >>>>>>> >
>> >>>>>>> > --
>> >>>>>>> > http://xiao-feng.blogspot.com
>> >>>>>>> >
>> >>>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>> --
>> >>>>>> С уважением,
>> >>>>>> Алексей Федотов,
>> >>>>>> ЗАО «Телеком Экспресс»
>> >>>>>>
>> >>>>>
>> >>>>
>> >>>
>> >>
>> >
>
>
>
> --
> С уважением,
> Алексей Федотов,
> ЗАО «Телеком Экспресс»
>



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