From 1581508988141bfb420d97759138203f30926b35 Mon Sep 17 00:00:00 2001 From: Afshin Zafari Date: Mon, 14 Oct 2024 10:51:37 +0000 Subject: [PATCH] 8335091: NMT: VMATree reserve_mapping and commit_mapping APIs need MEMFLAGS while un/-committing API has no MEMFLAGS arg Reviewed-by: jsjolen, gziemski --- src/hotspot/share/nmt/vmatree.cpp | 20 +++++++++++++++- src/hotspot/share/nmt/vmatree.hpp | 32 ++++++++++++++++--------- test/hotspot/gtest/nmt/test_vmatree.cpp | 18 +++++++++++++- 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/hotspot/share/nmt/vmatree.cpp b/src/hotspot/share/nmt/vmatree.cpp index 3795376d4768d..65a5bdb94aef0 100644 --- a/src/hotspot/share/nmt/vmatree.cpp +++ b/src/hotspot/share/nmt/vmatree.cpp @@ -24,6 +24,7 @@ */ #include "precompiled.hpp" +#include "logging/log.hpp" #include "nmt/vmatree.hpp" #include "utilities/growableArray.hpp" @@ -34,7 +35,9 @@ const char* VMATree::statetype_strings[3] = { }; VMATree::SummaryDiff VMATree::register_mapping(position A, position B, StateType state, - const RegionData& metadata) { + const RegionData& metadata, bool use_tag_inplace) { + assert(!use_tag_inplace || metadata.mem_tag == mtNone, + "If using use_tag_inplace, then the supplied tag should be mtNone, was instead: %s", NMTUtil::tag_to_name(metadata.mem_tag)); if (A == B) { // A 0-sized mapping isn't worth recording. return SummaryDiff(); @@ -55,6 +58,10 @@ VMATree::SummaryDiff VMATree::register_mapping(position A, position B, StateType AddressState LEQ_A; TreapNode* leqA_n = _tree.closest_leq(A); if (leqA_n == nullptr) { + assert(!use_tag_inplace, "Cannot use the tag inplace if no pre-existing tag exists. From: " PTR_FORMAT " To: " PTR_FORMAT, A, B); + if (use_tag_inplace) { + log_debug(nmt)("Cannot use the tag inplace if no pre-existing tag exists. From: " PTR_FORMAT " To: " PTR_FORMAT, A, B); + } // No match. We add the A node directly, unless it would have no effect. if (!stA.is_noop()) { _tree.upsert(A, stA); @@ -62,6 +69,17 @@ VMATree::SummaryDiff VMATree::register_mapping(position A, position B, StateType } else { LEQ_A_found = true; LEQ_A = AddressState{leqA_n->key(), leqA_n->val()}; + StateType leqA_state = leqA_n->val().out.type(); + StateType new_state = stA.out.type(); + // If we specify use_tag_inplace then the new region takes over the current tag instead of the tag in metadata. + // This is important because the VirtualMemoryTracker API doesn't require supplying the tag for some operations. + if (use_tag_inplace) { + assert(leqA_n->val().out.type() != StateType::Released, "Should not use inplace the tag of a released region"); + MemTag tag = leqA_n->val().out.mem_tag(); + stA.out.set_tag(tag); + stB.in.set_tag(tag); + } + // Unless we know better, let B's outgoing state be the outgoing state of the node at or preceding A. // Consider the case where the found node is the start of a region enclosing [A,B) stB.out = leqA_n->val().out; diff --git a/src/hotspot/share/nmt/vmatree.hpp b/src/hotspot/share/nmt/vmatree.hpp index 55399e51b9d22..cfb3c8ab5246d 100644 --- a/src/hotspot/share/nmt/vmatree.hpp +++ b/src/hotspot/share/nmt/vmatree.hpp @@ -66,7 +66,7 @@ class VMATree { return statetype_strings[static_cast(type)]; } - // Each point has some stack and a flag associated with it. + // Each point has some stack and a tag associated with it. struct RegionData { const NativeCallStackStorage::StackIndex stack_idx; const MemTag mem_tag; @@ -88,30 +88,34 @@ class VMATree { struct IntervalState { private: // Store the type and mem_tag as two bytes - uint8_t type_flag[2]; + uint8_t type_tag[2]; NativeCallStackStorage::StackIndex sidx; public: - IntervalState() : type_flag{0,0}, sidx() {} + IntervalState() : type_tag{0,0}, sidx() {} IntervalState(const StateType type, const RegionData data) { assert(!(type == StateType::Released) || data.mem_tag == mtNone, "Released type must have memory tag mtNone"); - type_flag[0] = static_cast(type); - type_flag[1] = static_cast(data.mem_tag); + type_tag[0] = static_cast(type); + type_tag[1] = static_cast(data.mem_tag); sidx = data.stack_idx; } StateType type() const { - return static_cast(type_flag[0]); + return static_cast(type_tag[0]); } MemTag mem_tag() const { - return static_cast(type_flag[1]); + return static_cast(type_tag[1]); } RegionData regiondata() const { return RegionData{sidx, mem_tag()}; } + void set_tag(MemTag tag) { + type_tag[1] = static_cast(tag); + } + NativeCallStackStorage::StackIndex stack() const { return sidx; } @@ -167,14 +171,20 @@ class VMATree { } }; - SummaryDiff register_mapping(position A, position B, StateType state, const RegionData& metadata); + private: + SummaryDiff register_mapping(position A, position B, StateType state, const RegionData& metadata, bool use_tag_inplace = false); + public: SummaryDiff reserve_mapping(position from, position sz, const RegionData& metadata) { - return register_mapping(from, from + sz, StateType::Reserved, metadata); + return register_mapping(from, from + sz, StateType::Reserved, metadata, false); + } + + SummaryDiff commit_mapping(position from, position sz, const RegionData& metadata, bool use_tag_inplace = false) { + return register_mapping(from, from + sz, StateType::Committed, metadata, use_tag_inplace); } - SummaryDiff commit_mapping(position from, position sz, const RegionData& metadata) { - return register_mapping(from, from + sz, StateType::Committed, metadata); + SummaryDiff uncommit_mapping(position from, position sz, const RegionData& metadata) { + return register_mapping(from, from + sz, StateType::Reserved, metadata, true); } SummaryDiff release_mapping(position from, position sz) { diff --git a/test/hotspot/gtest/nmt/test_vmatree.cpp b/test/hotspot/gtest/nmt/test_vmatree.cpp index 08b4340ae4fb7..7a5a98b786305 100644 --- a/test/hotspot/gtest/nmt/test_vmatree.cpp +++ b/test/hotspot/gtest/nmt/test_vmatree.cpp @@ -171,7 +171,6 @@ class NMTVMATreeTest : public testing::Test { }; - TEST_VM_F(NMTVMATreeTest, OverlappingReservationsResultInTwoNodes) { VMATree::RegionData rd{si[0], mtTest}; Tree tree; @@ -181,6 +180,23 @@ TEST_VM_F(NMTVMATreeTest, OverlappingReservationsResultInTwoNodes) { EXPECT_EQ(2, count_nodes(tree)); } +TEST_VM_F(NMTVMATreeTest, UseFlagInplace) { + Tree tree; + VMATree::RegionData rd1(si[0], mtTest); + VMATree::RegionData rd2(si[1], mtNone); + tree.reserve_mapping(0, 100, rd1); + tree.commit_mapping(20, 50, rd2, true); + tree.uncommit_mapping(30, 10, rd2); + tree.visit_in_order([&](Node* node) { + if (node->key() != 100) { + EXPECT_EQ(mtTest, node->val().out.mem_tag()) << "failed at: " << node->key(); + if (node->key() != 20 && node->key() != 40) { + EXPECT_EQ(VMATree::StateType::Reserved, node->val().out.type()); + } + } + }); +} + // Low-level tests inspecting the state of the tree. TEST_VM_F(NMTVMATreeTest, LowLevel) { adjacent_2_nodes(VMATree::empty_regiondata);