Storages: Fix memory trace for UniversalPageId (pingcap#9163) (pingca…

…p#226) Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Co-authored-by: JaySon <[email protected]>
JaySon-Huang · Jun 27, 2024 · aa2eb41 · aa2eb41
1 parent e395a9b
commit aa2eb41
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 22 deletions.
diff --git a/dbms/src/Storages/Page/V3/Universal/UniversalPageId.cpp b/dbms/src/Storages/Page/V3/Universal/UniversalPageId.cpp
@@ -15,6 +15,45 @@
 #include <Storages/Page/V3/Universal/UniversalPageId.h>
 #include <Storages/Page/V3/Universal/UniversalPageIdFormatImpl.h>
 
+namespace DB
+{
+UniversalPageId::~UniversalPageId()
+{
+    PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_sub(id.size());
+}
+
+UniversalPageId::UniversalPageId(UniversalPageId && other)
+{
+    // PS::PageStorageMemorySummary::uni_page_id_bytes has been set when `other` created
+    id = std::move(other.id);
+}
+UniversalPageId::UniversalPageId(const UniversalPageId & other)
+{
+    PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_add(other.id.size());
+    id = other.id;
+}
+UniversalPageId & UniversalPageId::operator=(UniversalPageId && other) noexcept
+{
+    PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_sub(id.size());
+    id = std::move(other.id);
+    return *this;
+}
+UniversalPageId & UniversalPageId::operator=(const UniversalPageId & other) noexcept
+{
+    PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_sub(id.size());
+    PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_add(other.id.size());
+    id = other.id;
+    return *this;
+}
+UniversalPageId & UniversalPageId::operator=(String && id_) noexcept
+{
+    PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_sub(id.size());
+    PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_add(id_.size());
+    id.swap(id_);
+    return *this;
+}
+} // namespace DB
+
 namespace DB::details
 {
 
@@ -31,4 +70,4 @@ String UniversalPageIdFormatHelper::format(const DB::UniversalPageId & value)
         DB::UniversalPageIdFormat::getU64ID(value));
 }
 
-} // namespace DB::details
+} // namespace DB::details
diff --git a/dbms/src/Storages/Page/V3/Universal/UniversalPageId.h b/dbms/src/Storages/Page/V3/Universal/UniversalPageId.h
@@ -26,12 +26,8 @@ class UniversalPageId final
 {
 public:
     UniversalPageId() { PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_add(id.size()); }
-    UniversalPageId(const UniversalPageId & other)
-        : id(other.id)
-    {
-        PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_add(id.size());
-    }
-
+    UniversalPageId(const UniversalPageId & other);
+    UniversalPageId(UniversalPageId && other);
     UniversalPageId(String id_) // NOLINT(google-explicit-constructor)
         : id(std::move(id_))
     {
@@ -48,22 +44,11 @@ class UniversalPageId final
         PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_add(id.size());
     }
 
-    ~UniversalPageId() { PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_sub(id.size()); }
+    ~UniversalPageId();
 
-    UniversalPageId & operator=(String && id_) noexcept
-    {
-        if (id.size() == id_.size()) {}
-        else if (id.size() > id_.size())
-        {
-            PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_sub(id.size() - id_.size());
-        }
-        else
-        {
-            PS::PageStorageMemorySummary::uni_page_id_bytes.fetch_add(id_.size() - id.size());
-        }
-        id.swap(id_);
-        return *this;
-    }
+    UniversalPageId & operator=(UniversalPageId && other) noexcept;
+    UniversalPageId & operator=(const UniversalPageId & other) noexcept;
+    UniversalPageId & operator=(String && id_) noexcept;
     bool operator==(const UniversalPageId & rhs) const noexcept { return id == rhs.id; }
     bool operator!=(const UniversalPageId & rhs) const noexcept { return id != rhs.id; }
     bool operator>=(const UniversalPageId & rhs) const noexcept { return id >= rhs.id; }

diff --git a/dbms/src/Storages/Page/V3/Universal/tests/gtest_universal_page_storage.cpp b/dbms/src/Storages/Page/V3/Universal/tests/gtest_universal_page_storage.cpp
@@ -561,5 +561,25 @@ TEST(UniPageStorageIdTest, UniversalPageId)
     }
 }
 
+TEST(UniPageStorageIdTest, UniversalPageIdMemoryTrace)
+{
+    auto prim_mem = PS::PageStorageMemorySummary::uni_page_id_bytes.load();
+    {
+        auto u_id = UniversalPageIdFormat::toFullPageId("aaa", 100);
+        auto page1_mem = PS::PageStorageMemorySummary::uni_page_id_bytes.load();
+        auto ps = page1_mem - prim_mem;
+        auto u_id_cpy = u_id;
+        ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 2);
+        UniversalPageId u_id_mv = UniversalPageIdFormat::toFullPageId("aaa", 100);
+        ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 3);
+        u_id_mv = std::move(u_id_cpy);
+        ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 2);
+        UniversalPageId u_id_cpy2 = UniversalPageIdFormat::toFullPageId("aaa", 100);
+        u_id_cpy2 = u_id_mv;
+        ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem + ps * 3);
+    }
+    ASSERT_EQ(PS::PageStorageMemorySummary::uni_page_id_bytes.load(), prim_mem);
+}
+
 } // namespace PS::universal::tests
 } // namespace DB