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 < [email protected]> 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. > > >
