Hi Christoph,

I agree this might not be called super often, but the refactoring is
straightforward and I think it clarifies the intent of the code, so
seems like a reasonable enhancement to make.

I've already volunteered to sponsor another patch of yours, so I might
as well pick this up too and test them together.

/Claes

On 2020-05-23 09:53, Christoph Dreis wrote:
Hi,

I was looking through the code and found a relatively straight-forward 
optimization in Executable.getAllGenericParameterTypes (attached below).
The idea is to simply move some code around to avoid unnecessary allocations from 
getParameters() or array initializations when working with generics and no 
"real" parameter data is available.

I used the following benchmark:

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class MyBenchmark {

     @State(Scope.Benchmark)
     public static class ThreadState {
private Parameter parameter;

         public ThreadState() {
             try {
                 this.parameter = 
Test1.class.getDeclaredConstructor(Set.class).getParameters()[0];
             } catch (NoSuchMethodException e) {
                 // Do nothing
             }
         }

     }

     @Benchmark
     public AnnotatedType test(ThreadState threadState) {
         return threadState.parameter.getAnnotatedType();
     }

}

public class Test1 {
       public Test1(Set<?> names) {}
}

With the attached patch and the benchmark above I get the following results.

Old
Benchmark                                                  Mode  Cnt    Score   
  Error   Units
MyBenchmark.test                                       avgt   10  358,103 ±  
58,098   ns/op
MyBenchmark.test:·gc.alloc.rate.norm    avgt   10  248,021 ±   0,003    B/op

Patched
MyBenchmark.test                                       avgt   10  309,133 ±  
6,319   ns/op
MyBenchmark.test:·gc.alloc.rate.norm    avgt   10  200,016 ±  0,001    B/op


I don't think the particular method is called super often, but it seems to be used 
in ByteBuddy & JUnit as far as I can see.
In case you think this is worthwhile, I would appreciate it if someone can 
sponsor this patch.

Cheers,
Christoph

========= PATCH ==========
--- a/src/java.base/share/classes/java/lang/reflect/Executable.java     Wed May 
13 16:18:16 2020 +0200
+++ b/src/java.base/share/classes/java/lang/reflect/Executable.java     Sat May 
23 08:31:23 2020 +0200
@@ -307,12 +307,12 @@
              final boolean realParamData = hasRealParameterData();
              final Type[] genericParamTypes = getGenericParameterTypes();
              final Type[] nonGenericParamTypes = getParameterTypes();
-            final Type[] out = new Type[nonGenericParamTypes.length];
-            final Parameter[] params = getParameters();
-            int fromidx = 0;
              // If we have real parameter data, then we use the
              // synthetic and mandate flags to our advantage.
              if (realParamData) {
+                final Type[] out = new Type[nonGenericParamTypes.length];
+                final Parameter[] params = getParameters();
+                int fromidx = 0;
                  for (int i = 0; i < out.length; i++) {
                      final Parameter param = params[i];
                      if (param.isSynthetic() || param.isImplicit()) {
@@ -325,6 +325,7 @@
                          fromidx++;
                      }
                  }
+                return out;
              } else {
                  // Otherwise, use the non-generic parameter data.
                  // Without method parameter reflection data, we have
@@ -334,7 +335,6 @@
                  return genericParamTypes.length == 
nonGenericParamTypes.length ?
                      genericParamTypes : nonGenericParamTypes;
              }
-            return out;
          }


Reply via email to