8346123: [REDO] NMT should not use ThreadCritical

Reviewed-by: dholmes, coleenp, stuefe
This commit is contained in:
Robert Toyonaga 2025-01-18 17:21:28 +00:00 committed by Thomas Stuefe
parent 1f0efc0091
commit 3804082cba
19 changed files with 96 additions and 49 deletions

View File

@ -1086,7 +1086,7 @@ static char* mmap_create_shared(size_t size) {
static void unmap_shared(char* addr, size_t bytes) {
int res;
if (MemTracker::enabled()) {
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
res = ::munmap(addr, bytes);
if (res == 0) {
MemTracker::record_virtual_memory_release((address)addr, bytes);

View File

@ -3639,10 +3639,7 @@ bool os::pd_release_memory(char* addr, size_t bytes) {
// Handle mapping error. We assert in debug, unconditionally print a warning in release.
if (err != nullptr) {
log_warning(os)("bad release: [" PTR_FORMAT "-" PTR_FORMAT "): %s", p2i(start), p2i(end), err);
#ifdef ASSERT
os::print_memory_mappings((char*)start, bytes, tty);
assert(false, "bad release: [" PTR_FORMAT "-" PTR_FORMAT "): %s", p2i(start), p2i(end), err);
#endif
return false;
}
// Free this range

View File

@ -1803,7 +1803,7 @@ void PerfMemory::detach(char* addr, size_t bytes) {
if (MemTracker::enabled()) {
// it does not go through os api, the operation has to record from here
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
remove_file_mapping(addr);
MemTracker::record_virtual_memory_release((address)addr, bytes);
} else {

View File

@ -141,7 +141,7 @@ void MemBaseline::baseline_summary() {
MallocMemorySummary::snapshot(&_malloc_memory_snapshot);
VirtualMemorySummary::snapshot(&_virtual_memory_snapshot);
{
MemoryFileTracker::Instance::Locker lock;
MemTracker::NmtVirtualMemoryLocker nvml;
MemoryFileTracker::Instance::summary_snapshot(&_virtual_memory_snapshot);
}

View File

@ -28,6 +28,7 @@
#include "nmt/mallocTracker.hpp"
#include "nmt/memTag.hpp"
#include "nmt/memReporter.hpp"
#include "nmt/memTracker.hpp"
#include "nmt/memoryFileTracker.hpp"
#include "nmt/threadStackTracker.hpp"
#include "nmt/virtualMemoryTracker.hpp"
@ -465,7 +466,7 @@ void MemDetailReporter::report_virtual_memory_region(const ReservedMemoryRegion*
void MemDetailReporter::report_memory_file_allocations() {
stringStream st;
{
MemoryFileTracker::Instance::Locker lock;
MemTracker::NmtVirtualMemoryLocker nvml;
MemoryFileTracker::Instance::print_all_reports_on(&st, scale());
}
output()->print_raw(st.freeze());

View File

@ -52,6 +52,8 @@ NMT_TrackingLevel MemTracker::_tracking_level = NMT_unknown;
MemBaseline MemTracker::_baseline;
bool MemTracker::NmtVirtualMemoryLocker::_safe_to_use;
void MemTracker::initialize() {
bool rc = true;
assert(_tracking_level == NMT_unknown, "only call once");

View File

@ -31,7 +31,6 @@
#include "nmt/threadStackTracker.hpp"
#include "nmt/virtualMemoryTracker.hpp"
#include "runtime/mutexLocker.hpp"
#include "runtime/threadCritical.hpp"
#include "utilities/debug.hpp"
#include "utilities/nativeCallStack.hpp"
@ -62,6 +61,12 @@ class MemTracker : AllStatic {
return _tracking_level != NMT_unknown;
}
// This may be called on a detached thread during VM init, so we should check that first.
static inline void assert_locked() {
assert(!NmtVirtualMemoryLocker::is_safe_to_use() || NmtVirtualMemory_lock->owned_by_self(),
"should have acquired NmtVirtualMemory_lock");
}
static inline NMT_TrackingLevel tracking_level() {
return _tracking_level;
}
@ -125,7 +130,7 @@ class MemTracker : AllStatic {
assert_post_init();
if (!enabled()) return;
if (addr != nullptr) {
ThreadCritical tc;
NmtVirtualMemoryLocker nvml;
VirtualMemoryTracker::add_reserved_region((address)addr, size, stack, mem_tag);
}
}
@ -151,7 +156,7 @@ class MemTracker : AllStatic {
assert_post_init();
if (!enabled()) return;
if (addr != nullptr) {
ThreadCritical tc;
NmtVirtualMemoryLocker nvml;
VirtualMemoryTracker::add_reserved_region((address)addr, size, stack, mem_tag);
VirtualMemoryTracker::add_committed_region((address)addr, size, stack);
}
@ -162,7 +167,7 @@ class MemTracker : AllStatic {
assert_post_init();
if (!enabled()) return;
if (addr != nullptr) {
ThreadCritical tc;
NmtVirtualMemoryLocker nvml;
VirtualMemoryTracker::add_committed_region((address)addr, size, stack);
}
}
@ -170,7 +175,7 @@ class MemTracker : AllStatic {
static inline MemoryFileTracker::MemoryFile* register_file(const char* descriptive_name) {
assert_post_init();
if (!enabled()) return nullptr;
MemoryFileTracker::Instance::Locker lock;
NmtVirtualMemoryLocker nvml;
return MemoryFileTracker::Instance::make_file(descriptive_name);
}
@ -178,7 +183,7 @@ class MemTracker : AllStatic {
assert_post_init();
if (!enabled()) return;
assert(file != nullptr, "must be");
MemoryFileTracker::Instance::Locker lock;
NmtVirtualMemoryLocker nvml;
MemoryFileTracker::Instance::free_file(file);
}
@ -187,7 +192,7 @@ class MemTracker : AllStatic {
assert_post_init();
if (!enabled()) return;
assert(file != nullptr, "must be");
MemoryFileTracker::Instance::Locker lock;
NmtVirtualMemoryLocker nvml;
MemoryFileTracker::Instance::allocate_memory(file, offset, size, stack, mem_tag);
}
@ -196,7 +201,7 @@ class MemTracker : AllStatic {
assert_post_init();
if (!enabled()) return;
assert(file != nullptr, "must be");
MemoryFileTracker::Instance::Locker lock;
NmtVirtualMemoryLocker nvml;
MemoryFileTracker::Instance::free_memory(file, offset, size);
}
@ -210,7 +215,7 @@ class MemTracker : AllStatic {
assert_post_init();
if (!enabled()) return;
if (addr != nullptr) {
ThreadCritical tc;
NmtVirtualMemoryLocker nvml;
VirtualMemoryTracker::split_reserved_region((address)addr, size, split, mem_tag, split_tag);
}
}
@ -219,7 +224,7 @@ class MemTracker : AllStatic {
assert_post_init();
if (!enabled()) return;
if (addr != nullptr) {
ThreadCritical tc;
NmtVirtualMemoryLocker nvml;
VirtualMemoryTracker::set_reserved_region_type((address)addr, mem_tag);
}
}
@ -269,6 +274,39 @@ class MemTracker : AllStatic {
// and return true; false if not found.
static bool print_containing_region(const void* p, outputStream* out);
/*
* NmtVirtualMemoryLocker is similar to MutexLocker but can be used during VM init before mutexes are ready or
* current thread has been assigned. Performs no action during VM init.
*
* Unlike malloc, NMT requires locking for virtual memory operations. This is because it must synchronize the usage
* of global data structures used for modelling the effect of virtual memory operations.
* It is important that locking is used such that the actual OS memory operations (mmap) are done atomically with the
* corresponding NMT accounting (updating the internal model). Currently, this is not the case in all situations
* (see JDK-8341491), but this should be changed in the future.
*
* An issue with using Mutex is that NMT is used early during VM initialization before mutexes are initialized
* and current thread is attached. Mutexes do not work under those conditions, so we must use a flag to avoid
* attempting to lock until initialization is finished. Lack of synchronization here should not be a problem since it
* is single threaded at that point in time anyway.
*/
class NmtVirtualMemoryLocker: StackObj {
// Returns true if it is safe to start using this locker.
static bool _safe_to_use;
ConditionalMutexLocker _cml;
public:
NmtVirtualMemoryLocker(): _cml(NmtVirtualMemory_lock, _safe_to_use, Mutex::_no_safepoint_check_flag){}
static inline bool is_safe_to_use() {
return _safe_to_use;
}
// Set in Threads::create_vm once threads and mutexes have been initialized.
static inline void set_safe_to_use() {
_safe_to_use = true;
}
};
private:
static void report(bool summary_only, outputStream* output, size_t scale);
@ -277,8 +315,6 @@ class MemTracker : AllStatic {
static NMT_TrackingLevel _tracking_level;
// Stored baseline
static MemBaseline _baseline;
// Query lock
static Mutex* _query_lock;
};
#endif // SHARE_NMT_MEMTRACKER_HPP

View File

@ -29,13 +29,11 @@
#include "nmt/nmtCommon.hpp"
#include "nmt/nmtNativeCallStackStorage.hpp"
#include "nmt/vmatree.hpp"
#include "runtime/mutex.hpp"
#include "utilities/growableArray.hpp"
#include "utilities/nativeCallStack.hpp"
#include "utilities/ostream.hpp"
MemoryFileTracker* MemoryFileTracker::Instance::_tracker = nullptr;
PlatformMutex* MemoryFileTracker::Instance::_mutex = nullptr;
MemoryFileTracker::MemoryFileTracker(bool is_detailed_mode)
: _stack_storage(is_detailed_mode), _files() {}
@ -132,7 +130,6 @@ bool MemoryFileTracker::Instance::initialize(NMT_TrackingLevel tracking_level) {
_tracker = static_cast<MemoryFileTracker*>(os::malloc(sizeof(MemoryFileTracker), mtNMT));
if (_tracker == nullptr) return false;
new (_tracker) MemoryFileTracker(tracking_level == NMT_TrackingLevel::NMT_detail);
_mutex = new PlatformMutex();
return true;
}
@ -189,11 +186,3 @@ void MemoryFileTracker::summary_snapshot(VirtualMemorySnapshot* snapshot) const
void MemoryFileTracker::Instance::summary_snapshot(VirtualMemorySnapshot* snapshot) {
_tracker->summary_snapshot(snapshot);
}
MemoryFileTracker::Instance::Locker::Locker() {
MemoryFileTracker::Instance::_mutex->lock();
}
MemoryFileTracker::Instance::Locker::~Locker() {
MemoryFileTracker::Instance::_mutex->unlock();
}

View File

@ -30,7 +30,6 @@
#include "nmt/nmtNativeCallStackStorage.hpp"
#include "nmt/virtualMemoryTracker.hpp"
#include "nmt/vmatree.hpp"
#include "runtime/mutex.hpp"
#include "runtime/os.inline.hpp"
#include "utilities/growableArray.hpp"
#include "utilities/nativeCallStack.hpp"
@ -93,14 +92,8 @@ public:
class Instance : public AllStatic {
static MemoryFileTracker* _tracker;
static PlatformMutex* _mutex;
public:
class Locker : public StackObj {
public:
Locker();
~Locker();
};
static bool initialize(NMT_TrackingLevel tracking_level);

View File

@ -25,6 +25,7 @@
#include "precompiled.hpp"
#include "nmt/mallocTracker.hpp"
#include "nmt/memoryFileTracker.hpp"
#include "nmt/memTracker.hpp"
#include "nmt/nmtCommon.hpp"
#include "nmt/nmtUsage.hpp"
#include "nmt/threadStackTracker.hpp"
@ -94,7 +95,7 @@ void NMTUsage::update_vm_usage() {
{ // MemoryFileTracker addition
using MFT = MemoryFileTracker::Instance;
MFT::Locker lock;
MemTracker::NmtVirtualMemoryLocker nvml;
MFT::iterate_summary([&](MemTag tag, const VirtualMemory* vm) {
int i = NMTUtil::tag_to_index(tag);
_vm_by_type[i].committed += vm->committed();

View File

@ -29,7 +29,6 @@
#include "nmt/threadStackTracker.hpp"
#include "nmt/virtualMemoryTracker.hpp"
#include "runtime/os.hpp"
#include "runtime/threadCritical.hpp"
#include "utilities/align.hpp"
#include "utilities/debug.hpp"
#include "utilities/globalDefinitions.hpp"
@ -53,7 +52,7 @@ void ThreadStackTracker::new_thread_stack(void* base, size_t size, const NativeC
assert(base != nullptr, "Should have been filtered");
align_thread_stack_boundaries_inward(base, size);
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
VirtualMemoryTracker::add_reserved_region((address)base, size, stack, mtThreadStack);
_thread_count++;
}
@ -63,7 +62,7 @@ void ThreadStackTracker::delete_thread_stack(void* base, size_t size) {
assert(base != nullptr, "Should have been filtered");
align_thread_stack_boundaries_inward(base, size);
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
VirtualMemoryTracker::remove_released_region((address)base, size);
_thread_count--;
}

View File

@ -30,7 +30,6 @@
#include "nmt/threadStackTracker.hpp"
#include "nmt/virtualMemoryTracker.hpp"
#include "runtime/os.hpp"
#include "runtime/threadCritical.hpp"
#include "utilities/ostream.hpp"
VirtualMemorySnapshot VirtualMemorySummary::_snapshot;
@ -338,6 +337,8 @@ bool VirtualMemoryTracker::add_reserved_region(address base_addr, size_t size,
assert(base_addr != nullptr, "Invalid address");
assert(size > 0, "Invalid size");
assert(_reserved_regions != nullptr, "Sanity check");
MemTracker::assert_locked();
ReservedMemoryRegion rgn(base_addr, size, stack, mem_tag);
ReservedMemoryRegion* reserved_rgn = _reserved_regions->find(rgn);
@ -416,6 +417,7 @@ bool VirtualMemoryTracker::add_reserved_region(address base_addr, size_t size,
void VirtualMemoryTracker::set_reserved_region_type(address addr, MemTag mem_tag) {
assert(addr != nullptr, "Invalid address");
assert(_reserved_regions != nullptr, "Sanity check");
MemTracker::assert_locked();
ReservedMemoryRegion rgn(addr, 1);
ReservedMemoryRegion* reserved_rgn = _reserved_regions->find(rgn);
@ -434,6 +436,7 @@ bool VirtualMemoryTracker::add_committed_region(address addr, size_t size,
assert(addr != nullptr, "Invalid address");
assert(size > 0, "Invalid size");
assert(_reserved_regions != nullptr, "Sanity check");
MemTracker::assert_locked();
ReservedMemoryRegion rgn(addr, size);
ReservedMemoryRegion* reserved_rgn = _reserved_regions->find(rgn);
@ -454,6 +457,7 @@ bool VirtualMemoryTracker::remove_uncommitted_region(address addr, size_t size)
assert(addr != nullptr, "Invalid address");
assert(size > 0, "Invalid size");
assert(_reserved_regions != nullptr, "Sanity check");
MemTracker::assert_locked();
ReservedMemoryRegion rgn(addr, size);
ReservedMemoryRegion* reserved_rgn = _reserved_regions->find(rgn);
@ -469,6 +473,7 @@ bool VirtualMemoryTracker::remove_uncommitted_region(address addr, size_t size)
bool VirtualMemoryTracker::remove_released_region(ReservedMemoryRegion* rgn) {
assert(rgn != nullptr, "Sanity check");
assert(_reserved_regions != nullptr, "Sanity check");
MemTracker::assert_locked();
// uncommit regions within the released region
ReservedMemoryRegion backup(*rgn);
@ -490,6 +495,7 @@ bool VirtualMemoryTracker::remove_released_region(address addr, size_t size) {
assert(addr != nullptr, "Invalid address");
assert(size > 0, "Invalid size");
assert(_reserved_regions != nullptr, "Sanity check");
MemTracker::assert_locked();
ReservedMemoryRegion rgn(addr, size);
ReservedMemoryRegion* reserved_rgn = _reserved_regions->find(rgn);
@ -621,6 +627,9 @@ public:
SnapshotThreadStackWalker() {}
bool do_allocation_site(const ReservedMemoryRegion* rgn) {
if (MemTracker::NmtVirtualMemoryLocker::is_safe_to_use()) {
assert_lock_strong(NmtVirtualMemory_lock);
}
if (rgn->mem_tag() == mtThreadStack) {
address stack_bottom = rgn->thread_stack_uncommitted_bottom();
address committed_start;
@ -661,7 +670,7 @@ void VirtualMemoryTracker::snapshot_thread_stacks() {
bool VirtualMemoryTracker::walk_virtual_memory(VirtualMemoryWalker* walker) {
assert(_reserved_regions != nullptr, "Sanity check");
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
// Check that the _reserved_regions haven't been deleted.
if (_reserved_regions != nullptr) {
LinkedListNode<ReservedMemoryRegion>* head = _reserved_regions->head();

View File

@ -3790,8 +3790,8 @@ static jint attach_current_thread(JavaVM *vm, void **penv, void *_args, bool dae
// be set in order for the Safepoint code to deal with it correctly.
thread->set_thread_state(_thread_in_vm);
thread->record_stack_base_and_size();
thread->register_thread_stack_with_NMT();
thread->initialize_thread_current();
thread->register_thread_stack_with_NMT();
MACOS_AARCH64_ONLY(thread->init_wx());
if (!os::create_attached_thread(thread)) {

View File

@ -135,6 +135,7 @@ Mutex* SharedDecoder_lock = nullptr;
Mutex* DCmdFactory_lock = nullptr;
Mutex* NMTQuery_lock = nullptr;
Mutex* NMTCompilationCostHistory_lock = nullptr;
Mutex* NmtVirtualMemory_lock = nullptr;
#if INCLUDE_CDS
#if INCLUDE_JVMTI
@ -284,10 +285,10 @@ void mutex_init() {
MUTEX_DEFN(CodeHeapStateAnalytics_lock , PaddedMutex , safepoint);
MUTEX_DEFN(ThreadsSMRDelete_lock , PaddedMonitor, service-2); // Holds ConcurrentHashTableResize_lock
MUTEX_DEFN(ThreadIdTableCreate_lock , PaddedMutex , safepoint);
MUTEX_DEFN(SharedDecoder_lock , PaddedMutex , tty-1);
MUTEX_DEFN(DCmdFactory_lock , PaddedMutex , nosafepoint);
MUTEX_DEFN(NMTQuery_lock , PaddedMutex , safepoint);
MUTEX_DEFN(NMTCompilationCostHistory_lock , PaddedMutex , nosafepoint);
MUTEX_DEFN(NmtVirtualMemory_lock , PaddedMutex , service-4); // Must be lower than G1Mapper_lock used from G1RegionsSmallerThanCommitSizeMapper::commit_regions
#if INCLUDE_CDS
#if INCLUDE_JVMTI
MUTEX_DEFN(CDSClassFileStream_lock , PaddedMutex , safepoint);
@ -345,6 +346,7 @@ void mutex_init() {
MUTEX_DEFL(JVMCI_lock , PaddedMonitor, JVMCIRuntime_lock);
#endif
MUTEX_DEFL(JvmtiThreadState_lock , PaddedMutex , JvmtiVTMSTransition_lock); // Used by JvmtiThreadState/JvmtiEventController
MUTEX_DEFL(SharedDecoder_lock , PaddedMutex , NmtVirtualMemory_lock); // Must be lower than NmtVirtualMemory_lock due to MemTracker::print_containing_region
// Allocate RecursiveMutex
MultiArray_lock = new RecursiveMutex();

View File

@ -117,6 +117,7 @@ extern Mutex* SharedDecoder_lock; // serializes access to the dec
extern Mutex* DCmdFactory_lock; // serialize access to DCmdFactory information
extern Mutex* NMTQuery_lock; // serialize NMT Dcmd queries
extern Mutex* NMTCompilationCostHistory_lock; // guards NMT compilation cost history
extern Mutex* NmtVirtualMemory_lock; // guards NMT virtual memory updates
#if INCLUDE_CDS
#if INCLUDE_JVMTI
extern Mutex* CDSClassFileStream_lock; // FileMapInfo::open_stream_for_jvmti

View File

@ -2203,7 +2203,7 @@ bool os::uncommit_memory(char* addr, size_t bytes, bool executable) {
assert_nonempty_range(addr, bytes);
bool res;
if (MemTracker::enabled()) {
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
res = pd_uncommit_memory(addr, bytes, executable);
if (res) {
MemTracker::record_virtual_memory_uncommit((address)addr, bytes);
@ -2225,7 +2225,7 @@ bool os::release_memory(char* addr, size_t bytes) {
assert_nonempty_range(addr, bytes);
bool res;
if (MemTracker::enabled()) {
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
res = pd_release_memory(addr, bytes);
if (res) {
MemTracker::record_virtual_memory_release((address)addr, bytes);
@ -2310,7 +2310,7 @@ char* os::map_memory(int fd, const char* file_name, size_t file_offset,
bool os::unmap_memory(char *addr, size_t bytes) {
bool result;
if (MemTracker::enabled()) {
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
result = pd_unmap_memory(addr, bytes);
if (result) {
MemTracker::record_virtual_memory_release((address)addr, bytes);
@ -2349,7 +2349,7 @@ char* os::reserve_memory_special(size_t size, size_t alignment, size_t page_size
bool os::release_memory_special(char* addr, size_t bytes) {
bool res;
if (MemTracker::enabled()) {
ThreadCritical tc;
MemTracker::NmtVirtualMemoryLocker nvml;
res = pd_release_memory_special(addr, bytes);
if (res) {
MemTracker::record_virtual_memory_release((address)addr, bytes);

View File

@ -542,6 +542,8 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
JavaThread* main_thread = new JavaThread();
main_thread->set_thread_state(_thread_in_vm);
main_thread->initialize_thread_current();
// Once mutexes and main_thread are ready, we can use NmtVirtualMemoryLocker.
MemTracker::NmtVirtualMemoryLocker::set_safe_to_use();
// must do this before set_active_handles
main_thread->record_stack_base_and_size();
main_thread->register_thread_stack_with_NMT();

View File

@ -649,6 +649,12 @@ void VMError::report(outputStream* st, bool _verbose) {
address lastpc = nullptr;
BEGIN
if (MemTracker::enabled() &&
NmtVirtualMemory_lock != nullptr &&
NmtVirtualMemory_lock->owned_by_self()) {
// Manually unlock to avoid reentrancy due to mallocs in detailed mode.
NmtVirtualMemory_lock->unlock();
}
STEP("printing fatal error message")
st->print_cr("#");

View File

@ -93,6 +93,8 @@ public:
size_t size = 0x01000000;
ReservedSpace rs = MemoryReserver::reserve(size, mtTest);
MemTracker::NmtVirtualMemoryLocker nvml;
address addr = (address)rs.base();
address frame1 = (address)0x1234;
@ -167,6 +169,8 @@ public:
size_t size = 0x01000000;
ReservedSpace rs = MemoryReserver::reserve(size, mtTest);
MemTracker::NmtVirtualMemoryLocker nvml;
address addr = (address)rs.base();
address frame1 = (address)0x1234;
@ -253,7 +257,10 @@ public:
static void test_add_committed_region_overlapping() {
size_t size = 0x01000000;
ReservedSpace rs = MemoryReserver::reserve(size, mtTest);
MemTracker::NmtVirtualMemoryLocker nvml;
address addr = (address)rs.base();
address frame1 = (address)0x1234;
@ -425,6 +432,8 @@ public:
size_t size = 0x01000000;
ReservedSpace rs = MemoryReserver::reserve(size, mtTest);
MemTracker::NmtVirtualMemoryLocker nvml;
address addr = (address)rs.base();
address frame1 = (address)0x1234;