Type erase clear function in order to move more code to cpp file and speed up compilation. PiperOrigin-RevId: 928631524 Change-Id: I43389680617c39d0fa3edd5ff97576756a826aee
diff --git a/absl/container/internal/raw_hash_set.cc b/absl/container/internal/raw_hash_set.cc index 69a411d..dc58245 100644 --- a/absl/container/internal/raw_hash_set.cc +++ b/absl/container/internal/raw_hash_set.cc
@@ -334,6 +334,27 @@ } // namespace +void CommonFields::AssertNotDebugCapacityImpl() const { + const HashtableCapacity cap = maybe_invalid_capacity(); + if (ABSL_PREDICT_TRUE(cap.IsValid())) { + return; + } + assert(!cap.IsReentrance() && + "Reentrant container access during element construction/destruction " + "is not allowed."); + if (cap.IsDestroyed()) { + ABSL_RAW_LOG(FATAL, "Use of destroyed hash table."); + } + if (SwisstableGenerationsEnabled() && ABSL_PREDICT_FALSE(cap.IsMovedFrom())) { + if (cap.IsSelfMovedFrom()) { + // If this log triggers, then a hash table was move-assigned to itself + // and then used again later without being reinitialized. + ABSL_RAW_LOG(FATAL, "Use of self-move-assigned hash table."); + } + ABSL_RAW_LOG(FATAL, "Use of moved-from hash table."); + } +} + void GrowthInfoAccessor::InitGrowthLeftNoDeleted(size_t growth_left, size_t capacity) { if (capacity <= GrowthInfoLowerBound::kMaxGrowthLeftLowerBound) { @@ -785,6 +806,20 @@ : CommonFields{non_soo_tag_t{}}; } +template <bool kSooEnabled> +void* SingleSlotAddress(CommonFields& c) { + return kSooEnabled ? c.soo_data() : c.slot_array(); +} + +template <bool kSooEnabled> +void DecrementSmallSize(CommonFields& c) { + if constexpr (kSooEnabled) { + c.set_empty_soo(); + } else { + c.decrement_size(); + } +} + } // namespace void EraseMetaOnlySmall(CommonFields& c, bool soo_enabled, size_t slot_size) { @@ -840,7 +875,7 @@ auto destroy_slot_wrapper = [&](const ctrl_t*, void* slot) { destroy_slot(&c, slot); }; - if constexpr (SwisstableAssertAccessToDestroyedTable()) { + if constexpr (SwisstableGenerationsOrDebugEnabled()) { CommonFields common_copy(non_soo_tag_t{}, c); c.set_capacity(HashtableCapacity::CreateDestroyed()); IterateOverFullSlotsImpl(common_copy, slot_size, destroy_slot_wrapper); @@ -858,6 +893,42 @@ c.blocked_element_count()); } +template <bool kSooEnabled> +void Clear(CommonFields& c, const PolicyFunctions& __restrict policy, + DestroySlotFn destroy_slot, void* alloc) { + if (SwisstableGenerationsEnabled() && + c.maybe_invalid_capacity().IsMovedFrom()) { + c.set_capacity(policy.soo_capacity()); + } + c.AssertNotDebugCapacity(); + const size_t cap = c.capacity(); + if constexpr (kSooEnabled) { + ABSL_ASSUME(cap > 0); + } + if (c.is_small()) { + if (!c.empty()) { + if (destroy_slot != nullptr) { + destroy_slot(&c, SingleSlotAddress<kSooEnabled>(c)); + } + DecrementSmallSize<kSooEnabled>(c); + } + } else { + if (destroy_slot != nullptr) { + DestroySlots(c, policy.slot_size, destroy_slot); + } + // Iterating over this container is O(bucket_count()). When bucket_count() + // is much greater than size(), iteration becomes prohibitively expensive. + // For clear() it is more important to reuse the allocated array when the + // container is small because allocation takes comparatively long time + // compared to destruction of the elements of the container. So we pick the + // largest bucket_count() threshold for which iteration is still fast and + // past that we simply deallocate the array. + ClearBackingArray(c, policy, alloc, /*reuse=*/cap < 128); + } + c.set_reserved_growth(0); + c.set_reservation_size(0); +} + void DestructSoo(CommonFields& c, size_t slot_size, size_t slot_align, DestroySlotFn destroy_slot, DeallocBackingArrayFn dealloc, void* alloc) { @@ -1073,12 +1144,12 @@ }; void AssertSoo([[maybe_unused]] CommonFields& common, - [[maybe_unused]] const PolicyFunctions& policy) { + [[maybe_unused]] const PolicyFunctions& __restrict policy) { ABSL_SWISSTABLE_ASSERT(policy.soo_enabled); ABSL_SWISSTABLE_ASSERT(common.capacity() == policy.soo_capacity()); } void AssertFullSoo([[maybe_unused]] CommonFields& common, - [[maybe_unused]] const PolicyFunctions& policy) { + [[maybe_unused]] const PolicyFunctions& __restrict policy) { AssertSoo(common, policy); ABSL_SWISSTABLE_ASSERT(common.size() == policy.soo_capacity()); } @@ -2286,7 +2357,7 @@ } size_t PrepareInsertLargeGenerationsEnabled( - CommonFields& common, const PolicyFunctions& policy, size_t hash, + CommonFields& common, const PolicyFunctions& __restrict policy, size_t hash, Group::NonIterableBitMaskType mask_empty, FindInfo target_group, absl::FunctionRef<size_t(size_t)> recompute_hash) { // NOLINTNEXTLINE(misc-static-assert) @@ -2377,6 +2448,11 @@ void* alloc, size_t capacity, ctrl_t* ctrl, size_t slot_size, size_t slot_align, bool had_infoz, size_t blocked_element_count); +template void Clear<true>(CommonFields& c, const PolicyFunctions& policy, + DestroySlotFn destroy_slot, void* alloc); +template void Clear<false>(CommonFields& c, const PolicyFunctions& policy, + DestroySlotFn destroy_slot, void* alloc); + } // namespace container_internal ABSL_NAMESPACE_END } // namespace absl
diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index befc8a5..b99d987 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h
@@ -280,9 +280,7 @@ constexpr size_t NumGenerationBytes() { return 0; } #endif -// Returns true if we should assert that the table is not accessed after it has -// been destroyed or during the destruction of the table. -constexpr bool SwisstableAssertAccessToDestroyedTable() { +constexpr bool SwisstableGenerationsOrDebugEnabled() { #ifndef NDEBUG return true; #endif @@ -1434,6 +1432,14 @@ set_capacity(cap); } + // Asserts that the capacity is not a sentinel invalid value. + void AssertNotDebugCapacity() const { + if (!SwisstableGenerationsOrDebugEnabled()) { + return; + } + AssertNotDebugCapacityImpl(); + } + private: // We store the has_infoz bit in the lowest bit of size_. static constexpr size_t HasInfozShift() { return 1; } @@ -1448,6 +1454,8 @@ ABSL_SWISSTABLE_ASSERT(!has_infoz()); } + void AssertNotDebugCapacityImpl() const; + // TODO(b/289225379): we could put size_ into HeapOrSoo and make capacity_ // encode the size in SOO case. We would be making size()/capacity() more // expensive in order to have more SOO space. @@ -2057,6 +2065,11 @@ void DeallocBackingArray(CommonFields& c, size_t slot_size, size_t slot_align, DeallocBackingArrayFn dealloc, void* alloc); +// Type erased version of raw_hash_set::clear. +template <bool kSooEnabled> +void Clear(CommonFields& c, const PolicyFunctions& policy, + DestroySlotFn destroy_slot, void* alloc); + // NOTE: Destruct* functions couldn't use PolicyFunctions in order to support // incomplete types. // TODO(b/515666499): try to use PolicyFunctions since it makes code simpler and @@ -2692,7 +2705,7 @@ ~raw_hash_set() { destructor_impl(); - if constexpr (SwisstableAssertAccessToDestroyedTable()) { + if constexpr (SwisstableGenerationsOrDebugEnabled()) { common().set_capacity(HashtableCapacity::CreateDestroyed()); } } @@ -2743,34 +2756,9 @@ } size_t max_size() const { return MaxValidSize(); } - // TODO(b/515666499): Type erase clear(). ABSL_ATTRIBUTE_REINITIALIZES void clear() { - if (SwisstableGenerationsEnabled() && - maybe_invalid_capacity().IsMovedFrom()) { - common().set_capacity(DefaultCapacity()); - } - AssertNotDebugCapacity(); - // Iterating over this container is O(bucket_count()). When bucket_count() - // is much greater than size(), iteration becomes prohibitively expensive. - // For clear() it is more important to reuse the allocated array when the - // container is small because allocation takes comparatively long time - // compared to destruction of the elements of the container. So we pick the - // largest bucket_count() threshold for which iteration is still fast and - // past that we simply deallocate the array. - const size_t cap = capacity(); - if (cap == 0) { - // Already guaranteed to be empty; so nothing to do. - } else if (is_small()) { - if (!empty()) { - destroy(single_slot()); - decrement_small_size(); - } - } else { - destroy_slots(); - clear_backing_array(/*reuse=*/cap < 128); - } - common().set_reserved_growth(0); - common().set_reservation_size(0); + Clear<SooEnabled()>(common(), GetPolicyFunctions(), get_destroy_slot_fn(), + &char_alloc_ref()); } // This overload kicks in when the argument is an rvalue of insertable and @@ -3692,34 +3680,7 @@ } // Asserts that the capacity is not a sentinel invalid value. - void AssertNotDebugCapacity() const { -#ifdef NDEBUG - if (!SwisstableGenerationsEnabled()) { - return; - } -#endif - const HashtableCapacity cap = maybe_invalid_capacity(); - if (ABSL_PREDICT_TRUE(cap.IsValid())) { - return; - } - assert(!cap.IsReentrance() && - "Reentrant container access during element construction/destruction " - "is not allowed."); - if constexpr (SwisstableAssertAccessToDestroyedTable()) { - if (cap.IsDestroyed()) { - ABSL_RAW_LOG(FATAL, "Use of destroyed hash table."); - } - } - if (SwisstableGenerationsEnabled() && - ABSL_PREDICT_FALSE(cap.IsMovedFrom())) { - if (cap.IsSelfMovedFrom()) { - // If this log triggers, then a hash table was move-assigned to itself - // and then used again later without being reinitialized. - ABSL_RAW_LOG(FATAL, "Use of self-move-assigned hash table."); - } - ABSL_RAW_LOG(FATAL, "Use of moved-from hash table."); - } - } + void AssertNotDebugCapacity() const { common().AssertNotDebugCapacity(); } // Asserts that hash and equal functors provided by the user are consistent, // meaning that `eq(k1, k2)` implies `hash(k1)==hash(k2)`. @@ -4162,6 +4123,12 @@ void* alloc, size_t capacity, ctrl_t* ctrl, size_t slot_size, size_t slot_align, bool had_infoz, size_t blocked_element_count); +extern template void Clear<true>(CommonFields& c, const PolicyFunctions& policy, + DestroySlotFn destroy_slot, void* alloc); +extern template void Clear<false>(CommonFields& c, + const PolicyFunctions& policy, + DestroySlotFn destroy_slot, void* alloc); + } // namespace container_internal ABSL_NAMESPACE_END } // namespace absl
diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index c7241c4..70a5952 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc
@@ -849,7 +849,8 @@ } } -template <class T, bool kTransferable = false, bool kSoo = false> +template <class T, bool kTransferable = false, bool kSoo = false, + bool kDestroyTrivial = false> struct ValuePolicy { using slot_type = T; using key_type = T; @@ -866,8 +867,13 @@ } template <class Allocator> - static void destroy(Allocator* alloc, slot_type* slot) { + static auto destroy(Allocator* alloc, slot_type* slot) { std::allocator_traits<Allocator>::destroy(*alloc, slot); + if constexpr (kDestroyTrivial) { + return std::true_type{}; + } else { + return std::false_type{}; + } } template <class Allocator> @@ -1061,10 +1067,10 @@ }; template <typename T, bool kTransferable = false, bool kSoo = false, - class Alloc = std::allocator<T>> -struct ValueTable : InstantiateRawHashSet<ValuePolicy<T, kTransferable, kSoo>, - hash_default_hash<T>, - std::equal_to<T>, Alloc>::type { + bool kDestroyTrivial = false, class Alloc = std::allocator<T>> +struct ValueTable : InstantiateRawHashSet< + ValuePolicy<T, kTransferable, kSoo, kDestroyTrivial>, + hash_default_hash<T>, std::equal_to<T>, Alloc>::type { using Base = typename ValueTable::raw_hash_set; using Base::Base; }; @@ -1202,16 +1208,24 @@ using NonSooIntTableSlotType = SizedValue<kNonSooSize>; static_assert(sizeof(NonSooIntTableSlotType) >= kNonSooSize, "too small"); using NonSooIntTable = ValueTable<NonSooIntTableSlotType>; +using NonSooIntTableTrivialDestroy = + ValueTable<NonSooIntTableSlotType, /*kTransferable=*/false, /*kSoo=*/false, + /*kDestroyTrivial=*/true>; using SooInt32Table = ValueTable<int32_t, /*kTransferable=*/true, /*kSoo=*/true>; using SooIntTable = ValueTable<int64_t, /*kTransferable=*/true, /*kSoo=*/true>; +using SooIntTableTrivialDestroy = + ValueTable<int64_t, /*kTransferable=*/true, /*kSoo=*/true, + /*kDestroyTrivial=*/true>; using NonMemcpyableSooIntTable = ValueTable<int64_t, /*kTransferable=*/false, /*kSoo=*/true>; using MemcpyableSooIntCustomAllocTable = ValueTable<int64_t, /*kTransferable=*/true, /*kSoo=*/true, + /*kDestroyTrivial=*/false, ChangingSizeAndTrackingTypeAlloc<int64_t>>; using NonMemcpyableSooIntCustomAllocTable = ValueTable<int64_t, /*kTransferable=*/false, /*kSoo=*/true, + /*kDestroyTrivial=*/false, ChangingSizeAndTrackingTypeAlloc<int64_t>>; TEST(Table, EmptyFunctorOptimization) { @@ -1303,7 +1317,8 @@ class SooTest : public testing::Test {}; using SooTableTypes = - ::testing::Types<SooIntTable, NonSooIntTable, NonMemcpyableSooIntTable, + ::testing::Types<SooIntTable, SooIntTableTrivialDestroy, NonSooIntTable, + NonSooIntTableTrivialDestroy, NonMemcpyableSooIntTable, MemcpyableSooIntCustomAllocTable, NonMemcpyableSooIntCustomAllocTable>; TYPED_TEST_SUITE(SooTest, SooTableTypes); @@ -4769,13 +4784,7 @@ } TEST(Table, DestroyedCallsFail) { -#ifdef NDEBUG - ASSERT_EQ(SwisstableAssertAccessToDestroyedTable(), - SwisstableGenerationsEnabled()); -#else - ASSERT_TRUE(SwisstableAssertAccessToDestroyedTable()); -#endif - if (!SwisstableAssertAccessToDestroyedTable()) { + if (!SwisstableGenerationsOrDebugEnabled()) { GTEST_SKIP() << "Validation not enabled."; } #if !defined(__clang__) && defined(__GNUC__) @@ -4798,7 +4807,7 @@ } TEST(Table, DestroyedCallsFailDuringDestruction) { - if (!SwisstableAssertAccessToDestroyedTable()) { + if (!SwisstableGenerationsOrDebugEnabled()) { GTEST_SKIP() << "Validation not enabled."; } #if !defined(__clang__) && defined(__GNUC__)