Remove `soo_enabled()` from raw_hash_set policy traits. `PolicyTraits::soo_enabled()` let a policy opt out of small object optimization (SOO). Only test policies in `raw_hash_set_test.cc` used it, and it was easy to confuse with `raw_hash_set::SooEnabled()`. SOO is now enabled purely based on whether the slot fits into `HeapOrSoo`. The `kSoo` template parameter of the test `ValuePolicy`/`ValueTable` is removed. Non-SOO coverage for small capacities is still provided by tables with slots larger than `HeapOrSoo` (e.g. `NonSooIntTable`). PiperOrigin-RevId: 1009588091
diff --git a/absl/container/internal/hash_policy_traits.h b/absl/container/internal/hash_policy_traits.h index acea6b7..0d3939d 100644 --- a/absl/container/internal/hash_policy_traits.h +++ b/absl/container/internal/hash_policy_traits.h
@@ -166,9 +166,6 @@ #endif } - // Whether small object optimization is enabled. True by default. - static constexpr bool soo_enabled() { return soo_enabled_impl(Rank1{}); } - private: template <class Hash, bool kIsAbsl, size_t kSeedShift> static size_t hash_slot_fn_non_type_erased(const void* hash_fn, void* slot, @@ -178,18 +175,6 @@ *static_cast<const Hash*>(hash_fn), seed}, Policy::element(static_cast<slot_type*>(slot))); } - - // Use go/ranked-overloads for dispatching. Rank1 is preferred. - struct Rank0 {}; - struct Rank1 : Rank0 {}; - - // Use auto -> decltype as an enabler. - template <class P = Policy> - static constexpr auto soo_enabled_impl(Rank1) -> decltype(P::soo_enabled()) { - return P::soo_enabled(); - } - - static constexpr bool soo_enabled_impl(Rank0) { return true; } }; } // namespace container_internal
diff --git a/absl/container/internal/hashtablez_sampler.h b/absl/container/internal/hashtablez_sampler.h index 9e9a47c..628acf6 100644 --- a/absl/container/internal/hashtablez_sampler.h +++ b/absl/container/internal/hashtablez_sampler.h
@@ -247,9 +247,11 @@ // It is useful to avoid sampling attempts and sampling delays in tests. void TestOnlyRefreshSamplingStateForCurrentThread(); -// Returns a sampling handle. -inline HashtablezInfoHandle Sample(size_t inline_element_size, size_t key_size, - size_t value_size, uint16_t soo_capacity) { +// Returns a sampling handle. For use in tests only: `raw_hash_set` calls +// `ShouldSampleNextTable()` and `ForcedTrySample()` directly. +inline HashtablezInfoHandle SampleForTesting(size_t inline_element_size, + size_t key_size, size_t value_size, + uint16_t soo_capacity) { if (ABSL_PREDICT_TRUE(!ShouldSampleNextTable())) { return HashtablezInfoHandle(nullptr); }
diff --git a/absl/container/internal/hashtablez_sampler_test.cc b/absl/container/internal/hashtablez_sampler_test.cc index 0d863a8..b61976d 100644 --- a/absl/container/internal/hashtablez_sampler_test.cc +++ b/absl/container/internal/hashtablez_sampler_test.cc
@@ -332,10 +332,10 @@ double sample_rate = 0.0; for (int i = 0; i < 1000000; ++i) { HashtablezInfoHandle h = - Sample(test_element_size, - /*key_size=*/test_key_size, /*value_size=*/test_value_size, - - /*soo_capacity=*/0); + SampleForTesting(test_element_size, + /*key_size=*/test_key_size, + /*value_size=*/test_value_size, + /*soo_capacity=*/0); ++total; if (h.IsSampled()) {
diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index b7fb4d1..6d991fd 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h
@@ -2466,8 +2466,7 @@ kIsAbslHash ? 0 : HashtableInlineData::kSeedShift; constexpr static bool SooEnabled() { - return PolicyTraits::soo_enabled() && - sizeof(slot_type) <= sizeof(HeapOrSoo) && + return sizeof(slot_type) <= sizeof(HeapOrSoo) && alignof(slot_type) <= alignof(HeapOrSoo); } @@ -2897,8 +2896,7 @@ } } that.common() = CommonFields::CreateDefault<SooEnabled()>(); - common().annotate_for_bug_detection_on_move(that.common(), - GetPolicyFunctions()); + annotate_for_bug_detection_on_move(that); } raw_hash_set(raw_hash_set&& that, const allocator_type& a) @@ -2908,8 +2906,7 @@ alloc_(a) { if (CharAlloc(a) == that.char_alloc_ref()) { swap_common(that); - common().annotate_for_bug_detection_on_move(that.common(), - GetPolicyFunctions()); + annotate_for_bug_detection_on_move(that); } else { move_elements_allocs_unequal(std::move(that)); } @@ -3761,6 +3758,15 @@ std::move(tmp)); } + // Only references `GetPolicyFunctions()` when generations are enabled, so + // that the `PolicyFunctions` instance is not kept alive otherwise. + void annotate_for_bug_detection_on_move(raw_hash_set& that) { + if constexpr (SwisstableGenerationsEnabled()) { + common().annotate_for_bug_detection_on_move(that.common(), + GetPolicyFunctions()); + } + } + template <bool propagate_alloc> raw_hash_set& assign_impl(raw_hash_set&& that) { // We don't bother checking for this/that aliasing. We just need to avoid @@ -3773,8 +3779,7 @@ CopyAlloc(char_alloc_ref(), that.char_alloc_ref(), std::bool_constant<propagate_alloc>()); that.common() = CommonFields::CreateDefault<SooEnabled()>(); - common().annotate_for_bug_detection_on_move(that.common(), - GetPolicyFunctions()); + annotate_for_bug_detection_on_move(that); return *this; } @@ -3792,8 +3797,7 @@ &that.char_alloc_ref()); } that.common() = CommonFields::CreateDefault<SooEnabled()>(); - common().annotate_for_bug_detection_on_move(that.common(), - GetPolicyFunctions()); + annotate_for_bug_detection_on_move(that); return *this; }
diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index 60fb142..b31af8f 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc
@@ -965,8 +965,7 @@ } } -template <class T, bool kTransferable = false, bool kSoo = false, - bool kDestroyTrivial = false> +template <class T, bool kTransferable = false, bool kDestroyTrivial = false> struct ValuePolicy { using slot_type = T; using key_type = T; @@ -1015,8 +1014,6 @@ static constexpr HashSlotFn get_hash_slot_fn() { return nullptr; } - - static constexpr bool soo_enabled() { return kSoo; } }; using IntPolicy = ValuePolicy<int64_t>; @@ -1058,9 +1055,8 @@ static_assert(N >= sizeof(int64_t)); int64_t vals_[N / sizeof(int64_t)]; }; -template <int N, bool kSoo> -using SizedValuePolicy = - ValuePolicy<SizedValue<N>, /*kTransferable=*/true, kSoo>; +template <int N> +using SizedValuePolicy = ValuePolicy<SizedValue<N>, /*kTransferable=*/true>; // Value is aligned as type T and contains N copies of it. template <typename T, int N> @@ -1185,10 +1181,10 @@ using Base::Base; }; -template <typename T, bool kTransferable = false, bool kSoo = false, - bool kDestroyTrivial = false, class Alloc = std::allocator<T>> +template <typename T, bool kTransferable = false, bool kDestroyTrivial = false, + class Alloc = std::allocator<T>> struct ValueTable : InstantiateRawHashSet< - ValuePolicy<T, kTransferable, kSoo, kDestroyTrivial>, + ValuePolicy<T, kTransferable, kDestroyTrivial>, hash_default_hash<T>, std::equal_to<T>, Alloc>::type { using Base = typename ValueTable::raw_hash_set; using Base::Base; @@ -1294,7 +1290,7 @@ template <int N> struct FreezableSizedValueSooTable - : raw_hash_set<SizedValuePolicy<N, /*kSoo=*/true>, + : raw_hash_set<SizedValuePolicy<N>, container_internal::hash_default_hash<SizedValue<N>>, std::equal_to<SizedValue<N>>, FreezableAlloc<SizedValue<N>>> { @@ -1328,23 +1324,18 @@ static_assert(sizeof(NonSooIntTableSlotType) > MaxSooSlotSize(), "too small"); using NonSooIntTable = ValueTable<NonSooIntTableSlotType>; using NonSooIntTableTrivialDestroy = - ValueTable<NonSooIntTableSlotType, /*kTransferable=*/false, /*kSoo=*/false, + ValueTable<NonSooIntTableSlotType, /*kTransferable=*/false, /*kDestroyTrivial=*/true>; -using SooInt32Table = - ValueTable<int32_t, /*kTransferable=*/true, /*kSoo=*/true>; -using SooIntTable = ValueTable<int64_t, /*kTransferable=*/true, /*kSoo=*/true>; +using SooInt32Table = ValueTable<int32_t, /*kTransferable=*/true>; +using SooIntTable = ValueTable<int64_t, /*kTransferable=*/true>; using SooIntTableTrivialDestroy = - ValueTable<int64_t, /*kTransferable=*/true, /*kSoo=*/true, - /*kDestroyTrivial=*/true>; -using NonMemcpyableSooIntTable = - ValueTable<int64_t, /*kTransferable=*/false, /*kSoo=*/true>; + ValueTable<int64_t, /*kTransferable=*/true, /*kDestroyTrivial=*/true>; +using NonMemcpyableSooIntTable = ValueTable<int64_t, /*kTransferable=*/false>; using MemcpyableSooIntCustomAllocTable = - ValueTable<int64_t, /*kTransferable=*/true, /*kSoo=*/true, - /*kDestroyTrivial=*/false, + ValueTable<int64_t, /*kTransferable=*/true, /*kDestroyTrivial=*/false, ChangingSizeAndTrackingTypeAlloc<int64_t>>; using NonMemcpyableSooIntCustomAllocTable = - ValueTable<int64_t, /*kTransferable=*/false, /*kSoo=*/true, - /*kDestroyTrivial=*/false, + ValueTable<int64_t, /*kTransferable=*/false, /*kDestroyTrivial=*/false, ChangingSizeAndTrackingTypeAlloc<int64_t>>; TEST(Table, EmptyFunctorOptimization) { @@ -2757,24 +2748,22 @@ using SmallTableTypes = ::testing::Types< IntTable, TransferableIntTable, SooIntTable, // int8 - ValueTable<int8_t, /*kTransferable=*/true, /*kSoo=*/true>, - ValueTable<int8_t, /*kTransferable=*/false, /*kSoo=*/true>, + ValueTable<int8_t, /*kTransferable=*/true>, + ValueTable<int8_t, /*kTransferable=*/false>, // int16 - ValueTable<int16_t, /*kTransferable=*/true, /*kSoo=*/true>, + ValueTable<int16_t, /*kTransferable=*/true>, // int128 - ValueTable<SizedValue<16>, /*kTransferable=*/true, /*kSoo=*/true>, + ValueTable<SizedValue<16>, /*kTransferable=*/true>, // Special tables. MinimumAlignmentUint8Table, CustomAllocIntTable, ChangingSizeAllocIntTable, BadTable, // alignment 1, size 2. - ValueTable<AlignedValue<uint8_t, 2>, /*kTransferable=*/true, /*kSoo=*/true>, + ValueTable<AlignedValue<uint8_t, 2>, /*kTransferable=*/true>, // alignment 1, size 7. - ValueTable<AlignedValue<uint8_t, 7>, /*kTransferable=*/true, /*kSoo=*/true>, - ValueTable<AlignedValue<uint8_t, 7>, /*kTransferable=*/false, - /*kSoo=*/true>, + ValueTable<AlignedValue<uint8_t, 7>, /*kTransferable=*/true>, + ValueTable<AlignedValue<uint8_t, 7>, /*kTransferable=*/false>, // alignment 2, size 6. - ValueTable<AlignedValue<uint16_t, 3>, /*kTransferable=*/true, - /*kSoo=*/true>>; + ValueTable<AlignedValue<uint16_t, 3>, /*kTransferable=*/true>>; TYPED_TEST_SUITE(SmallTableResizeTest, SmallTableTypes); TYPED_TEST(SmallTableResizeTest, InsertIntoSmallTable) { @@ -3840,9 +3829,8 @@ } }; - const auto test_table = [](auto soo) { - using Policy = ValuePolicy<absl::string_view, /*kTransferable=*/true, - /*kSoo=*/decltype(soo)::value>; + const auto test_table = [] { + using Policy = ValuePolicy<absl::string_view, /*kTransferable=*/true>; using Table = raw_hash_set<Policy, hash_default_hash<absl::string_view>, hash_default_eq<absl::string_view>>; Table t; @@ -3907,8 +3895,7 @@ } }; - test_table(/*soo=*/std::false_type{}); - test_table(/*soo=*/std::true_type{}); + test_table(); } TEST(Iterator, IsDefaultConstructible) {