Skip to content

Commit 54615fe

Browse files
goldvitalycopybara-github
authored andcommitted
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
1 parent e9f87bc commit 54615fe

3 files changed

Lines changed: 55 additions & 19 deletions

File tree

absl/container/internal/raw_hash_set.cc

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,24 @@ ABSL_ATTRIBUTE_ALWAYS_INLINE inline void InitializeThreeElementsControlBytes(
616616
// new_ctrl after 2nd store = EHNSEHNEEEE
617617
}
618618

619+
// ClearBackingArrayNoReuse clears the backing array and sets the common
620+
// fields to the default values for empty non-allocated tables.
621+
// REQUIRES: c.capacity > policy.soo_capacity.
622+
void ClearBackingArrayNoReuse(CommonFields& c,
623+
const PolicyFunctions& __restrict policy,
624+
void* alloc) {
625+
ABSL_SWISSTABLE_ASSERT(c.capacity() > policy.soo_capacity());
626+
// We need to record infoz before calling dealloc, which will unregister
627+
// infoz.
628+
c.infoz().RecordClearedReservation();
629+
c.infoz().RecordStorageChanged(0, policy.soo_capacity());
630+
c.infoz().Unregister();
631+
(*policy.dealloc)(alloc, c.capacity(), c.control(), policy.slot_size,
632+
policy.slot_align, c.has_infoz());
633+
c = policy.soo_enabled ? CommonFields{soo_tag_t{}}
634+
: CommonFields{non_soo_tag_t{}};
635+
}
636+
619637
} // namespace
620638

621639
void EraseMetaOnlySmall(CommonFields& c, bool soo_enabled, size_t slot_size) {
@@ -649,22 +667,15 @@ void EraseMetaOnlyLarge(CommonFields& c, const ctrl_t* ctrl, size_t slot_size) {
649667

650668
void ClearBackingArray(CommonFields& c,
651669
const PolicyFunctions& __restrict policy, void* alloc,
652-
bool reuse, bool soo_enabled) {
670+
bool reuse) {
671+
ABSL_SWISSTABLE_ASSERT(c.capacity() > MaxSmallCapacity());
653672
if (reuse) {
654673
c.set_size_to_zero();
655-
ABSL_SWISSTABLE_ASSERT(!soo_enabled || c.capacity() > SooCapacity());
656674
ResetCtrl(c, policy.slot_size);
657675
ResetGrowthLeft(c);
658676
c.infoz().RecordStorageChanged(0, c.capacity());
659677
} else {
660-
// We need to record infoz before calling dealloc, which will unregister
661-
// infoz.
662-
c.infoz().RecordClearedReservation();
663-
c.infoz().RecordStorageChanged(0, soo_enabled ? SooCapacity() : 0);
664-
c.infoz().Unregister();
665-
(*policy.dealloc)(alloc, c.capacity(), c.control(), policy.slot_size,
666-
policy.slot_align, c.has_infoz());
667-
c = soo_enabled ? CommonFields{soo_tag_t{}} : CommonFields{non_soo_tag_t{}};
678+
ClearBackingArrayNoReuse(c, policy, alloc);
668679
}
669680
}
670681

@@ -1835,8 +1846,7 @@ void Rehash(CommonFields& common, const PolicyFunctions& __restrict policy,
18351846
const size_t cap = common.capacity();
18361847

18371848
auto clear_backing_array = [&]() {
1838-
ClearBackingArray(common, policy, policy.get_char_alloc(common),
1839-
/*reuse=*/false, policy.soo_enabled);
1849+
ClearBackingArrayNoReuse(common, policy, policy.get_char_alloc(common));
18401850
};
18411851

18421852
const size_t slot_size = policy.slot_size;

absl/container/internal/raw_hash_set.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1938,9 +1938,9 @@ void ResizeAllocatedTableWithSeedChange(CommonFields& common,
19381938

19391939
// ClearBackingArray clears the backing array, either modifying it in place,
19401940
// or creating a new one based on the value of "reuse".
1941-
// REQUIRES: c.capacity > 0
1941+
// REQUIRES: c.capacity > MaxSmallCapacity().
19421942
void ClearBackingArray(CommonFields& c, const PolicyFunctions& policy,
1943-
void* alloc, bool reuse, bool soo_enabled);
1943+
void* alloc, bool reuse);
19441944

19451945
// Type-erased versions of raw_hash_set::erase_meta_only_{small,large}.
19461946
void EraseMetaOnlySmall(CommonFields& c, bool soo_enabled, size_t slot_size);
@@ -2917,8 +2917,8 @@ class raw_hash_set {
29172917
iterator erase(const_iterator first,
29182918
const_iterator last) ABSL_ATTRIBUTE_LIFETIME_BOUND {
29192919
AssertNotDebugCapacity();
2920-
// We check for empty first because clear_backing_array requires that
2921-
// capacity() > 0 as a precondition.
2920+
// We check for empty and for is_small because clear_backing_array requires
2921+
// that capacity() > MaxSmallCapacity() as a precondition.
29222922
if (empty()) return end();
29232923
if (first == last) return last.inner_;
29242924
if (is_small()) {
@@ -3273,9 +3273,8 @@ class raw_hash_set {
32733273
}
32743274

32753275
void clear_backing_array(bool reuse) {
3276-
ABSL_SWISSTABLE_ASSERT(capacity() > DefaultCapacity());
3277-
ClearBackingArray(common(), GetPolicyFunctions(), &char_alloc_ref(), reuse,
3278-
SooEnabled());
3276+
ABSL_SWISSTABLE_ASSERT(capacity() > MaxSmallCapacity());
3277+
ClearBackingArray(common(), GetPolicyFunctions(), &char_alloc_ref(), reuse);
32793278
}
32803279

32813280
void destroy_slots() {

absl/container/internal/raw_hash_set_test.cc

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,6 +1165,33 @@ TYPED_TEST(SooTest, InsertWithinCapacity) {
11651165
EXPECT_THAT(addr(0), original_addr_0);
11661166
}
11671167

1168+
TYPED_TEST(SooTest, ClearDifferentSizes) {
1169+
for (size_t size = 0; size < 32; ++size) {
1170+
for (bool reserve : {false, true}) {
1171+
for (bool clear_via_erase : {false, true}) {
1172+
SCOPED_TRACE(absl::StrCat("size: ", size, ", reserve: ", reserve,
1173+
", clear_via_erase: ", clear_via_erase));
1174+
TypeParam t;
1175+
if (reserve) {
1176+
t.reserve(size);
1177+
}
1178+
for (size_t i = 0; i < size; ++i) {
1179+
ASSERT_TRUE(t.insert(static_cast<int>(i)).second) << i;
1180+
}
1181+
if (clear_via_erase) {
1182+
t.erase(t.begin(), t.end());
1183+
} else {
1184+
t.clear();
1185+
}
1186+
ASSERT_EQ(t.size(), 0);
1187+
for (size_t i = 0; i < size; ++i) {
1188+
ASSERT_TRUE(t.insert(static_cast<int>(i)).second) << i;
1189+
}
1190+
}
1191+
}
1192+
}
1193+
}
1194+
11681195
template <class TableType>
11691196
class SmallTableResizeTest : public testing::Test {};
11701197

0 commit comments

Comments
 (0)