On Wed, 26 Apr 2023 10:52:29 GMT, Roman Kennke <rken...@openjdk.org> wrote:
>> This stub has 2 instructions, and it seems not really uncommon, is it worth >> it to have a stub here? > > Ok I will change the value. > Yes, this path is relatively uncommon (monitors are inflated only once, and > not necessarily via ANONYMOUS handshake, but used often), and this path is > performance relevant. The original impl had the two instructions inlined, but > the common forward branch impacted performance. I see, thanks a lot for your explanations >> Also, I think this `if (LockingMode == LM_LIGHTWEIGHT)` block should be >> moved out of the enclosing if block, we are checking for inflation here, it >> seems logical to separate the inflation path out. > > How would I check if we are emitting code? > > I am not sure I understand. The check for ANONYMOUS is only relevant when we > observe an already-inflated monitor. I think this is the right place to put > it. The entry barrier does this: https://github.com/openjdk/jdk/blob/86f41a4c42268d364175263804eb4d1ce82fa943/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp#L139 `testptr(tmpReg, markWord::monitor_value)` is checking for inflation, and the following `if` block acts when inflation is detected, what I mean is to move the whole enclosed if down out of the `if (LockingMode != LM_MONITOR)` ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177712615 PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177711237
