From aa2eb418f1108cb267523d8ec85c3d5a5e58e6f2 Mon Sep 17 00:00:00 2001 From: Calvin Neo Date: Thu, 27 Jun 2024 18:52:41 +0800 Subject: [PATCH] Storages: Fix memory trace for UniversalPageId (#9163) (#226) Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Co-authored-by: JaySon --- .../Page/V3/Universal/UniversalPageId.cpp | 41 ++++++++++++++++++- .../Page/V3/Universal/UniversalPageId.h | 27 +++--------- .../tests/gtest_universal_page_storage.cpp | 20 +++++++++ 3 files changed, 66 insertions(+), 22 deletions(-) diff --git a/dbms/src/Storages/Page/V3/Universal/UniversalPageId.cpp b/dbms/src/Storages/Page/V3/Universal/UniversalPageId.cpp index b6443c2a8ae..931b2ebabaf 100644 --- a/dbms/src/Storages/Page/V3/Universal/UniversalPageId.cpp +++ b/dbms/src/Storages/Page/V3/Universal/UniversalPageId.cpp @@ -15,6 +15,45 @@ #include #include +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 \ No newline at end of file +} // namespace DB::details diff --git a/dbms/src/Storages/Page/V3/Universal/UniversalPageId.h b/dbms/src/Storages/Page/V3/Universal/UniversalPageId.h index 193092c255e..726b89c9ea1 100644 --- 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 index a538610f3ac..06865d4cdee 100644 --- 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