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

[Erik Österlund]

On Wed, 29 May 2024 15:35:15 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:
> 
>   Update full name

It seems fine to me that the GC backends are responsible for checking if the nmethod is disarmed outside the lock. However, we have some callers that now check it redundantly. I think those callers should stop doing that now. Otherwise, this looks good to me.

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

Changes requested by eosterlund (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19285#pullrequestreview-2085826962