On Fri, 5 May 2023 13:00:11 GMT, Maurizio Cimadamore <mcimadam...@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
>> VarHandleLazyStaticCreation.eagerInitialize ss 10 348580.000 ±
>> 115234.161 ns/op
>> VarHandleLazyStaticCreation.lazyInitialize ss 10 961670.000 ±
>> 154813.238 ns/op
>
> src/java.base/share/classes/java/lang/invoke/IndirectVarHandle.java line 74:
>
>> 72: @Override
>> 73: public boolean isAccessModeSupported(AccessMode accessMode) {
>> 74: var directTarget = this.directTarget;
>
> Why is this not defined in the superclass, and then use `asDirect` (as done
> in other cases) ?
I aim to avoid eager evaluation of directTarget (which is lazy in
LazyStaticVarHandle, computing it requires initializing the holder class). This
code path checks a stable field, so it should allow constant-folding when the
direct target is available.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1186070262
