RFR: 8346123: [REDO] NMT should not use ThreadCritical [v5]

[Robert Toyonaga]

On Wed, 18 Dec 2024 23:01:38 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Robert Toyonaga has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
>> 
>>  - Merge master
>>  - move MemTracker::set_done_bootstrap()
>>  - Update src/hotspot/share/utilities/vmError.cpp
>>    
>>    Co-authored-by: David Holmes <62092539+dholmes-ora at users.noreply.github.com>
>>  - updates tests and remove old class
>>  - use ConditionalMutexLocker directly
>>  - Fix concurrency issue. Add assertions. Update tests.
>>  - Revert "8343726: [BACKOUT] NMT should not use ThreadCritical"
>>    
>>    This reverts commit c3df050b88ecef123199a4e96f6d9884d064ae45.
>
> Oh yes, now I see, sorry I did repeat Kim's suggestion.

Thank you @coleenp  and @kimbarrett for the advice on the best way to use `ConditionalMutexLocker` without subclassing it. I've adopted your suggestions.

> src/hotspot/share/runtime/mutexLocker.cpp line 292:
> 
>> 290:   MUTEX_DEFN(NMTQuery_lock                   , PaddedMutex  , safepoint);
>> 291:   MUTEX_DEFN(NMTCompilationCostHistory_lock  , PaddedMutex  , nosafepoint);
>> 292:   MUTEX_DEFN(NmtVirtualMemory_lock           , PaddedMutex  , service-4);
> 
> Why is this service-4?  Does it depend on being a rank lower than another lock?  If so, this and the SharedDecoder_lock should be declared below as MUTEX_DEFL and call out that lock.

Yes, it must have a lower rank than "G1Mapper_lock" which is in `G1RegionsSmallerThanCommitSizeMapper`, not in `mutexLocker.cpp`.  I've moved `SharedDecoder_lock` down below as `MUTEX_DEFL` and I've left a comment beside `MUTEX_DEFN(NmtVirtualMemory_lock, PaddedMutex  , service-4);` to explain the rank. Would this be enough?

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

PR Comment: https://git.openjdk.org/jdk/pull/22745#issuecomment-2555767690
PR Review Comment: https://git.openjdk.org/jdk/pull/22745#discussion_r1893136241