On Fri, 9 Jun 2023 18:59:41 GMT, Paul Sandoz <psan...@openjdk.org> wrote:
>> 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.directTarget = target instanceof LazyInitializingVarHandle lazyTarget
> ? lazyTarget.lazyTarget() : target.target();
>
> ?
@PaulSandoz I think this patch as of now is what I want:
1. Lazy VH works both when obtained directly or delegated by an Indirect VH
2. In the most common usage scenario (direct calls on VH), the lazy VH has a
good initial performance and a similar long-term performance compared to the
eager VH
3. The invasive changes to existing structures are minimized.
For the performance concerns of `getMethodHandle` and `toMethodHandle` (mainly
creation and initial invocation), I find that the VH invocation semantics
itself has some problems: for example, it's quite pointless to pass a
VH-specific direct VH to a VH-specific `getMethodHandle`. My focus on this
patch will be to ensure the correctness of MH-related content, but caring about
their performance is out of scope for me.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13821#issuecomment-1609577849
