neoremind edited a comment on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation URL: https://github.com/apache/calcite/pull/1875#issuecomment-605091418 @vlsi I have add benchmark to `ubenchmark` module. As @hsyuan suggested, I make global caching by default. In order to benchmark against current instance level caching using `Reflection` and `MethodHandle`, especially for `MethodHandle`, classes like `ReflectUtil` and `ReflectiveVisitDispatcher` need to be updated much, so I make that change in my [fork branch](https://github.com/neoremind/calcite/blob/refvisit_benchmark/ubenchmark/src/jmh/java/org/apache/calcite/benchmarks/ReflectiveVisitDispatcherTest.java). The test result is a little out of expectation. ``` Benchmark Mode Cnt Score Error Units ReflectiveVisitDispatcherTest.testReflectiveVisitorDispatcherInvokeGlobalCaching avgt 3 143.483 ± 32.596 ns/op ReflectiveVisitDispatcherTest.testReflectiveVisitorDispatcherInvokeInstanceCaching avgt 3 290.380 ± 857.199 ns/op ReflectiveVisitDispatcherTest.testReflectiveVisitorDispatcherInvokeInstanceCachingUseMethodHandle avgt 3 2487.406 ± 1083.180 ns/op ``` PS. I refine test to return generated result from the benchmark methods, update benchmark parameters like fork and iteration times. MethodHandle is way slow than reflection, the below part is performance killer. If I make candidateMethod to be feteched from ThreadLocal which I initialized explicitly, the average time can drop down to about 208 ns/op. ``` MethodType mt = MethodType.methodType(returnType, paramTypes); MethodHandle candidateMethod = MethodHandles.lookup().findVirtual(visitorClass, visitMethodName, mt); ``` I will look into the reason if I have more bandwidth. My benchmark result is pretty much the same with [JavaReflectionButMuchFaster.html](http://www.jboss.org/optaplanner/blog/2018/01/09/JavaReflectionButMuchFaster.html). ``` DirectAccess avgt 60 2.590 ± 0.014 ns/op Reflection avgt 60 5.275 ± 0.053 ns/op // 104% slower MethodHandle avgt 60 6.100 ± 0.079 ns/op // 136% slower ``` We need the reflection agility to be able to find methods at runtime, `MethodHanlde` seems not a good choice.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services