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

Hi Mandy,

So, here's the mentioned approach:
https://github.com/plevart/jdk/commit/1a31508b7d73bd9fa40aae845c06b24c3cbcfd16
This is a commit above your changes that I have picked from your branch and 
squashed as a single commit above current master:
https://github.com/plevart/jdk/commit/a2e4a1f9dad9e6951949ce82af54639ed2da9ddf
I think you could chery-pick the former commit into your branch if you wanted 
to.

What I have done is mainly removing all code around MHInvoker and VHInvoker and 
calling the target MethodHandle/VarHandle directly in the XxxAccessors. I also 
made sure that the path of references going from Method/Constructor/Field via 
MethodAccessor(s)/ConstructorAccessor(s)/FieldAccessor(s) and to 
MethodHandle/VarHandle is marked with @Stable. So if the 
Method/Constructor/Field is found to be constant by JIT, so is its associated 
MethodHandle/VarHandle. I also "fixed" existing 
MethodAccessor(s)/ConstructorAccessor(s)/FieldAccessor(s) so that they are safe 
to be "published" via data-race as the fields holding them in 
Method/Constructor are no longer volatile (in Field they never have been 
volatile). In Field I had to make two distinct call-sites for fieldAccessor vs. 
overrideFieldAccessor in order for JIT to propagate @Stable-ness down the chain.

I then ran the following JMH benchmarks (not included in my variant of patch 
yet) on jdk16, mandy's variant and peter's variant:
https://gist.github.com/plevart/b9c62c8cac9784e2c6d5f51a6386fa84

The results can be visualized via the following links:
jdk16 vs. mandy: 
https://jmh.morethan.io/?gists=9219d79a09a805eb39607830270d1513,80203847700b9ab8baeb346a02efc804
jdk16 vs. peter: 
https://jmh.morethan.io/?gists=9219d79a09a805eb39607830270d1513,b11842fbd48ebbd9234251fded50d52e
mandy vs. peter: 
https://jmh.morethan.io/?gists=80203847700b9ab8baeb346a02efc804,b11842fbd48ebbd9234251fded50d52e

Comparing mandy vs. peter, there are pros and cons. If Method/Field instance is 
found to be constant by JIT, peter's variant is a little better (16...36%). It 
Method/Field is not constant as found by JIT, but still a single instance of 
Method/Field is invoked in a particular call-site, peter's variant is worse 
(22...48%). I added another variant called "Poly" (for static methods/fields 
only, but I think results would be similar for instance too). In this variant a 
single call-site operates with distinct instances of Method/Field and peter's 
variant is a little better there (3...11%). The cold-start is a little better 
for peter's variant too (5%).
I think the "Poly" variants are an important representative of real-world 
usage. Every usage happens in the real world (Const, Var, Poly), but most 
applications/frameworks that use reflection and where performance matters (such 
as JPA, various serialization frameworks, etc.) are generic algorithm(s) that 
abstract over various runtime types, accessing methods/fields/constructors via 
call-sites that deal with multiple instances of Mehod/Field/Constructor each.
Top speed is possible with peter's variant and even better than with mandys's 
variant when each call-site is dealing with single instance of 
Mehod/Field/Constructor but user has to make such instance a constant for JIT 
(by accessing it from static final or @Stable field)
I don't know how realistic for real world the "Var" class of invocations is 
where mandy's variant performs equivalently to "Const" class. Are there real 
world apps where call-sites deal with single instances of 
Mehod/Field/Constructor but such instances are not constant for JIT?

Comparing jdk16 vs. peter's variant we see peter's variant performs better in 
"Const" class (11...67%) and worse for "Var" class (11...46%). For "Poly" 
variant jdk16 is still better (44%) when accessing fields (nothing can beat 
Unsafe) but when accessing methods peter's variant is just 3% short of jdk16.

So what do you think? Does increased speed of "Var" class of usages (single 
Mehod/Field/Constructor instance per call-site but not constant) outweight 
complexity and total work added by MHInvoker/VHInvoker code generation and 
class loading?

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

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

Reply via email to