RFR: 8331911: Reconsider locking for recently disarmed nmethods [v4]

[Erik Österlund]

On Wed, 5 Jun 2024 20:45:22 GMT, Neethu Prasad <duke at openjdk.org> wrote:

>> **Notes**
>> We are spending significant time on acquiring the per-nmethod as all the
>> threads are in same nmethod.
>> Adding double-check lock by calling is_armed before lock acquisition.
>> 
>> **Verification**
>> 
>> Shenendoah
>> 
>>> % /home/neethp/Development/opensource/jdk/build/linux-x86_64-server-release/images/jdk/bin/java -Xmx1g -Xms1g -XX:+UseShenandoahGC -Xlog:gc ManyThreadsStacks.java 2>&1 | grep "marking roots"
>>> [0.706s][info][gc] GC(0) Concurrent marking roots 11.519ms
>>> [0.752s][info][gc] GC(1) Concurrent marking roots 9.833ms
>>> [0.814s][info][gc] GC(2) Concurrent marking roots 10.000ms
>>> [0.855s][info][gc] GC(3) Concurrent marking roots 9.314ms
>>> [0.895s][info][gc] GC(4) Concurrent marking roots 8.937ms
>>> [1.213s][info][gc] GC(5) Concurrent marking roots 12.582ms
>>> [1.340s][info][gc] GC(6) Concurrent marking roots 9.574ms
>>> [1.465s][info][gc] GC(7) Concurrent marking roots 12.791ms
>> 
>> ZGC
>> 
>>> % /home/neethp/Development/opensource/jdk/build/linux-x86_64-server-release/images/jdk/bin/java -Xmx1g -Xms1g -XX:+UseShenandoahGC -Xlog:gc ManyThreadsStacks.java 2>&1 | grep "marking roots"
>>> [0.732s][info][gc] GC(0) Concurrent marking roots 10.694ms
>>> [0.782s][info][gc] GC(1) Concurrent marking roots 14.614ms
>>> [0.825s][info][gc] GC(2) Concurrent marking roots 12.700ms
>>> [0.863s][info][gc] GC(3) Concurrent marking roots 9.622ms
>>> [0.904s][info][gc] GC(4) Concurrent marking roots 12.892ms
>>> [1.244s][info][gc] GC(5) Concurrent marking roots 12.422ms
>>> [1.375s][info][gc] GC(6) Concurrent marking roots 12.756ms
>>> [1.503s][info][gc] GC(7) Concurrent marking roots 12.265ms
>>> [1.628s][info][gc] GC(8) Concurrent marking roots 12.309ms
>>> [1.754s][info][gc] GC(9) Concurrent marking roots 12.996ms
>>> [1.879s][info][gc] GC(10) Concurrent marking roots 9.416ms
>> 
>> **Issue**
>> https://bugs.openjdk.org/browse/JDK-8331911
>
> Neethu Prasad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8331911: Remove is_armed check from callers

Changes requested by eosterlund (Reviewer).

src/hotspot/share/gc/shared/barrierSetNMethod.cpp line 184:

> 182:   // too often. nmethod_entry_barrier checks for disarmed status itself,
> 183:   // but we have no visibility into whether the barrier acted or not.
> 184:   if (!bs_nm->is_armed(nm)) {

I would still like this check gone, together with the comment above.

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

PR Review: https://git.openjdk.org/jdk/pull/19285#pullrequestreview-2101609057
PR Review Comment: https://git.openjdk.org/jdk/pull/19285#discussion_r1629278896