From 384ff82a1bd4c1329858e22b7f0af3678e1b0015 Mon Sep 17 00:00:00 2001 From: Kazu Hirata Date: Tue, 2 Sep 2025 10:46:03 -0700 Subject: [PATCH] [ADT] Skip DenseMapBase::destroyAll on trivially destructible types DenseMap::destroyAll currently iterates through the entire bucket array to call destructors keys and values. We don't need to do that if we know that both key and value types are trivially destructible, meaning that the destructors are no-ops. This patch introduces "constexpr if" at the beginning of destroyAll to skip the rest of the function if both key and value types are trivially destructible. --- llvm/include/llvm/ADT/DenseMap.h | 6 +++ llvm/unittests/ADT/DenseMapTest.cpp | 70 +++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h index 25b5262800a10..3d91defe2f98d 100644 --- a/llvm/include/llvm/ADT/DenseMap.h +++ b/llvm/include/llvm/ADT/DenseMap.h @@ -353,6 +353,12 @@ class DenseMapBase : public DebugEpochBase { DenseMapBase() = default; void destroyAll() { + // No need to iterate through the buckets if both KeyT and ValueT are + // trivially destructible. + if constexpr (std::is_trivially_destructible_v && + std::is_trivially_destructible_v) + return; + if (getNumBuckets() == 0) // Nothing to do. return; diff --git a/llvm/unittests/ADT/DenseMapTest.cpp b/llvm/unittests/ADT/DenseMapTest.cpp index 50e9c6e138ef1..aceb4f30d878d 100644 --- a/llvm/unittests/ADT/DenseMapTest.cpp +++ b/llvm/unittests/ADT/DenseMapTest.cpp @@ -70,6 +70,16 @@ class CtorTester { int getValue() const { return Value; } bool operator==(const CtorTester &RHS) const { return Value == RHS.Value; } + + // Return the number of live CtorTester objects, excluding the empty and + // tombstone keys. + static size_t getNumConstructed() { + return std::count_if(Constructed.begin(), Constructed.end(), + [](const CtorTester *Obj) { + int V = Obj->getValue(); + return V != -1 && V != -2; + }); + } }; std::set CtorTester::Constructed; @@ -1031,4 +1041,64 @@ TEST(SmallDenseMapCustomTest, InitSize) { } } +TEST(DenseMapCustomTest, KeyDtor) { + // This test relies on CtorTester being non-trivially destructible. + static_assert(!std::is_trivially_destructible_v, + "CtorTester must not be trivially destructible"); + + // Test that keys are destructed on scope exit. + EXPECT_EQ(0u, CtorTester::getNumConstructed()); + { + DenseMap Map; + Map.try_emplace(CtorTester(0), 1); + Map.try_emplace(CtorTester(1), 2); + EXPECT_EQ(2u, CtorTester::getNumConstructed()); + } + EXPECT_EQ(0u, CtorTester::getNumConstructed()); + + // Test that keys are destructed on erase and shrink_and_clear. + EXPECT_EQ(0u, CtorTester::getNumConstructed()); + { + DenseMap Map; + Map.try_emplace(CtorTester(0), 1); + Map.try_emplace(CtorTester(1), 2); + EXPECT_EQ(2u, CtorTester::getNumConstructed()); + Map.erase(CtorTester(1)); + EXPECT_EQ(1u, CtorTester::getNumConstructed()); + Map.shrink_and_clear(); + EXPECT_EQ(0u, CtorTester::getNumConstructed()); + } + EXPECT_EQ(0u, CtorTester::getNumConstructed()); +} + +TEST(DenseMapCustomTest, ValueDtor) { + // This test relies on CtorTester being non-trivially destructible. + static_assert(!std::is_trivially_destructible_v, + "CtorTester must not be trivially destructible"); + + // Test that values are destructed on scope exit. + EXPECT_EQ(0u, CtorTester::getNumConstructed()); + { + DenseMap Map; + Map.try_emplace(0, CtorTester(1)); + Map.try_emplace(1, CtorTester(2)); + EXPECT_EQ(2u, CtorTester::getNumConstructed()); + } + EXPECT_EQ(0u, CtorTester::getNumConstructed()); + + // Test that values are destructed on erase and shrink_and_clear. + EXPECT_EQ(0u, CtorTester::getNumConstructed()); + { + DenseMap Map; + Map.try_emplace(0, CtorTester(1)); + Map.try_emplace(1, CtorTester(2)); + EXPECT_EQ(2u, CtorTester::getNumConstructed()); + Map.erase(1); + EXPECT_EQ(1u, CtorTester::getNumConstructed()); + Map.shrink_and_clear(); + EXPECT_EQ(0u, CtorTester::getNumConstructed()); + } + EXPECT_EQ(0u, CtorTester::getNumConstructed()); +} + } // namespace