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

[Thomas Stuefe]

On Sat, 21 Dec 2024 16:57:08 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Robert Toyonaga has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   move boostrapping_done flag into Mutex from MutexLocker
>
> src/hotspot/share/runtime/mutexLocker.cpp line 291:
> 
>> 289:   MUTEX_DEFN(NMTQuery_lock                   , PaddedMutex  , safepoint);
>> 290:   MUTEX_DEFN(NMTCompilationCostHistory_lock  , PaddedMutex  , nosafepoint);
>> 291:   MUTEX_DEFN(NmtVirtualMemory_lock           , PaddedMutex  , service-4); // Must be lower than G1Mapper_lock used from G1RegionsSmallerThanCommitSizeMapper::commit_regions
> 
> Is it really okay for nmt to use a VM mutex? That means relevant functions can
> only be called from a VM thread. In particular, what happens if we get into
> the error handler because of a signal on a non-VM thread, or something
> similarly weird.
> 
> This is also replacing the PlatformMutex that was being used by
> MemoryFileTracker. I don't know why that was using a PlatformMutex, but I
> presume there was a reason. It was doing so in the initial version of
> JDK-8312132. Looking back through the development commits, is see these two in
> sequence:
> 
> 3472573 Introduce a separate Mutex for MemoryFileTracker
> 46f10f6 Use own PlatformMutex instead
> 
> I found no discussion about that choice in the PR. Maybe it was because of
> problems determining the needed lock rank? (The original VM mutex had
> Mutex::service rank. That's apparently too high, as discussed in this change.)
> Or maybe there was some other reason?
> 
> I'll ask @jdksjolen about this, though holidays...

@kimbarrett This was my proposal, but I now doubt whether it was a good proposal. My reasoning was that we want to have deadlock detection (NMT locking is somewhat "midlevel" since the section it encompasses can be large) and things like "assert_lock_strong". Admittedly, the latter can be easily done differently with a raw OS-side mutex.

After thinking about this more, the advantages of a raw OS-side mutex would be:
- not having to think about initialization order
- can be statically initialized at least on Posix
- it works when we are called from a non-VM thread - ideas both I and @jdksjolen had would be extending NMT to outside, which opens us to being called both long before VM init and outside any VM thread

Not sure anymore what to think. What do others think? Go back to Roberts original proposal?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22745#discussion_r1895386033