On Tue, 1 Oct 2024 17:49:12 GMT, Maurizio Cimadamore <mcimadam...@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:
> 
>   Address review comments

I don't have a clear understanding how `VarHandleGuards` is used, but I assume 
that `MethodHandle::asType` calls there are no-ops in scenarios you care about.

It does make sense to annotate`setAsTypeCache` with `@DontInline`, but I'd also 
add `@ForceInline` on `asType`.

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

PR Comment: https://git.openjdk.org/jdk/pull/21283#issuecomment-2387220756

Reply via email to