On Thu, 13 Nov 2025 09:39:13 GMT, Christian Hagedorn <[email protected]> wrote:
> When running with `-XX:+DeoptimizeNMethodBarriersALot` (done in stress jobs), > we could occasionally take the slow path in the nmethod stub entry barrier > which does not directly deoptimize the nmethod but just simulate the > indirection over the stubs: > https://github.com/openjdk/valhalla/blob/a180fefdd8f427c647f3a4f3a4bc937207d1fe72/src/hotspot/share/gc/shared/barrierSetNMethod.cpp#L198-L209 > > In `deoptimize()`, we make sure to jump to the "handle wrong method" stub: > https://github.com/openjdk/valhalla/blob/a180fefdd8f427c647f3a4f3a4bc937207d1fe72/src/hotspot/cpu/x86/gc/shared/barrierSetNMethod_x86.cpp#L149-L152 > > which calls into the VM to `SharedRuntime::handle_wrong_method()`. In this > method, we try to reresolve the call site: > https://github.com/openjdk/valhalla/blob/a180fefdd8f427c647f3a4f3a4bc937207d1fe72/src/hotspot/share/runtime/sharedRuntime.cpp#L1608-L1609 > > We also try to find out if the call was optimized or not by setting the > out-parameter `is_optimized`. However, when we find that the caller is > already deoptimized (which can happen more often when additionally running > with `-XX:+DeoptimizeALot`), we will not set `is_optimized`: > https://github.com/openjdk/valhalla/blob/c4030c8fbc80ff691e1d129c9ad8fabe9282969c/src/hotspot/share/runtime/sharedRuntime.cpp#L1795-L1800 > > This is wrongly adpated from mainline where we are only concerned about > updating the IC. There it makes sense to skip the code below when the caller > is already deoptimized. > > #### Solution > I therefore propose to change the code to set `is_optimized`, regardless > whether the caller is deoptimized or not, when the caller is scalarized and > the callee has a scalarized receiver. This avoids wrongly skipping a buffer > allocation. > > Additional changes: > - I also removed the `is_static_call` out-parameter of > `reresolve_call_site()`- we can directly set that via the `callee_method` in > `handle_wrong_method()`. I added a sanity assert in `reresolve_call_site()` > that we would get the same result. > - Renamed `caller_is_c1` to `caller_does_not_scalarize` in the header file > which was missed in a prior renaming. > - There are two remaining issues: > - Issues with virtual threads (tracking issue: > [JDK-8370177](https://bugs.openjdk.org/browse/JDK-8370177)) -> I disabled > virtual threads in the test to reduce noise. > - Problems on AArch64 with the framepointer (tracking issue: > [JDK-8367151](https://bugs.openjdk.org/browse/JDK-8367151)) > > Thanks, > Christian Nice analysis, Christian! Scary that this went unnoticed for so long. The fix looks good to me. ------------- Marked as reviewed by thartmann (Committer). PR Review: https://git.openjdk.org/valhalla/pull/1735#pullrequestreview-3459635052
