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

[Aleksey Shipilev]

On Thu, 23 May 2024 18:06:13 GMT, Neethu <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 has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address feedback on whitespace and comments

I think this is good. 

Yes, we could have restructured the code so that `nmethod_entry_barrier` was not called when nmethod is already disarmed. There are already some places where we check it externally, but the reproducer in the bug shows that it is easy to miss. So checking right here in the method looks appropriate.

@fisk might want to take a look as well.

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

Marked as reviewed by shade (Reviewer).

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