RFR: 8341127: Extra call to MethodHandle::asType from memory segment var handles fails to inline [v2]

[Jorn Vernee]

On Tue, 1 Oct 2024 10:18:15 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> The fix for JDK-8331865 introduced an accidental performance regression.
>> The main issue is that now *all* memory segment var handles go through some round of adaptation.
>> Adapting a var handle results in a so called *indirect* var handle.
>> When an indirect var handle is called through a *var handle guard*, an extra `MethodHandle::asType` call is triggered.
>> In some cases, if `asType` has already been compiled into a big method, it cannot be inlined into the caller, which then results in a failure to inline through the var handle call, resulting in a big performance regression.
>> 
>> The solution is to make sure that the compiled size of `MethodHandle::asType` stays small: this is done by making sure that the slowpath (the one which populates the cache used by `asType`) is not inlined by the JVM. This is done by consolidating the slow path into a separate method, which is annotated with the internal `@DontInline` annotation.
>> 
>> This problem was originally reported here:
>> https://mail.openjdk.org/pipermail/panama-dev/2024-September/020643.html
>> 
>> While we did not test this fix directly, we have made sure that running the problematic benchmark with the flags:
>> 
>> 
>> -XX:CompileCommand=dontinline,java/lang/invoke/MethodHandle.setAsTypeCache
>> -XX:CompileCommand=dontinline,java/lang/invoke/MethodHandle.asTypeUncached
>> 
>> 
>> Solves the performance regression. The fix in this PR is just a programmatic way to achieve the same results w/o the need of command line flags.
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update copyright

src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 889:

> 887:     }
> 888: 
> 889:     @DontInline

Could you leave a comment explaining why we put `DontInline` here?

test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstantAsType.java line 103:

> 101:     }
> 102: 
> 103:     @CompilerControl(CompilerControl.Mode.DONT_INLINE)

I think the intent was to block inlining of `asType`, so it gets compiled in isolation? That should be done with a `CompileCommand` though. This annotation just blocks inlining of `compileAsType` AFAIK.

test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstantAsType.java line 136:

> 134:         for (int i = 0; i < ELEM_SIZE; i++) {
> 135:             sum += segment.get(JAVA_LONG, i * CARRIER_SIZE);
> 136: 

Suggestion:

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21283#discussion_r1782937040
PR Review Comment: https://git.openjdk.org/jdk/pull/21283#discussion_r1782940655
PR Review Comment: https://git.openjdk.org/jdk/pull/21283#discussion_r1782944436