On Wed, 18 Dec 2024 23:01:38 GMT, Coleen Phillimore <cole...@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-...@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
