On Tue, 1 Oct 2024 10:18:15 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: > > 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
