On Sat, 3 Jun 2023 00:16:23 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 12.668 ±
>> 0.069 ns/op
>> VarHandleLazyStaticInvocation.lazyInvocation avgt 30 12.683 ±
>> 0.069 ns/op
>>
>>
>> Benchmark Mode Cnt Score
>> Error Units
>> LazyStaticColdStart.methodHandleCreateEager ss 10 50.980 ±
>> 9.454 us/op
>> LazyStaticColdStart.methodHandleCreateLazy ss 10 24.350 ±
>> 6.701 us/op
>> LazyStaticColdStart.methodHandleInitializeCallEager ss 10 65.140 ±
>> 7.552 us/op
>> LazyStaticColdStart.methodHandleInitializeCallLazy ss 10 118.360 ±
>> 20.320 us/op
>> LazyStaticColdStart.varHandleCreateEager ss 10 49.500 ±
>> 4.277 us/op
>> LazyStaticColdStart.varHandleCreateLazy ss 10 26.690 ±
>> 5.157 us/op
>> LazyStaticColdStart.varHandleInitializeCallEager ss 10 87.930 ±
>> 12.643 us/op
>> LazyStaticColdStart.varHandleInitializeCallLazy ss 10 1057.120 ±
>> 189.810 us/op
>
> Chen Liang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove lazy init of MH, for all invocation of the Lazy VH needs this MH
src/java.base/share/classes/java/lang/invoke/LazyInitializingVarHandle.java
line 106:
> 104: UNSAFE.ensureClassInitialized(refc);
> 105: this.initialized = true;
> 106: }
This should probably have a fast‑path when `this.initialized` is `true`, so
that it can be better constant folded by the **JIT**:
Suggestion:
private void ensureInitialized() {
if (this.initialized) {
return;
}
UNSAFE.ensureClassInitialized(refc);
this.initialized = true;
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1215132431
