RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v8]

[Peter Levart]

On Wed, 1 Sep 2021 01:05:32 GMT, Mandy Chung <mchung at 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 cleanup and more test case.

Hi Claes,

> I'm not sure how to assess how minor the "Var" case really is. I wouldn't be surprised if reflection-heavy frameworks hold `Method`s etc in some collection, which means they wouldn't be rooted in a way that allows the JIT to fold through. Thus leaning only on MH customization could be adding some performance risks. Off-list Vladimir Ivanov suggested the "Var" micros have some issues with inlining that make them look worse than they should, though.

Frameworks that keep Method(s) etc in some collection don't fall into the "Var" category of MH tests presented here, but into the "Poly" category. The test that uses Jackson serialization is one such example. The "Var" class of tests exhibit a use-case where there is a single instance of Method (or Field) object used in a particular call-site (i.e. the Method::invoke invocation in bytecode) but such Method object can't be proved by JIT to be constant (it is not read from static final or @Stable field). I doubt that such use-cases exist in practice. Mostly they would amount to cases that were meant to be "Constant" but are not because the user forgot to use "final" modifier on "static" field...

If you look at "Poly" results, spinning MHInvoker/VHInvoker classes for each instance of Method/Field does not help at all.

I would try to optimize the "Poly" case further if it was possible. The simplified Method/MethodHandle is practically equivalent in final "Poly" performance with the generated MethodAccessor, but the Field/VarHandle or Field/MethodHandle lags behind Unsafe-based accessor in "Poly" performance. Nothing can beat Unsafe when access to fields is concerned. It doesn't matter where the offset and base are read-from, the access is super-fast. I wish MethodHandles obtained by unreflectGetter/unreflectSetter could be less sensitive to where they are read from (constant vs. not-constant) and optimize better in non-constant case. I think we should search in this direction... to make MethodHandles obtained by unreflectGetter/unreflectSetter more optimal in non-constant case. If Unsafe can be better, why not MethodHandles?

> 
> On balance I think removing class-spinning might mean a better overall story w.r.t. footprint and maintainability. Had we started this review using a patch that looked more like what Peter is suggestion and someone had suggested we spin classes to get a performance edge in a subset of cases I think we'd not be looking favorably at that and instead tried reaching for narrowing those performance gaps via other less intrusive means. So I think I'm in favor of simplifying and filing a follow-up to try and win back some of the performance we might be losing on corner-case micros without the custom class spinning.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5027