On Thu, 8 Jun 2023 01:32:20 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.817 ±
>> 0.012 ns/op
>> VarHandleLazyStaticInvocation.lazyInvocation avgt 30 0.805 ±
>> 0.007 ns/op
>>
>>
>> Benchmark Mode Cnt Score
>> Error Units
>> Benchmark Mode Cnt Score
>> Error Units
>> LazyStaticColdStart.methodHandleCreateEager ss 10 36.890 ±
>> 2.891 us/op
>> LazyStaticColdStart.methodHandleCreateLazy ss 10 18.340 ±
>> 1.537 us/op
>> LazyStaticColdStart.methodHandleInitializeCallEager ss 10 50.000 ±
>> 5.590 us/op
>> LazyStaticColdStart.methodHandleInitializeCallLazy ss 10 90.550 ±
>> 10.142 us/op
>> LazyStaticColdStart.varHandleCreateEager ss 10 36.610 ±
>> 2.685 us/op
>> LazyStaticColdStart.varHandleCreateLazy ss 10 18.200 ±
>> 1.811 us/op
>> LazyStaticColdStart.varHandleInitializeCallEager ss 10 71.680 ±
>> 11.097 us/op
>> LazyStaticColdStart.varHandleInitializeCallLazy ss 10 72.090 ±
>> 4.494 us/op
>
> Chen Liang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove meaningless target calls and clear outdated cache as needed
Something was bothering me about the current complexity with method handles and
the potential interactions with indirect var handles. Its hard to reason about
all the interactions so i needed to take a deeper dive, which i really should
have done earlier on. Apologies if we are circling around this many times.
Since we are now leaning on `LazyInitializingVarHandle::target` to initialize I
think we can just do this:
private void initialize() {
UNSAFE.ensureClassInitialized(refc);
this.initialized = true;
}
@Override
public MethodHandle toMethodHandle(AccessMode accessMode) {
if (initialized) {
return target.toMethodHandle(accessMode);
} else {
if (isAccessModeSupported(accessMode)) {
MethodHandle mh = target.getMethodHandle(accessMode.ordinal());
// Ensure target on this VH is called to initialize the class
return mh.bindTo(this);
}
else {
// Ensure an UnsupportedOperationException is thrown
return MethodHandles.varHandleInvoker(accessMode,
accessModeType(accessMode)).
bindTo(this);
}
}
}
However, construction of an indirect VH will result in initialization:
private IndirectVarHandle(VarHandle target, Class<?> value, Class<?>[]
coordinates,
BiFunction<AccessMode, MethodHandle, MethodHandle>
handleFactory, VarForm form, boolean exact) {
super(form, exact);
this.handleFactory = handleFactory;
this.target = target;
this.directTarget = target.target();
this.value = value;
this.coordinates = coordinates;
}
Since this is not performance sensitive code we could check if target is an
instance of `LazyInitializingVarHandle` then conditionally get it's target if
initialized e.g.,
this.target = target instanceof LazyInitializingVarHandle lazyTarget ?
lazyTarget.lazyTarget() : target.target();
?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13821#issuecomment-1585012917
