On Fri, 5 May 2023 04:48:09 GMT, Chen Liang <[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/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) ?
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?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1186067149
PR Review Comment: https://git.openjdk.org/jdk/pull/13821#discussion_r1186068196
