On Wed, 17 May 2023 17:20:37 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Also fixed the bug with NPE in `IndirectVarHandle::isAccessModeSupported`.
>>
>> A few implementation-detail methods in VarHandle are now documented with the
>> implied constraints to avoid subtle problems in the future. Changed
>> `IndirectVarHandle` to call `asDirect` lazily to accomodate the lazy
>> VarHandle changes. Also changed VarHandleBaseTest to report the whole
>> incorrect type of exception caught than swallow it and leaving only a
>> message.
>>
>> Current problems:
>> - [ ] The lazy var handle is quite slow on the first invocation.
>> - As seen in the benchmark, users can first call
>> `Lookup::ensureInitialized` to create an eager handle.
>> - After that, the lazy handle has a performance on par with the regular
>> var handle.
>> - [ ] The class-loading-based test is not in a unit test
>> - The test frameworks don't seem to offer fine-grained control for
>> class-loading detection or reliable unloading
>>
>>
>> Benchmark Mode Cnt Score
>> Error Units
>> VarHandleLazyStaticInvocation.initializedInvocation avgt 30 0.769 ±
>> 0.003 ns/op
>> VarHandleLazyStaticInvocation.lazyInvocation avgt 30 0.767 ±
>> 0.002 ns/op
>>
>>
>> Benchmark Mode Cnt Score
>> Error Units
>> LazyStaticColdStart.methodHandleCreateEager ss 10 73140.100 ±
>> 7735.276 ns/op
>> LazyStaticColdStart.methodHandleCreateLazy ss 10 50000.000 ±
>> 8482.883 ns/op
>> LazyStaticColdStart.methodHandleInitializeCallEager ss 10 91350.100 ±
>> 9776.343 ns/op
>> LazyStaticColdStart.methodHandleInitializeCallLazy ss 10 145540.200 ±
>> 72894.865 ns/op
>> LazyStaticColdStart.varHandleCreateEager ss 10 47069.900 ±
>> 3513.909 ns/op
>> LazyStaticColdStart.varHandleCreateLazy ss 10 24780.300 ±
>> 5144.540 ns/op
>> LazyStaticColdStart.varHandleInitializeCallEager ss 10 85479.700 ±
>> 10816.983 ns/op
>> LazyStaticColdStart.varHandleInitializeCallLazy ss 10 413630.100 ±
>> 189370.448 ns/op
>
> Chen Liang has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains eight additional commits since
> the last revision:
>
> - Remove export for removed package
> - Merge branch 'master' into lazy-static-varhandle
> - Test the creation performance of handles, lazy one indeed better
> - Merge branch 'master' into lazy-static-varhandle
> - copyright year
> - Benchmarks. lazy initialize is SLOW
> - nuke meaningless overrides
> - make static field var handles lazy and fix NPE in isAccessModeSupported
src/java.base/share/classes/java/lang/invoke/VarHandles.java line 111:
> 109: else {
> 110: if (UNSAFE.shouldBeInitialized(refc)) {
> 111: return new LazyStaticVarHandle(refc, f,
> isWriteAllowedOnFinalFields, false);
This should probably call `maybeAdapt` on the result:
Suggestion:
return maybeAdapt(new LazyStaticVarHandle(refc, f,
isWriteAllowedOnFinalFields, false));
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1207261501
