From 1063d2b829bbf1b5cf03f5f8e1e9fe4c71c9f966 Mon Sep 17 00:00:00 2001 From: Samuel Benzaquen Date: Thu, 1 Dec 2022 10:08:04 -0800 Subject: Move the vtable into a function to delay instantiation until the function is called. When the variable is a global the compiler is allowed to instantiate it more aggresively and it might happen before the types involved are complete. When it is inside a function the compiler can't instantiate it until after the functions are called. Remove an unused member from the vtable. Replace transfer_slot_fn with a generic function when relocation is available to reduce duplication. PiperOrigin-RevId: 492227302 Change-Id: I07499f63b91c59c0ae42402683387c7b84a6f0ee --- absl/container/internal/common_policy_traits.h | 45 ++++++++++++++++++-------- absl/container/internal/raw_hash_set.h | 44 ++++++++++++++----------- 2 files changed, 56 insertions(+), 33 deletions(-) diff --git a/absl/container/internal/common_policy_traits.h b/absl/container/internal/common_policy_traits.h index 0fd4866e..b42c9a48 100644 --- a/absl/container/internal/common_policy_traits.h +++ b/absl/container/internal/common_policy_traits.h @@ -63,7 +63,7 @@ struct common_policy_traits { // UNINITIALIZED template static void transfer(Alloc* alloc, slot_type* new_slot, slot_type* old_slot) { - transfer_impl(alloc, new_slot, old_slot, 0); + transfer_impl(alloc, new_slot, old_slot, Rank0{}); } // PRECONDITION: `slot` is INITIALIZED @@ -80,29 +80,46 @@ struct common_policy_traits { return P::element(slot); } + static constexpr bool transfer_uses_memcpy() { + return std::is_same>( + nullptr, nullptr, nullptr, Rank0{})), + std::true_type>::value; + } + private: + // To rank the overloads below for overload resoltion. Rank0 is preferred. + struct Rank2 {}; + struct Rank1 : Rank2 {}; + struct Rank0 : Rank1 {}; + // Use auto -> decltype as an enabler. template static auto transfer_impl(Alloc* alloc, slot_type* new_slot, - slot_type* old_slot, int) + slot_type* old_slot, Rank0) -> decltype((void)P::transfer(alloc, new_slot, old_slot)) { P::transfer(alloc, new_slot, old_slot); } - template - static void transfer_impl(Alloc* alloc, slot_type* new_slot, - slot_type* old_slot, char) { #if defined(__cpp_lib_launder) && __cpp_lib_launder >= 201606 - if (absl::is_trivially_relocatable()) { - // TODO(b/247130232,b/251814870): remove casts after fixing warnings. - std::memcpy(static_cast( - std::launder(const_cast*>( - &element(new_slot)))), - static_cast(&element(old_slot)), - sizeof(value_type)); - return; - } + // This overload returns true_type for the trait below. + // The conditional_t is to make the enabler type dependent. + template >::value>> + static std::true_type transfer_impl(Alloc*, slot_type* new_slot, + slot_type* old_slot, Rank1) { + // TODO(b/247130232): remove casts after fixing warnings. + // TODO(b/251814870): remove casts after fixing warnings. + std::memcpy( + static_cast(std::launder( + const_cast*>(&element(new_slot)))), + static_cast(&element(old_slot)), sizeof(value_type)); + return {}; + } #endif + template + static void transfer_impl(Alloc* alloc, slot_type* new_slot, + slot_type* old_slot, Rank2) { construct(alloc, new_slot, std::move(element(old_slot))); destroy(alloc, old_slot); } diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index ab59856e..f7c63467 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -1058,7 +1058,6 @@ ABSL_ATTRIBUTE_NOINLINE void InitializeSlots(CommonFields& c, // work. struct PolicyFunctions { size_t slot_size; - size_t slot_align; // Return the hash of the pointed-to slot. size_t (*hash_slot)(void* set, void* slot); @@ -1098,6 +1097,14 @@ ABSL_ATTRIBUTE_NOINLINE void DeallocateStandard(void*, AllocSize(n, policy.slot_size, AlignOfSlot)); } +// For trivially relocatable types we use memcpy directly. This allows us to +// share the same function body for raw_hash_set instantiations that have the +// same slot size as long as they are relocatable. +template +ABSL_ATTRIBUTE_NOINLINE void TransferRelocatable(void*, void* dst, void* src) { + memcpy(dst, src, SizeOfSlot); +} + // Type-erased version of raw_hash_set::drop_deletes_without_resize. void DropDeletesWithoutResize(CommonFields& common, size_t& growth_left, const PolicyFunctions& policy, void* tmp_space); @@ -1538,7 +1545,7 @@ class raw_hash_set : private CommonFields { // Already guaranteed to be empty; so nothing to do. } else { destroy_slots(); - ClearBackingArray(common(), growth_left(), kPolicyFunctions, + ClearBackingArray(common(), growth_left(), GetPolicyFunctions(), /*reuse=*/cap < 128); } } @@ -1846,7 +1853,7 @@ class raw_hash_set : private CommonFields { void rehash(size_t n) { if (n == 0 && capacity() == 0) return; if (n == 0 && size() == 0) { - ClearBackingArray(common(), growth_left(), kPolicyFunctions, + ClearBackingArray(common(), growth_left(), GetPolicyFunctions(), /*reuse=*/false); return; } @@ -2127,7 +2134,8 @@ class raw_hash_set : private CommonFields { inline void drop_deletes_without_resize() { // Stack-allocate space for swapping elements. alignas(slot_type) unsigned char tmp[sizeof(slot_type)]; - DropDeletesWithoutResize(common(), growth_left(), kPolicyFunctions, tmp); + DropDeletesWithoutResize(common(), growth_left(), GetPolicyFunctions(), + tmp); } // Called whenever the table *might* need to conditionally grow. @@ -2347,15 +2355,19 @@ class raw_hash_set : private CommonFields { AllocSize(n, sizeof(slot_type), alignof(slot_type))); } - static constexpr PolicyFunctions kPolicyFunctions = { - sizeof(slot_type), - alignof(slot_type), - &raw_hash_set::hash_slot_fn, - &raw_hash_set::transfer_slot_fn, - (std::is_same>::value - ? &DeallocateStandard - : &raw_hash_set::dealloc_fn), - }; + static const PolicyFunctions& GetPolicyFunctions() { + static constexpr PolicyFunctions value = { + sizeof(slot_type), + &raw_hash_set::hash_slot_fn, + PolicyTraits::transfer_uses_memcpy() + ? TransferRelocatable + : &raw_hash_set::transfer_slot_fn, + (std::is_same>::value + ? &DeallocateStandard + : &raw_hash_set::dealloc_fn), + }; + return value; + } // Bundle together growth_left (number of slots that can be filled without // rehashing) plus other objects which might be empty. CompressedTuple will @@ -2366,12 +2378,6 @@ class raw_hash_set : private CommonFields { settings_{0u, hasher{}, key_equal{}, allocator_type{}}; }; -#ifdef ABSL_INTERNAL_NEED_REDUNDANT_CONSTEXPR_DECL -template -constexpr PolicyFunctions - raw_hash_set::kPolicyFunctions; -#endif - // Erases all elements that satisfy the predicate `pred` from the container `c`. template typename raw_hash_set::size_type EraseIf( -- cgit v1.2.3