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

[Robert Toyonaga]

On Sat, 21 Dec 2024 07:16:22 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/nmt/threadStackTracker.cpp line 55:
> 
>> 53:   align_thread_stack_boundaries_inward(base, size);
>> 54: 
>> 55:   MemTracker::MemTracker::NmtVirtualMemoryLocker nvml;
> 
> Too many "MemTracker"s here?

thank you for spotting. I'll remove that

> src/hotspot/share/nmt/virtualMemoryTracker.cpp line 632:
> 
>> 630:     if (Mutex::is_bootstrapping_done()) {
>> 631:       assert_lock_strong(NmtVirtualMemory_lock);
>> 632:     }
> 
> It seems like this can use `MemTracker::assert_locked()`, since that is conditioned on bootstrapping state.

`assert_lock_strong`  skips checking the lock ownership if `DebuggingContext::is_enabled() || VMError::is_error_reported()`. `MemTracker::assert_locked()` does not do that.  Is that ok?

> src/hotspot/share/runtime/mutex.hpp line 222:
> 
>> 220:   }
>> 221:  private:
>> 222:   static bool _bootstrapping_done;
> 
> A description of what these mean would be nice. It seems to be mutex_init completed + threading
> initialization completed?

Yes, that's right. I'll add a comment describing this.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22745#discussion_r1895809641
PR Review Comment: https://git.openjdk.org/jdk/pull/22745#discussion_r1895811421
PR Review Comment: https://git.openjdk.org/jdk/pull/22745#discussion_r1895812737