Loading classes with many methods is very expensive

[Martin Buchholz]

I'm having trouble keeping up with this thread I started, but I did update
the test/benchmark
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/Class.getMethods-benchmark/

I added a test *LoadAllClassesAndMethods *that does what its name says.
Although originally written as just a benchmark, it also checks "en
passant" that all classes in all JRE jar files can be loaded.
Interestingly, openjdk9 passes this test, but proprietary Oracle JDK fails
it.  I think this is a useful packaging-level test of the entire JDK as
well.

java.lang.NoClassDefFoundError: Failed to load
jfxrt.jar(com/sun/deploy/uitoolkit/impl/fx/FXApplet2Adapter$1.class)
at
LoadAllClassesAndMethods.loadAllExtensionClasses(LoadAllClassesAndMethods.java:178)
at
LoadAllClassesAndMethods.methodsByClass(LoadAllClassesAndMethods.java:198)
at LoadAllClassesAndMethods.main(LoadAllClassesAndMethods.java:65)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at com.sun.javatest.regtest.MainWrapper$MainThread.run(MainWrapper.java:94)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NoClassDefFoundError:
com/sun/deploy/uitoolkit/Applet2Adapter
at java.lang.ClassLoader.defineClass1(Native Method)

Jeremy is independently working on yet another performance problem exposed
by *LoadAllClassesAndMethods*


On Mon, Oct 27, 2014 at 8:48 AM, Aleksey Shipilev <
aleksey.shipilev at oracle.com> wrote:

> Hi Peter,
>
> On 10/27/2014 03:45 PM, Peter Levart wrote:
> > I merged the functionality of the method-signature-key with the
> > linked-list node, holding just a reference to Method object and a link
> > to 'next' node. I think this is the most compact temporary datastructure
> > representation for that purpose that leverages standard LinkedHashMap.
>
> I see. I wonder when somebody come with a stress test using multiple
> interfaces with the same signature method :) Seems very unlikely.
>
>
> >>   * Formatting and logic in MethodList.consolidateWith gives me chills.
> I
> >> think at very least introducing variables for m.getDeclaringClass() and
> >> ml.m.getDeclaringClass() improve readability. Also, moving the comments
> >> at the end of the line should improve readability and better highlight
> >> the boolean expression you are spelling out. Below is the
> >> straight-forward rewrite:
> >> ...
> >
> > Excellent. I'll take your formatting if you don't mind ;-)
>
> Sure. Check if I had captured it right first. I think we need to be more
> strict with parentheses to avoid precedence overlooks:
>   (A || B && C) should better be ((A || B) && C) or (A || (B && C))
>
>
> >>    * Should we use just methods.put() here?
> >>        2851             MethodList ml0 = methods.putIfAbsent(ml, ml);
> >
> > It really doesn't matter as the exception is thrown if (ml0 != null) and
> > LinkedHashMap is forgotten...
> >
> > if (ml0 == null), put and putIfAbsent are equivalent. I used putIfAbsent
> > to make the 3 loops (declared methods, superclass methods,
> > (super)interface methods) more uniform.
>
> Okay.
>
>
> >>
> >>    * I wonder if the code would be simpler if we push the
> Map<MethodList,
> >> MethodList> maintenance to some internal utility class? That would
> >> probably simplify the loops in privateGetPublicMethods?
> >
> > I thought MethodList.add()/consolidateWith() were close to those utility
> > methods. I could extract the logic in each of 3 loops to 3 void methods
> > taking 'methods' Map and a Method instance, but I don't want to pollute
> > the method namespace in j.l.Class any more than necessary and making a
> > special inner class for that is an overkill. Previously the holder for
> > such methods was MethodArray, but it's gone now. I could subclass
> > LinkedHashMap if I wanted to avoid an unnecessary indirection, but
> > that's not a good practice. It's not that bad as it is - 38 lines of
> > code for 3 loops with comments and spacing. I could improve comments
> > though...
>
> Yes. I had to read the entire thing a couple of times to understand how
> this works. Containing most of the logic into MethodList seems like a
> way to improve this. Not sure though.
>
>
> Thanks,
> -Aleksey.
>
>
>