On Wed, 11 Sep 2024 06:10:11 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>>> Thank you Robert. >>> >>> I think that the `MemoryFileTracker`'s locker should probably also be >>> replaced with this semaphore, as in the future we have plans to have a >>> globally shared `NativeCallStackStorage`. >> >> Hi @jdksjolen. No problem! Ok I've replaced it. > >> > @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 >> ``` > > Hi @roberttoyonaga, > > sorry for the sluggish responses, I am swamped atm. Thanks a lot for your > work here. > > I dislike the self-written lock because of duplication and because Mutex > gives us infrastructure benefits we don't have here. E.g. locking monitoring, > deadlock detection, and things like `assert_lock_strong`. > > You run into this problem at the initialization phase. I expected that, and > here is where things gets interesting - here and in deadlock detection. > > I see two better solutions: > > 1) During initialization, we don't need a lock. We are single-threaded (note > that it may be different threads at different times, since different threads > may handle CreateJavaVM and the initial invocation, but only one thread will > run simultaneously). In any case, before the mutex system is initialized and > the Mutexes are created, we don't need to lock. > ... Hi @tstuefe, > I prefer (1), since then, locking would work as it does in all other places > in hotspot. Ok I've switched to using a regular hotspot `Mutex` instead of my own lock based on a semaphore. - I had to decrease the rank of `SharedDecoder_lock` in order for it to be acquired after `NMT_lock` inside `MemTracker::print_containing_region` - I needed to add a check for `Thread::current_or_null() != nullptr` in the constructor/destructor for `MutexLockerImpl` because, during VM init, `NMT_lock` seems to be used by detached threads. - I'm still a little concerned about possible reentrancy due to the [previous comment above](https://github.com/openjdk/jdk/pull/20852#discussion_r1748744931) so I manually unlock `NMT_lock` inside error reporting in addition to reducing tracking to summary level. ------------- PR Comment: https://git.openjdk.org/jdk/pull/20852#issuecomment-2346513123
