On Fri, 5 May 2023 13:01:09 GMT, Maurizio Cimadamore <[email protected]>
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/VarHandles.java line 110:
>
>> 108: }
>> 109: else {
>> 110: if (UNSAFE.shouldBeInitialized(refc)) {
>
> This seems unrelated to the issue this PR is really about (if I understand
> correctly). Would it make sense to address this as part of another PR?
Also, perhaps @PaulSandoz knows more history on this one (I believe this code
predates FFM).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1186069095
