On Sat, 7 Aug 2021 02:07:24 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> This reimplements core reflection with method handles. >> >> For `Constructor::newInstance` and `Method::invoke`, the new implementation >> uses `MethodHandle`. For `Field` accessor, the new implementation uses >> `VarHandle`. For the first few invocations of one of these reflective >> methods on a specific reflective object we invoke the corresponding method >> handle directly. After that we spin a dynamic bytecode stub defined in a >> hidden class which loads the target `MethodHandle` or `VarHandle` from its >> class data as a dynamically computed constant. Loading the method handle >> from a constant allows JIT to inline the method-handle invocation in order >> to achieve good performance. >> >> The VM's native reflection methods are needed during early startup, before >> the method-handle mechanism is initialized. That happens soon after >> System::initPhase1 and before System::initPhase2, after which we switch to >> using method handles exclusively. >> >> The core reflection and method handle implementation are updated to handle >> chained caller-sensitive method calls [1] properly. A caller-sensitive >> method can define with a caller-sensitive adapter method that will take an >> additional caller class parameter and the adapter method will be annotated >> with `@CallerSensitiveAdapter` for better auditing. See the detailed >> description from [2]. >> >> Ran tier1-tier8 tests. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8013527 >> [2] >> https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430 > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > minor test cleanup Hi Peter, I also like the previous simpler version as much as you do. However, both of these complications are done for the performance that I worked with @cl4es on these changes. Claes can explain and provide additional data w.r.t. the cost in the `catchException` and `asSpreader` he observes. A few observations from the performance analysis are the following: 1. The `catchException` combination is costly for setup. 2. Funneling the target method handle through `MethodHandles::asSpreader` costs another overhead. Dropping the `catchException` and specializing for a few arguments improve the cold startup for ~3x when measuring with `ReflectionColdstartBenchmark` you had [1]. Specializing to remove the asSpreader combinator also improves slightly on the throughtput. Claes can say more about the performance improvement without the spreader. 3. Splitting the virtual and static methods also improve on the throughtput. `isIllegalArgument` is not ideal but I found it acceptable as the solution for this work because it's on the exception case and depends on the implementation of core reflection and method handle exception when types mismatch. We also have tests to validate these cases. I think we should look into longer-term solution in reducing the overhead of the combinators that will benefit not only the core reflection code but also the clients of method handles. [1] http://cr.openjdk.java.net/~plevart/jdk-dev/6824466_MHReflectionAccessors/ReflectionColdstartBenchmark.java ------------- PR: https://git.openjdk.java.net/jdk/pull/5027
