RFR: 8291065: Creating a VarHandle for a static field triggers class initialization [v17]

[Mandy Chung]

On Wed, 28 Jun 2023 01:34:30 GMT, Chen Liang <liach at openjdk.org> wrote:

>> This patch implements lazy initialization for VarHandle working on static fields. It has a good initial call performance.
>> 
>> We introduce a new internal API, `target()` to unpack a lazy VarHandle in VH implementation methods. If called via MethodHandle, a barrier is added in the MethodHandle instead.
>> 
>> The new test ensures the correctness of Lazy VH for both direct and indirect invocation; the performance of MethodHandle version of lazy VH is not yet tested.
>> 
>> 
>> Benchmark                                            Mode  Cnt    Score    Error  Units
>> LazyStaticColdStart.methodHandleCreateEager            ss   10   41.490 ± 12.331  us/op
>> LazyStaticColdStart.methodHandleCreateLazy             ss   10   21.810 ± 16.964  us/op
>> LazyStaticColdStart.methodHandleInitializeCallEager    ss   10   57.860 ± 13.738  us/op
>> LazyStaticColdStart.methodHandleInitializeCallLazy     ss   10   93.300 ± 18.858  us/op
>> LazyStaticColdStart.varHandleCreateEager               ss   10   39.860 ±  9.362  us/op
>> LazyStaticColdStart.varHandleCreateLazy                ss   10   17.630 ±  1.111  us/op
>> LazyStaticColdStart.varHandleInitializeCallEager       ss   10  123.170 ± 62.468  us/op
>> LazyStaticColdStart.varHandleInitializeCallLazy        ss   10  105.390 ± 41.815  us/op
>
> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Comment update from mandy
>   
>   Co-authored-by: Mandy Chung <mandy.chung at oracle.com>

Looks good in general.  Some minor comments.

I suggest to create a CSR to document the behavioral change to match the specification.

src/java.base/share/classes/java/lang/invoke/LazyInitializingVarHandle.java line 129:

> 127: 
> 128:         try {
> 129:             return MH_ensureInitialized = IMPL_LOOKUP.findVirtual(LazyInitializingVarHandle.class,

Suggestion:

            return MH_ensureInitialized = MethodHandles.lookup().findVirtual(LazyInitializingVarHandle.class,


No need to use the superpower lookup.

src/java.base/share/classes/java/lang/invoke/VarHandle.java line 2086:

> 2084:      * be linked directly. If a VarHandle is indirect, it must override
> 2085:      * {@link #isAccessModeSupported} and {@link #getMethodHandleUncached}
> 2086:      * which access MemberNames..

Suggestion:

     * which access MemberNames.

test/jdk/java/lang/invoke/VarHandles/LazyInitializingTest.java line 30:

> 28:  *          initialization mechanism..
> 29:  * @run junit LazyInitializingTest
> 30:  * @run junit/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT=true LazyInitializingTest

I wonder if this new test should run with all combination of 
`java.lang.invoke.VarHandle.VAR_HANDLE_GUARDS` and `java.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT` as done in other `VarHandleTestXXX`.

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

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13821#pullrequestreview-1504321038
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1245899498
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1245901726
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1245905901