From 972f4c8789c9bc73b0be936eb181ae3cc84d42e8 Mon Sep 17 00:00:00 2001 From: Robert Toyonaga Date: Fri, 31 Jul 2026 11:04:53 -0400 Subject: [PATCH] change stack when changing tag --- src/hotspot/share/memory/arena.cpp | 4 ++-- src/hotspot/share/nmt/mallocTracker.cpp | 20 ++++-------------- src/hotspot/share/nmt/mallocTracker.hpp | 4 ++-- src/hotspot/share/nmt/memTracker.hpp | 8 +++---- .../gtest/nmt/test_nmt_chunk_accounting.cpp | 21 ++++++++++++------- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/src/hotspot/share/memory/arena.cpp b/src/hotspot/share/memory/arena.cpp index 310a57dc322..eaa3dda7fe3 100644 --- a/src/hotspot/share/memory/arena.cpp +++ b/src/hotspot/share/memory/arena.cpp @@ -204,7 +204,7 @@ Chunk* ChunkPool::allocate_chunk(Arena* arena, size_t length, AllocFailType allo } chunk = (Chunk*)p; } - MemTracker::chunk_assigned_to_arena(chunk, arena->get_mem_tag()); + MemTracker::chunk_assigned_to_arena(chunk, arena->get_mem_tag(), CALLER_PC); ::new(chunk) Chunk(length); // We rely on arena alignment <= malloc alignment. assert(is_aligned(chunk, ARENA_AMALLOC_ALIGNMENT), "Chunk start address misaligned."); @@ -229,7 +229,7 @@ void ChunkPool::deallocate_chunk(Chunk* c) { c->set_stamp(0); } - MemTracker::add_chunk_to_pool(c); + MemTracker::add_chunk_to_pool(c, CALLER_PC); // If this is a standard-sized chunk, return it to its pool; otherwise free it. ChunkPool* pool = ChunkPool::get_pool_for_size(c->length()); diff --git a/src/hotspot/share/nmt/mallocTracker.cpp b/src/hotspot/share/nmt/mallocTracker.cpp index abe9246e6f1..d31b5b9a509 100644 --- a/src/hotspot/share/nmt/mallocTracker.cpp +++ b/src/hotspot/share/nmt/mallocTracker.cpp @@ -187,7 +187,7 @@ void* MallocTracker::record_malloc(void* malloc_base, size_t size, MemTag mem_ta return memblock; } -void MallocTracker::chunk_assigned_to_arena(void* memblock, MemTag new_tag) { +void MallocTracker::chunk_assigned_to_arena(void* memblock, MemTag new_tag, const NativeCallStack& new_stack) { MallocHeader* header = (MallocHeader*)memblock - 1; // Only decrement per-tag counters, leave the total malloc amounts unchanged. @@ -195,14 +195,8 @@ void MallocTracker::chunk_assigned_to_arena(void* memblock, MemTag new_tag) { uint32_t new_mst_marker = 0; if (MemTracker::tracking_level() == NMT_detail && header->mem_tag() != new_tag) { - // retrieve the old stack from MST - NativeCallStack old_stack; - if (!MallocSiteTable::access_stack(old_stack, *header)) { - fatal("NMT is now out of sync."); - } MallocSiteTable::deallocation_at(header->size(), header->mst_marker()); - // update MST with new tag - if (!MallocSiteTable::allocation_at(old_stack, header->size(), &new_mst_marker, new_tag)) { + if (!MallocSiteTable::allocation_at(new_stack, header->size(), &new_mst_marker, new_tag)) { fatal("NMT is now out of sync."); } } @@ -216,7 +210,7 @@ void MallocTracker::chunk_assigned_to_arena(void* memblock, MemTag new_tag) { } } -void MallocTracker::add_chunk_to_pool(void* memblock) { +void MallocTracker::add_chunk_to_pool(void* memblock, const NativeCallStack& new_stack) { MallocHeader* header = (MallocHeader*)memblock - 1; assert(header->mem_tag() != mtChunk, "Should only be operating on arena chunks"); @@ -225,14 +219,8 @@ void MallocTracker::add_chunk_to_pool(void* memblock) { uint32_t new_mst_marker = 0; if (MemTracker::tracking_level() == NMT_detail) { - NativeCallStack old_stack; - // retrieve the old stack from MST - if (!MallocSiteTable::access_stack(old_stack, *header)) { - fatal("NMT is now out of sync."); - } MallocSiteTable::deallocation_at(header->size(), header->mst_marker()); - // update MST with new tag - if (!MallocSiteTable::allocation_at(old_stack, header->size(), &new_mst_marker, mtChunk)) { + if (!MallocSiteTable::allocation_at(new_stack, header->size(), &new_mst_marker, mtChunk)) { fatal("NMT is now out of sync."); } } diff --git a/src/hotspot/share/nmt/mallocTracker.hpp b/src/hotspot/share/nmt/mallocTracker.hpp index 78b35048b56..00ec4fb5cd8 100644 --- a/src/hotspot/share/nmt/mallocTracker.hpp +++ b/src/hotspot/share/nmt/mallocTracker.hpp @@ -278,8 +278,8 @@ class MallocTracker : AllStatic { static void* record_malloc(void* malloc_base, size_t size, MemTag mem_tag, const NativeCallStack& stack); - static void chunk_assigned_to_arena(void* memblock, MemTag new_tag); - static void add_chunk_to_pool(void* memblock); + static void chunk_assigned_to_arena(void* memblock, MemTag new_tag, const NativeCallStack& new_stack); + static void add_chunk_to_pool(void* memblock, const NativeCallStack& new_stack); // Given a block returned by os::malloc() or os::realloc(): // deaccount block from NMT, mark its header as dead and return pointer to header. diff --git a/src/hotspot/share/nmt/memTracker.hpp b/src/hotspot/share/nmt/memTracker.hpp index 395c3131dfe..ea0948a50e7 100644 --- a/src/hotspot/share/nmt/memTracker.hpp +++ b/src/hotspot/share/nmt/memTracker.hpp @@ -90,20 +90,20 @@ class MemTracker : AllStatic { return mem_base; } - static inline void chunk_assigned_to_arena(void* memblock, MemTag new_tag) { + static inline void chunk_assigned_to_arena(void* memblock, MemTag new_tag, const NativeCallStack& new_stack) { assert(memblock != nullptr, "caller should handle null"); if (!enabled()) { return; } - MallocTracker::chunk_assigned_to_arena(memblock, new_tag); + MallocTracker::chunk_assigned_to_arena(memblock, new_tag, new_stack); } - static inline void add_chunk_to_pool(void* memblock) { + static inline void add_chunk_to_pool(void* memblock, const NativeCallStack& new_stack) { assert(memblock != nullptr, "caller should handle null"); if (!enabled()) { return; } - MallocTracker::add_chunk_to_pool(memblock); + MallocTracker::add_chunk_to_pool(memblock, new_stack); } // Record malloc free and return malloc base address diff --git a/test/hotspot/gtest/nmt/test_nmt_chunk_accounting.cpp b/test/hotspot/gtest/nmt/test_nmt_chunk_accounting.cpp index 748179624ce..5b94e0972ae 100644 --- a/test/hotspot/gtest/nmt/test_nmt_chunk_accounting.cpp +++ b/test/hotspot/gtest/nmt/test_nmt_chunk_accounting.cpp @@ -156,34 +156,39 @@ TEST_VM(NMTChunkAccounting, mst) { if (!MemTracker::enabled() || MemTracker::tracking_level() != NMT_detail) { return; } - void* allocation = os::malloc(100, mtChunk, CALLER_PC); + NativeCallStack stack_a = CALLER_PC; + NativeCallStack stack_b = CURRENT_PC; + void* allocation = os::malloc(100, mtChunk, stack_a); MallocHeader* header = (MallocHeader*)allocation - 1; uint32_t old_marker = header->mst_marker(); NativeCallStack old_stack; ASSERT_TRUE(MallocSiteTable::access_stack(old_stack, *header)); + EXPECT_TRUE(old_stack.equals(stack_a)); - MemTracker::chunk_assigned_to_arena(allocation, mtTest); + MemTracker::chunk_assigned_to_arena(allocation, mtTest, stack_b); EXPECT_TRUE(header->mem_tag() == mtTest); EXPECT_NE(header->mst_marker(), old_marker); NativeCallStack new_stack; EXPECT_TRUE(MallocSiteTable::access_stack(new_stack, *header)); - EXPECT_TRUE(new_stack.equals(old_stack)); + EXPECT_TRUE(new_stack.equals(stack_b)); - MemTracker::add_chunk_to_pool(allocation); + MemTracker::add_chunk_to_pool(allocation, stack_a); ASSERT_TRUE(header->mem_tag() == mtChunk); ASSERT_TRUE(MallocSiteTable::access_stack(new_stack, *header)); - EXPECT_TRUE(new_stack.equals(old_stack)); + EXPECT_TRUE(new_stack.equals(stack_a)); os::free(allocation); // Now test the path where new and old tags are equal. - allocation = os::malloc(100, mtTest, CALLER_PC); + // This should only happen when a fresh chunk is created to grow an arena. + allocation = os::malloc(100, mtTest, stack_b); header = (MallocHeader*)allocation - 1; old_marker = header->mst_marker(); ASSERT_TRUE(MallocSiteTable::access_stack(old_stack, *header)); + EXPECT_TRUE(old_stack.equals(stack_b)); - MemTracker::chunk_assigned_to_arena(allocation, mtTest); + MemTracker::chunk_assigned_to_arena(allocation, mtTest, stack_b); EXPECT_TRUE(header->mem_tag() == mtTest); EXPECT_EQ(header->mst_marker(), old_marker); EXPECT_TRUE(MallocSiteTable::access_stack(new_stack, *header)); - EXPECT_TRUE(new_stack.equals(old_stack)); + EXPECT_TRUE(new_stack.equals(stack_b)); }