RFR: 8304824: NMT should not use ThreadCritical

[Robert Toyonaga]

On Thu, 5 Sep 2024 21:10:27 GMT, Robert Toyonaga <duke at openjdk.org> wrote:

>> ### Summary
>> This PR just replaces `ThreadCritical` with a lock specific to NMT.  `ThreadCritical` is a big lock and is unnecessary for the purposes of NMT. I've implemented the new lock with a semaphore so that it can be used early before VM init.  There is also the possibility of adding assertions in places we expect NMT to have synchronization. I haven't added assertions yet in many places because some code paths such as the (NMT tests)  don't lock yet. I think it makes sense to close any gaps in locking in another PR in which I can also add more assertions. 
>> 
>> Testing:
>> - hotspot_nmt
>> - gtest:VirtualSpace
>> - tier1
>
> The GHA `macos-aarch64 / test - Test (tier1) `  is failing due to `TEST: runtime/cds/DeterministicDump.java`  with the message `java.lang.RuntimeException: File content different at byte #4, b0 = -126, b1 = -52`. 
> But I don't think it's related to the changes in this PR.  It also looks like this test is failing on other recent PRs.

> @roberttoyonaga Why don't we use a normal hotspot mutex here?

Hi @tstuefe. I tried using a regular mutex along with `MutexLocker`, but it seems like the mutex isn't initialized early enough to be used during VM init. After building I get: 

`assert(NMT_lock != nullptr) failed: not initialized!`


Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0x178448c]  VMError::report_and_die(int, char const*, char const*, __va_list_tag*, Thread*, unsigned char*, void*, void*, char const*, int, unsigned long)+0x948  (nmtCommon.hpp:137)
V  [libjvm.so+0x1783aed]  (vmError.cpp:1611)
V  [libjvm.so+0xa8e2d2]  report_vm_status_error(char const*, int, char const*, int, char const*)+0x0  (debug.cpp:193)
V  [libjvm.so+0x8cb317]  NMTUtil::nmt_lock()+0x69  (nmtCommon.hpp:137)
V  [libjvm.so+0x13a0459]  MemTracker::record_virtual_memory_reserve(void*, unsigned long, NativeCallStack const&, MEMFLAGS)+0x36  (memTracker.hpp:128)
V  [libjvm.so+0x139d292]  os::reserve_memory(unsigned long, bool, MEMFLAGS)+0x80  (os.cpp:1877)
V  [libjvm.so+0xa8fc31]  initialize_assert_poison()+0x1f  (debug.cpp:712)
V  [libjvm.so+0x16e5720]  Threads::create_vm(JavaVMInitArgs*, bool*)+0x1be  (threads.cpp:489)
V  [libjvm.so+0xf62555]  JNI_CreateJavaVM_inner(JavaVM_**, void**, void*)+0xbd  (jni.cpp:3582)
V  [libjvm.so+0xf629e3]  JNI_CreateJavaVM+0x32  (jni.cpp:3673)
C  [libjli.so+0x8c04]  InitializeJVM+0x14d  (java.c:1592)
C  [libjli.so+0x5337]  JavaMain+0xe8  (java.c:505)
C  [libjli.so+0xc61e]  ThreadJavaMain+0x27  (java_md.c:653)
C  [libc.so.6+0x97507]  start_thread+0x377

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

PR Comment: https://git.openjdk.org/jdk/pull/20852#issuecomment-2336137156