From 9ebb40865ff39f585e065ca031fcdfaeed872edd Mon Sep 17 00:00:00 2001 From: Vitaly Goldshteyn Date: Tue, 4 Feb 2025 13:12:44 -0800 Subject: [PATCH] Leave the call to `SampleSlow` only in type erased InitializeSlots. Call to `SampleSlow` was happening in SOO tables on insert. That increases binary size in the hot place. With this change only verification of whether sample needs to be done is performed. PiperOrigin-RevId: 723201811 Change-Id: I75eba1c4803390587edc9342a177d4ed5eaf342c --- absl/container/internal/hashtablez_sampler.cc | 14 +++ absl/container/internal/hashtablez_sampler.h | 38 ++++--- absl/container/internal/raw_hash_set.cc | 6 +- absl/container/internal/raw_hash_set.h | 102 +++++++++--------- 4 files changed, 94 insertions(+), 66 deletions(-) diff --git a/absl/container/internal/hashtablez_sampler.cc b/absl/container/internal/hashtablez_sampler.cc index fd21d966b70..7b9a463f640 100644 --- a/absl/container/internal/hashtablez_sampler.cc +++ b/absl/container/internal/hashtablez_sampler.cc @@ -126,6 +126,20 @@ static bool ShouldForceSampling() { return state == kForce; } +#if defined(ABSL_INTERNAL_HASHTABLEZ_SAMPLE) +HashtablezInfoHandle ForcedTrySample(size_t inline_element_size, + size_t key_size, size_t value_size, + uint16_t soo_capacity) { + return HashtablezInfoHandle(SampleSlow(global_next_sample, + inline_element_size, key_size, + value_size, soo_capacity)); +} +#else +HashtablezInfoHandle ForcedTrySample(size_t, size_t, size_t, uint16_t) { + return HashtablezInfoHandle{nullptr}; +} +#endif // ABSL_INTERNAL_HASHTABLEZ_SAMPLE + HashtablezInfo* SampleSlow(SamplingState& next_sample, size_t inline_element_size, size_t key_size, size_t value_size, uint16_t soo_capacity) { diff --git a/absl/container/internal/hashtablez_sampler.h b/absl/container/internal/hashtablez_sampler.h index d74acf8c6e2..0c30a7e856a 100644 --- a/absl/container/internal/hashtablez_sampler.h +++ b/absl/container/internal/hashtablez_sampler.h @@ -219,22 +219,36 @@ class HashtablezInfoHandle { extern ABSL_PER_THREAD_TLS_KEYWORD SamplingState global_next_sample; #endif // defined(ABSL_INTERNAL_HASHTABLEZ_SAMPLE) -// Returns a sampling handle. -inline HashtablezInfoHandle Sample( - ABSL_ATTRIBUTE_UNUSED size_t inline_element_size, - ABSL_ATTRIBUTE_UNUSED size_t key_size, - ABSL_ATTRIBUTE_UNUSED size_t value_size, - ABSL_ATTRIBUTE_UNUSED uint16_t soo_capacity) { +// Returns true if the next table should be sampled. +// This function updates the global state. +// If the function returns true, actual sampling should be done by calling +// ForcedTrySample(). +inline bool ShouldSampleNextTable() { #if defined(ABSL_INTERNAL_HASHTABLEZ_SAMPLE) if (ABSL_PREDICT_TRUE(--global_next_sample.next_sample > 0)) { - return HashtablezInfoHandle(nullptr); + return false; } - return HashtablezInfoHandle(SampleSlow(global_next_sample, - inline_element_size, key_size, - value_size, soo_capacity)); + return true; #else - return HashtablezInfoHandle(nullptr); -#endif // !ABSL_PER_THREAD_TLS + return false; +#endif // ABSL_INTERNAL_HASHTABLEZ_SAMPLE +} + +// Returns a sampling handle. +// Must be called only if HashSetShouldBeSampled() returned true. +// Returned handle still can be unsampled if sampling is not possible. +HashtablezInfoHandle ForcedTrySample(size_t inline_element_size, + size_t key_size, size_t value_size, + uint16_t soo_capacity); + +// Returns a sampling handle. +inline HashtablezInfoHandle Sample(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); + } + return ForcedTrySample(inline_element_size, key_size, value_size, + soo_capacity); } using HashtablezSampler = diff --git a/absl/container/internal/raw_hash_set.cc b/absl/container/internal/raw_hash_set.cc index 5b9cb2f2a51..2878e798dc9 100644 --- a/absl/container/internal/raw_hash_set.cc +++ b/absl/container/internal/raw_hash_set.cc @@ -574,7 +574,7 @@ FindInfo FindInsertPositionWithGrowthOrRehash(CommonFields& common, size_t hash, DropDeletesWithoutResize(common, policy); } else { // Otherwise grow the container. - policy.resize(common, NextCapacity(cap), HashtablezInfoHandle{}); + policy.resize(common, NextCapacity(cap), /*force_infoz=*/false); } // This function is typically called with tables containing deleted slots. // The table will be big and `FindFirstNonFullAfterResize` will always @@ -630,7 +630,7 @@ size_t PrepareInsertNonSoo(CommonFields& common, size_t hash, FindInfo target, // 3. Table with deleted slots that needs to be rehashed or resized. if (ABSL_PREDICT_TRUE(common.growth_info().HasNoGrowthLeftAndNoDeleted())) { const size_t old_capacity = common.capacity(); - policy.resize(common, NextCapacity(old_capacity), HashtablezInfoHandle{}); + policy.resize(common, NextCapacity(old_capacity), /*force_infoz=*/false); target = HashSetResizeHelper::FindFirstNonFullAfterResize( common, old_capacity, hash); } else { @@ -643,7 +643,7 @@ size_t PrepareInsertNonSoo(CommonFields& common, size_t hash, FindInfo target, const size_t cap = common.capacity(); policy.resize(common, common.growth_left() > 0 ? cap : NextCapacity(cap), - HashtablezInfoHandle{}); + /*force_infoz=*/false); } if (ABSL_PREDICT_TRUE(common.growth_left() > 0)) { target = find_first_non_full(common, hash); diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index df5effc97df..4edd397926b 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -1399,9 +1399,10 @@ class CommonFields : public CommonFieldsGenerationInfo { size_ += size_t{1} << HasInfozShift(); } void decrement_size() { - ABSL_SWISSTABLE_ASSERT(size() > 0); + ABSL_SWISSTABLE_ASSERT(!empty()); size_ -= size_t{1} << HasInfozShift(); } + bool empty() const { return size() == 0; } // The total number of available slots. size_t capacity() const { return capacity_; } @@ -1963,7 +1964,7 @@ ABSL_ATTRIBUTE_ALWAYS_INLINE inline void IterateOverFullSlots( } template -constexpr bool ShouldSampleHashtablezInfo() { +constexpr bool ShouldSampleHashtablezInfoForAlloc() { // Folks with custom allocators often make unwarranted assumptions about the // behavior of their classes vis-a-vis trivial destructability and what // calls they will or won't make. Avoid sampling for people with custom @@ -1972,24 +1973,26 @@ constexpr bool ShouldSampleHashtablezInfo() { return std::is_same>::value; } -template -HashtablezInfoHandle SampleHashtablezInfo(size_t sizeof_slot, size_t sizeof_key, - size_t sizeof_value, - size_t old_capacity, bool was_soo, - HashtablezInfoHandle forced_infoz, - CommonFields& c) { - if (forced_infoz.IsSampled()) return forced_infoz; +template +bool ShouldSampleHashtablezInfoOnResize(bool force_sampling, + size_t old_capacity, CommonFields& c) { + if (!ShouldSampleHashtablezInfoForAlloc()) { + return false; + } + // Force sampling is only allowed for SOO tables. + ABSL_SWISSTABLE_ASSERT(kSooEnabled || !force_sampling); + if (kSooEnabled && force_sampling) { + return true; + } // In SOO, we sample on the first insertion so if this is an empty SOO case // (e.g. when reserve is called), then we still need to sample. - if (kSooEnabled && was_soo && c.size() == 0) { - return Sample(sizeof_slot, sizeof_key, sizeof_value, SooCapacity()); + if (kSooEnabled && old_capacity == SooCapacity() && c.empty()) { + return ShouldSampleNextTable(); } - // For non-SOO cases, we sample whenever the capacity is increasing from zero - // to non-zero. if (!kSooEnabled && old_capacity == 0) { - return Sample(sizeof_slot, sizeof_key, sizeof_value, 0); + return ShouldSampleNextTable(); } - return c.infoz(); + return false; } // PolicyFunctions bundles together some information for a particular @@ -2013,8 +2016,7 @@ struct PolicyFunctions { // Resizes set to the new capacity. // Arguments are used as in raw_hash_set::resize_impl. - void (*resize)(CommonFields& common, size_t new_capacity, - HashtablezInfoHandle forced_infoz); + void (*resize)(CommonFields& common, size_t new_capacity, bool force_infoz); }; // Returns the index of the SOO slot when growing from SOO to non-SOO in a @@ -2081,12 +2083,12 @@ ABSL_ATTRIBUTE_ALWAYS_INLINE inline void InitializeSmallControlBytesAfterSoo( class HashSetResizeHelper { public: explicit HashSetResizeHelper(CommonFields& c, bool was_soo, bool had_soo_slot, - HashtablezInfoHandle forced_infoz) + bool force_infoz) : old_capacity_(c.capacity()), had_infoz_(c.has_infoz()), was_soo_(was_soo), had_soo_slot_(had_soo_slot), - forced_infoz_(forced_infoz) {} + force_infoz_(force_infoz) {} // Optimized for small groups version of `find_first_non_full`. // Beneficial only right after calling `raw_hash_set::resize`. @@ -2150,13 +2152,14 @@ class HashSetResizeHelper { size_t value_size, const PolicyFunctions& policy) { ABSL_SWISSTABLE_ASSERT(c.capacity()); - HashtablezInfoHandle infoz = - ShouldSampleHashtablezInfo() - ? SampleHashtablezInfo(SizeOfSlot, key_size, value_size, - old_capacity_, was_soo_, - forced_infoz_, c) - : HashtablezInfoHandle{}; - + HashtablezInfoHandle infoz = c.infoz(); + const bool should_sample = + ShouldSampleHashtablezInfoOnResize(force_infoz_, + old_capacity_, c); + if (ABSL_PREDICT_FALSE(should_sample)) { + infoz = ForcedTrySample(SizeOfSlot, key_size, value_size, + SooEnabled ? SooCapacity() : 0); + } const bool has_infoz = infoz.IsSampled(); RawHashSetLayout layout(c.capacity(), AlignOfSlot, has_infoz); @@ -2380,8 +2383,9 @@ class HashSetResizeHelper { bool had_infoz_; bool was_soo_; bool had_soo_slot_; - // Either null infoz or a pre-sampled forced infoz for SOO tables. - HashtablezInfoHandle forced_infoz_; + // True if it is known that table needs to be sampled. + // Used for sampling SOO tables. + bool force_infoz_; }; inline void PrepareInsertCommon(CommonFields& common) { @@ -2880,8 +2884,7 @@ class raw_hash_set { ABSL_SWISSTABLE_ASSERT(size == 1); common().set_full_soo(); emplace_at(soo_iterator(), *that.begin()); - const HashtablezInfoHandle infoz = try_sample_soo(); - if (infoz.IsSampled()) resize_with_soo_infoz(infoz); + if (should_sample_soo()) resize_with_soo_sample(); return; } ABSL_SWISSTABLE_ASSERT(!that.is_soo()); @@ -3742,14 +3745,13 @@ class raw_hash_set { } } - // Conditionally samples hashtablez for SOO tables. This should be called on - // insertion into an empty SOO table and in copy construction when the size - // can fit in SOO capacity. - inline HashtablezInfoHandle try_sample_soo() { + // Returns true if the table needs to be sampled. + // This should be called on insertion into an empty SOO table and in copy + // construction when the size can fit in SOO capacity. + inline bool should_sample_soo() { ABSL_SWISSTABLE_ASSERT(is_soo()); - if (!ShouldSampleHashtablezInfo()) return HashtablezInfoHandle{}; - return Sample(sizeof(slot_type), sizeof(key_type), sizeof(value_type), - SooCapacity()); + if (!ShouldSampleHashtablezInfoForAlloc()) return false; + return ShouldSampleNextTable(); } inline void destroy_slots() { @@ -3811,32 +3813,31 @@ class raw_hash_set { // common(), old_capacity, hash) // can be called right after `resize`. void resize(size_t new_capacity) { - raw_hash_set::resize_impl(common(), new_capacity, HashtablezInfoHandle{}); + raw_hash_set::resize_impl(common(), new_capacity, /*force_infoz=*/false); } - // As above, except that we also accept a pre-sampled, forced infoz for - // SOO tables, since they need to switch from SOO to heap in order to - // store the infoz. - void resize_with_soo_infoz(HashtablezInfoHandle forced_infoz) { - ABSL_SWISSTABLE_ASSERT(forced_infoz.IsSampled()); + // As above, except that we also force SOO table to be sampled. + // SOO tables need to switch from SOO to heap in order to store the infoz. + void resize_with_soo_sample() { raw_hash_set::resize_impl(common(), NextCapacity(SooCapacity()), - forced_infoz); + /*force_infoz=*/true); } // Resizes set to the new capacity. // It is a static function in order to use its pointer in GetPolicyFunctions. - ABSL_ATTRIBUTE_NOINLINE static void resize_impl( - CommonFields& common, size_t new_capacity, - HashtablezInfoHandle forced_infoz) { + ABSL_ATTRIBUTE_NOINLINE static void resize_impl(CommonFields& common, + size_t new_capacity, + bool force_infoz) { raw_hash_set* set = reinterpret_cast(&common); ABSL_SWISSTABLE_ASSERT(IsValidCapacity(new_capacity)); ABSL_SWISSTABLE_ASSERT(!set->fits_in_soo(new_capacity)); + ABSL_SWISSTABLE_ASSERT(!force_infoz || SooEnabled()); const bool was_soo = set->is_soo(); const bool had_soo_slot = was_soo && !set->empty(); const size_t soo_slot_hash = had_soo_slot ? set->hash_of(set->soo_slot()) : 0; HashSetResizeHelper resize_helper(common, was_soo, had_soo_slot, - forced_infoz); + force_infoz); common.set_capacity(new_capacity); // Note that `InitializeSlots` does different number initialization steps // depending on the values of `transfer_uses_memcpy` and capacities. @@ -3995,9 +3996,8 @@ class raw_hash_set { template std::pair find_or_prepare_insert_soo(const K& key) { if (empty()) { - const HashtablezInfoHandle infoz = try_sample_soo(); - if (infoz.IsSampled()) { - resize_with_soo_infoz(infoz); + if (should_sample_soo()) { + resize_with_soo_sample(); } else { common().set_full_soo(); return {soo_iterator(), true};