Refactor ClearBackingArray and add test for clearing with different sizes. This change refactors the "no reuse" logic from ClearBackingArray into a new helper function ClearBackingArrayNoReuse. It also clarifies that ClearBackingArray is only applicable to tables with capacity greater than MaxSmallCapacity(), adding asserts and checks to enforce this. PiperOrigin-RevId: 920163391 Change-Id: I22b5fe24d796b00fc1231d4e42720d58a2a7aa60
diff --git a/absl/container/internal/raw_hash_set.cc b/absl/container/internal/raw_hash_set.cc index ef680e7..6ab06e4 100644 --- a/absl/container/internal/raw_hash_set.cc +++ b/absl/container/internal/raw_hash_set.cc
@@ -616,6 +616,24 @@ // new_ctrl after 2nd store = EHNSEHNEEEE } +// ClearBackingArrayNoReuse clears the backing array and sets the common +// fields to the default values for empty non-allocated tables. +// REQUIRES: c.capacity > policy.soo_capacity. +void ClearBackingArrayNoReuse(CommonFields& c, + const PolicyFunctions& __restrict policy, + void* alloc) { + ABSL_SWISSTABLE_ASSERT(c.capacity() > policy.soo_capacity()); + // We need to record infoz before calling dealloc, which will unregister + // infoz. + c.infoz().RecordClearedReservation(); + c.infoz().RecordStorageChanged(0, policy.soo_capacity()); + c.infoz().Unregister(); + (*policy.dealloc)(alloc, c.capacity(), c.control(), policy.slot_size, + policy.slot_align, c.has_infoz()); + c = policy.soo_enabled ? CommonFields{soo_tag_t{}} + : CommonFields{non_soo_tag_t{}}; +} + } // namespace void EraseMetaOnlySmall(CommonFields& c, bool soo_enabled, size_t slot_size) { @@ -649,22 +667,15 @@ void ClearBackingArray(CommonFields& c, const PolicyFunctions& __restrict policy, void* alloc, - bool reuse, bool soo_enabled) { + bool reuse) { + ABSL_SWISSTABLE_ASSERT(c.capacity() > MaxSmallCapacity()); if (reuse) { c.set_size_to_zero(); - ABSL_SWISSTABLE_ASSERT(!soo_enabled || c.capacity() > SooCapacity()); ResetCtrl(c, policy.slot_size); ResetGrowthLeft(c); c.infoz().RecordStorageChanged(0, c.capacity()); } else { - // We need to record infoz before calling dealloc, which will unregister - // infoz. - c.infoz().RecordClearedReservation(); - c.infoz().RecordStorageChanged(0, soo_enabled ? SooCapacity() : 0); - c.infoz().Unregister(); - (*policy.dealloc)(alloc, c.capacity(), c.control(), policy.slot_size, - policy.slot_align, c.has_infoz()); - c = soo_enabled ? CommonFields{soo_tag_t{}} : CommonFields{non_soo_tag_t{}}; + ClearBackingArrayNoReuse(c, policy, alloc); } } @@ -1835,8 +1846,7 @@ const size_t cap = common.capacity(); auto clear_backing_array = [&]() { - ClearBackingArray(common, policy, policy.get_char_alloc(common), - /*reuse=*/false, policy.soo_enabled); + ClearBackingArrayNoReuse(common, policy, policy.get_char_alloc(common)); }; const size_t slot_size = policy.slot_size;
diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 07ffe61..a50a488 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h
@@ -1938,9 +1938,9 @@ // ClearBackingArray clears the backing array, either modifying it in place, // or creating a new one based on the value of "reuse". -// REQUIRES: c.capacity > 0 +// REQUIRES: c.capacity > MaxSmallCapacity(). void ClearBackingArray(CommonFields& c, const PolicyFunctions& policy, - void* alloc, bool reuse, bool soo_enabled); + void* alloc, bool reuse); // Type-erased versions of raw_hash_set::erase_meta_only_{small,large}. void EraseMetaOnlySmall(CommonFields& c, bool soo_enabled, size_t slot_size); @@ -2917,8 +2917,8 @@ iterator erase(const_iterator first, const_iterator last) ABSL_ATTRIBUTE_LIFETIME_BOUND { AssertNotDebugCapacity(); - // We check for empty first because clear_backing_array requires that - // capacity() > 0 as a precondition. + // We check for empty and for is_small because clear_backing_array requires + // that capacity() > MaxSmallCapacity() as a precondition. if (empty()) return end(); if (first == last) return last.inner_; if (is_small()) { @@ -3273,9 +3273,8 @@ } void clear_backing_array(bool reuse) { - ABSL_SWISSTABLE_ASSERT(capacity() > DefaultCapacity()); - ClearBackingArray(common(), GetPolicyFunctions(), &char_alloc_ref(), reuse, - SooEnabled()); + ABSL_SWISSTABLE_ASSERT(capacity() > MaxSmallCapacity()); + ClearBackingArray(common(), GetPolicyFunctions(), &char_alloc_ref(), reuse); } void destroy_slots() {
diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index 176fefb..6d63a06 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc
@@ -1165,6 +1165,33 @@ EXPECT_THAT(addr(0), original_addr_0); } +TYPED_TEST(SooTest, ClearDifferentSizes) { + for (size_t size = 0; size < 32; ++size) { + for (bool reserve : {false, true}) { + for (bool clear_via_erase : {false, true}) { + SCOPED_TRACE(absl::StrCat("size: ", size, ", reserve: ", reserve, + ", clear_via_erase: ", clear_via_erase)); + TypeParam t; + if (reserve) { + t.reserve(size); + } + for (size_t i = 0; i < size; ++i) { + ASSERT_TRUE(t.insert(static_cast<int>(i)).second) << i; + } + if (clear_via_erase) { + t.erase(t.begin(), t.end()); + } else { + t.clear(); + } + ASSERT_EQ(t.size(), 0); + for (size_t i = 0; i < size; ++i) { + ASSERT_TRUE(t.insert(static_cast<int>(i)).second) << i; + } + } + } + } +} + template <class TableType> class SmallTableResizeTest : public testing::Test {};