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

[Aleksey Shipilev]

On Thu, 6 Jun 2024 10:49:32 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> Neethu Prasad has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8331911: Remove is_armed check from callers
>
> 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.

I agree with Neethu, though: I think we should proceed to the `DeoptimizeNMethodBarrierALot` block and cross-modify fences only when barrier acted. We know from testing that it hurts otherwise. I would prefer this change not to introduce new performance potholes, even for fastdebug modes.

If the argument is cleanliness on who is checking "armed", and that we decide it should be solely in backend, then the middle ground might be adding the out-parameter, like `nmethod_entry_barrier(nmethod* nm, bool has_acted)`, and checking that before proceeding here? That feels uglier than just leaving the check here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19285#discussion_r1629299546