On Thu, 5 Sep 2024 21:10:27 GMT, Robert Toyonaga <d...@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
