diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h index a071f6e..eff2bb2 100644 --- a/parallel_hashmap/phmap.h +++ b/parallel_hashmap/phmap.h @@ -60,27 +60,81 @@ namespace phmap { // --------------------------------------------------------------- -// from http://burtleburtle.net/bob/hash/integer.html -// fast and efficient for power of two table sizes where we always -// consider the last bits. // --------------------------------------------------------------- -inline size_t phmap_mix_32(uint32_t a) +template +struct phmap_mix { - a = a ^ (a >> 4); - a = (a ^ 0xdeadbeef) + (a << 5); - a = a ^ (a >> 11); - return static_cast(a); -} + inline size_t operator()(size_t) const; +}; -// Very fast mixing: https://godbolt.org/z/3F709Y -// ---------------------------------------------- -inline size_t phmap_mix_64(uint64_t a) +template<> +struct phmap_mix<4> { - static constexpr uint64_t k = UINT64_C(0xde5fb9d2630458e9); - uint64_t h; - uint64_t l = umul128(a, k, &h); - return static_cast(h + l); -} + inline size_t operator()(size_t a) const + { + static constexpr uint64_t kmul = 0xcc9e2d51; + uint64_t l = a * kmul; + return static_cast(l ^ (l >> 32)); + } +}; + +#ifdef PHMAP_HAS_UMUL128 + template<> + struct phmap_mix<8> + { + // Very fast mixing (similar to Abseil) + inline size_t operator()(size_t a) const + { + static constexpr uint64_t k = UINT64_C(0xde5fb9d2630458e9); + uint64_t h; + uint64_t l = umul128(a, k, &h); + return static_cast(h + l); + } + }; +#else + template<> + struct phmap_mix<8> + { + inline size_t operator()(size_t a) const + { + a = (~a) + (a << 21); // a = (a << 21) - a - 1; + a = a ^ (a >> 24); + a = (a + (a << 3)) + (a << 8); // a * 265 + a = a ^ (a >> 14); + a = (a + (a << 2)) + (a << 4); // a * 21 + a = a ^ (a >> 28); + a = a + (a << 31); + return static_cast(a); + } + }; +#endif + +// -------------------------------------------- +template +struct fold_if_needed +{ + inline size_t operator()(uint64_t) const; +}; + +template<> +struct fold_if_needed<4> +{ + inline size_t operator()(uint64_t a) const + { + return static_cast(a ^ (a >> 32)); + } +}; + +template<> +struct fold_if_needed<8> +{ + inline size_t operator()(uint64_t a) const + { + return static_cast(a); + } +}; + + // --------------------------------------------------------------- // phmap::Hash // --------------------------------------------------------------- @@ -89,8 +143,7 @@ struct Hash { inline size_t operator()(const T& val) const { - // we mix for safety in case std::hash broken. - return phmap_mix_64(std::hash()(val)); + return std::hash()(val); } }; @@ -99,7 +152,7 @@ struct Hash { inline size_t operator()(const T *val) const noexcept { - return phmap_mix_64((const uintptr_t)val); + return (size_t)(const uintptr_t)val; } }; @@ -149,42 +202,42 @@ template <> struct Hash : public phmap_unary_function { inline size_t operator()(int16_t val) const noexcept - { return phmap_mix_64(static_cast(val)); } + { return static_cast(val); } }; template <> struct Hash : public phmap_unary_function { inline size_t operator()(uint16_t val) const noexcept - { return phmap_mix_64(static_cast(val)); } + { return static_cast(val); } }; template <> struct Hash : public phmap_unary_function { inline size_t operator()(int32_t val) const noexcept - { return phmap_mix_64(static_cast(val)); } + { return static_cast(val); } }; template <> struct Hash : public phmap_unary_function { inline size_t operator()(uint32_t val) const noexcept - { return phmap_mix_64(static_cast(val)); } + { return static_cast(val); } }; template <> struct Hash : public phmap_unary_function { inline size_t operator()(int64_t val) const noexcept - { return phmap_mix_64(static_cast(val)); } + { return fold_if_needed()(static_cast(val)); } }; template <> struct Hash : public phmap_unary_function { inline size_t operator()(uint64_t val) const noexcept - { return phmap_mix_64(static_cast(val)); } + { return fold_if_needed()(val); } }; template <> @@ -194,7 +247,8 @@ struct Hash : public phmap_unary_function { // -0.0 and 0.0 should return same hash uint32_t *as_int = reinterpret_cast(&val); - return (val == 0) ? static_cast(0) : phmap_mix_64(*as_int); + return (val == 0) ? static_cast(0) : + static_cast(*as_int); } }; @@ -205,7 +259,8 @@ struct Hash : public phmap_unary_function { // -0.0 and 0.0 should return same hash uint64_t *as_int = reinterpret_cast(&val); - return (val == 0) ? static_cast(0) : phmap_mix_64(*as_int); + return (val == 0) ? static_cast(0) : + fold_if_needed()(*as_int); } }; @@ -396,17 +451,6 @@ inline ctrl_t* EmptyGroup() { return const_cast(empty_group); } -// -------------------------------------------------------------------------- -// Mixes a randomly generated per-process seed with `hash` and `ctrl` to -// randomize insertion order within groups. -// -------------------------------------------------------------------------- -static inline bool ShouldInsertBackwards(size_t hash, ctrl_t* ctrl); - -// -------------------------------------------------------------------------- -// Returns a hash seed. -// -// The seed consists of the ctrl_ pointer, which adds enough entropy to ensure -// non-determinism of iteration order in most cases. // -------------------------------------------------------------------------- inline size_t HashSeed(const ctrl_t* ctrl) { // The low bits of the pointer have little or no entropy because of @@ -416,14 +460,20 @@ inline size_t HashSeed(const ctrl_t* ctrl) { } inline size_t H1(size_t hash, const ctrl_t* ctrl) { - return (hash >> 7) ^ HashSeed(ctrl); + return (hash >> 7) +#if PHMAP_NON_DETERMINISTIC + // use ctrl_ pointer to add entropy to ensure + // non-deterministic iteration order. + ^ HashSeed(ctrl) +#endif + ; // last seven bits stored in the control bytes } -inline ctrl_t H2(size_t hash) { return hash & 0x7F; } +inline ctrl_t H2(size_t hash) { return hash & 0x7F; } -inline bool IsEmpty(ctrl_t c) { return c == kEmpty; } -inline bool IsFull(ctrl_t c) { return c >= 0; } -inline bool IsDeleted(ctrl_t c) { return c == kDeleted; } +inline bool IsEmpty(ctrl_t c) { return c == kEmpty; } +inline bool IsFull(ctrl_t c) { return c >= 0; } +inline bool IsDeleted(ctrl_t c) { return c == kDeleted; } inline bool IsEmptyOrDeleted(ctrl_t c) { return c < kSentinel; } #if PHMAP_HAVE_SSE2 @@ -1728,7 +1778,7 @@ public: template void prefetch(const key_arg& key) const { - prefetch_hash(hash_ref()(key)); + prefetch_hash(HashElement{hash_ref()}(key)); } // The API of find() has two extensions. @@ -1749,13 +1799,14 @@ public: PolicyTraits::element(slots_ + seq.offset(i))))) return iterator_at(seq.offset(i)); } - if (PHMAP_PREDICT_TRUE(g.MatchEmpty())) return end(); + if (PHMAP_PREDICT_TRUE(g.MatchEmpty())) + return end(); seq.next(); } } template iterator find(const key_arg& key) { - return find(key, hash_ref()(key)); + return find(key, HashElement{hash_ref()}(key)); } template @@ -1764,7 +1815,7 @@ public: } template const_iterator find(const key_arg& key) const { - return find(key, hash_ref()(key)); + return find(key, HashElement{hash_ref()}(key)); } template @@ -1835,7 +1886,7 @@ private: { template size_t operator()(const K& key, Args&&...) const { - return h(key); + return phmap_mix()(h(key)); } const hasher& h; }; @@ -1866,7 +1917,7 @@ private: { template std::pair operator()(const K& key, Args&&... args) const { - return s.emplace_decomposable(key, s.hash_ref()(key), + return s.emplace_decomposable(key, typename raw_hash_set::HashElement{s.hash_ref()}(key), std::forward(args)...); } raw_hash_set& s; @@ -1936,7 +1987,8 @@ private: void initialize_slots() { assert(capacity_); - if (slots_ == nullptr) { + if (std::is_same>::value && + slots_ == nullptr) { infoz_ = Sample(); } @@ -2117,15 +2169,6 @@ private: Group g{ctrl_ + seq.offset()}; auto mask = g.MatchEmptyOrDeleted(); if (mask) { -#if !defined(NDEBUG) - // We want to add entropy even when ASLR is not enabled. - // In debug build we will randomly insert in either the front or back of - // the group. - // TODO(kfm,sbenza): revisit after we do unconditional mixing - if (!is_small() && ShouldInsertBackwards(hash, ctrl_)) { - return {seq.offset(mask.HighestBitSet()), seq.index()}; - } -#endif return {seq.offset(mask.LowestBitSet()), seq.index()}; } assert(seq.index() < capacity_ && "full table!"); @@ -2165,7 +2208,7 @@ protected: template std::pair find_or_prepare_insert(const K& key) { - return find_or_prepare_insert(key, hash_ref()(key)); + return find_or_prepare_insert(key, HashElement{hash_ref()}(key)); } size_t prepare_insert(size_t hash) PHMAP_ATTRIBUTE_NOINLINE { @@ -2460,13 +2503,6 @@ inline size_t RandomSeed() return value ^ static_cast(reinterpret_cast(&counter)); } -static inline bool ShouldInsertBackwards(size_t hash, ctrl_t* ctrl) -{ - // To avoid problems with weak hashes and single bit tests, we use % 13. - // TODO(kfm,sbenza): revisit after we do unconditional mixing - return (H1(hash, ctrl) ^ RandomSeed()) % 13 > 6; -} - // ---------------------------------------------------------------------------- // ---------------------------------------------------------------------------- template std::pair emplace_decomposable(const K& key, Args&&... args) { - size_t hash = hash_ref()(key); + size_t hash = HashElement{hash_ref()}(key); Inner& inner = sets_[subidx(hash)]; auto& set = inner.set_; MutexLock_ m(&inner); @@ -3046,7 +3082,7 @@ public: PolicyTraits::construct(&alloc_ref(), slot, std::forward(args)...); const auto& elem = PolicyTraits::element(slot); - size_t hash = hash_ref()(PolicyTraits::key(slot)); + size_t hash = HashElement{hash_ref()}(PolicyTraits::key(slot)); Inner& inner = sets_[subidx(hash)]; auto& set = inner.set_; MutexLock_ m(&inner); @@ -3075,7 +3111,7 @@ public: template iterator lazy_emplace(const key_arg& key, F&& f) { - auto hash = hash_ref()(key); + auto hash = HashElement{hash_ref()}(key); Inner& inner = sets_[subidx(hash)]; auto& set = inner.set_; MutexLock_ m(&inner); @@ -3094,7 +3130,7 @@ public: // -------------------------------------------------------------------- template size_type erase(const key_arg& key) { - auto hash = hash_ref()(key); + auto hash = HashElement{hash_ref()}(key); Inner& inner = sets_[subidx(hash)]; auto& set = inner.set_; MutexLock_ m(&inner); @@ -3222,7 +3258,7 @@ public: void prefetch(const key_arg& key) const { (void)key; #if defined(__GNUC__) - size_t hash = hash_ref()(key); + size_t hash = HashElement{hash_ref()}(key); const Inner& inner = sets_[subidx(hash)]; const auto& set = inner.set_; MutexLock_ m(const_cast(&inner)); @@ -3249,7 +3285,7 @@ public: template iterator find(const key_arg& key) { - return find(key, hash_ref()(key)); + return find(key, HashElement{hash_ref()}(key)); } template @@ -3259,7 +3295,7 @@ public: template const_iterator find(const key_arg& key) const { - return find(key, hash_ref()(key)); + return find(key, HashElement{hash_ref()}(key)); } template @@ -3336,7 +3372,7 @@ private: { template size_t operator()(const K& key, Args&&...) const { - return h(key); + return phmap_mix()(h(key)); } const hasher& h; }; @@ -3397,7 +3433,7 @@ protected: template std::tuple find_or_prepare_insert(const K& key, MutexLock_ &mutexlock) { - auto hash = hash_ref()(key); + auto hash = HashElement{hash_ref()}(key); Inner& inner = sets_[subidx(hash)]; auto& set = inner.set_; mutexlock.set_mutex(&inner); @@ -4244,7 +4280,7 @@ struct HashtableDebugAccess> static size_t GetNumProbes(const Set& set, const typename Set::key_type& key) { size_t num_probes = 0; - size_t hash = set.hash_ref()(key); + size_t hash = typename Set::HashElement{set.hash_ref()}(key); auto seq = set.probe(hash); while (true) { container_internal::Group g{set.ctrl_ + seq.offset()}; diff --git a/parallel_hashmap/phmap_bits.h b/parallel_hashmap/phmap_bits.h index 296bbaf..df953fb 100644 --- a/parallel_hashmap/phmap_bits.h +++ b/parallel_hashmap/phmap_bits.h @@ -474,6 +474,7 @@ PHMAP_BASE_INTERNAL_FORCEINLINE int CountTrailingZerosNonZero32(uint32_t n) { *high = static_cast(result >> 64); return static_cast(result); } + #define PHMAP_HAS_UMUL128 1 #elif (defined(_MSC_VER)) #if defined(_WIN64) #pragma intrinsic(_umul128) @@ -481,29 +482,7 @@ PHMAP_BASE_INTERNAL_FORCEINLINE int CountTrailingZerosNonZero32(uint32_t n) { { return _umul128(a, b, high); } - #else - #pragma intrinsic(__emulu) - inline uint64_t umul128(uint64_t multiplier, uint64_t multiplicand, uint64_t *product_hi) - { - uint64_t a = multiplier >> 32; - uint64_t b = (uint32_t)multiplier; // & 0xFFFFFFFF; - uint64_t c = multiplicand >> 32; - uint64_t d = (uint32_t)multiplicand; // & 0xFFFFFFFF; - - uint64_t ad = __emulu(a, d); - uint64_t bd = __emulu(b, d); - - uint64_t adbc = ad + __emulu(b, c); - uint64_t adbc_carry = (adbc < ad); // ? 1 : 0; - // MSVC gets confused by the ternary and makes worse code than using a boolean in an integer context for 1 : 0 - - // multiplier * multiplicand = product_hi * 2^64 + product_lo - uint64_t product_lo = bd + (adbc << 32); - uint64_t product_lo_carry = (product_lo < bd); // ? 1 : 0; - *product_hi = __emulu(a, c) + (adbc >> 32) + (adbc_carry << 32) + product_lo_carry; - - return product_lo; - } + #define PHMAP_HAS_UMUL128 1 #endif #endif diff --git a/tests/raw_hash_set_test.cc b/tests/raw_hash_set_test.cc index 96c4493..7a58f12 100644 --- a/tests/raw_hash_set_test.cc +++ b/tests/raw_hash_set_test.cc @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +// define this so that the "IterationOrderChanges" tests pass +#define PHMAP_NON_DETERMINISTIC 1 + #include "parallel_hashmap/phmap.h" //#include "container_memory.h" //#include "hash_function_defaults.h" @@ -349,6 +352,25 @@ struct IntTable using Base::Base; }; +template +struct CustomAlloc : std::allocator { + CustomAlloc() {} + + template + CustomAlloc(const CustomAlloc& other) {} + + template struct rebind { + using other = CustomAlloc; + }; +}; + +struct CustomAllocIntTable + : raw_hash_set, + std::equal_to, CustomAlloc> { + using Base = typename CustomAllocIntTable::raw_hash_set; + using Base::Base; +}; + struct BadFastHash { template size_t operator()(const T&) const { @@ -855,6 +877,25 @@ TEST(Table, Erase) { EXPECT_TRUE(t.find(0) == t.end()); } +TEST(Table, EraseMaintainsValidIterator) { + IntTable t; + const int kNumElements = 100; + for (int i = 0; i < kNumElements; i ++) { + EXPECT_TRUE(t.emplace(i).second); + } + EXPECT_EQ(t.size(), kNumElements); + + int num_erase_calls = 0; + auto it = t.begin(); + while (it != t.end()) { + t.erase(it++); + num_erase_calls++; + } + + EXPECT_TRUE(t.empty()); + EXPECT_EQ(num_erase_calls, kNumElements); +} + // Collect N bad keys by following algorithm: // 1. Create an empty table and reserve it to 2 * N. // 2. Insert N random elements. @@ -1581,7 +1622,7 @@ TEST(Table, HeterogeneousLookup) { size_t operator()(int64_t i) const { return i; } size_t operator()(double i) const { ADD_FAILURE(); - return i; + return (size_t)i; } }; struct Eq { @@ -1603,7 +1644,7 @@ TEST(Table, HeterogeneousLookup) { struct THash { using is_transparent = void; size_t operator()(int64_t i) const { return i; } - size_t operator()(double i) const { return i; } + size_t operator()(double i) const { return (size_t)i; } }; struct TEq { using is_transparent = void; @@ -1615,7 +1656,7 @@ TEST(Table, HeterogeneousLookup) { raw_hash_set> s{0, 1, 2}; // It will convert to int64_t before the query. - EXPECT_EQ(1, *s.find(double{1.1})); + EXPECT_EQ(1, *s.find((int)double{1.1})); raw_hash_set> ts{0, 1, 2}; // It will try to use the double, and fail to find the object. @@ -1778,6 +1819,7 @@ std::vector OrderOfIteration(const IntTable& t) { // we are touching different memory pages to cause the ordering to change. // We also need to keep the old tables around to avoid getting the same memory // blocks over and over. +// not randomizing in phmap TEST(Table, IterationOrderChangesByInstance) { for (size_t size : {2, 6, 12, 20}) { const auto reference_table = MakeSimpleTable(size); @@ -1785,7 +1827,7 @@ TEST(Table, IterationOrderChangesByInstance) { std::vector tables; bool found_difference = false; - for (int i = 0; !found_difference && i < 500; ++i) { + for (int i = 0; !found_difference && i < 5000; ++i) { tables.push_back(MakeSimpleTable(size)); found_difference = OrderOfIteration(tables.back()) != reference; } @@ -1797,9 +1839,10 @@ TEST(Table, IterationOrderChangesByInstance) { } } +// not randomizing in phmap TEST(Table, IterationOrderChangesOnRehash) { std::vector garbage; - for (int i = 0; i < 500; ++i) { + for (int i = 0; i < 5000; ++i) { auto t = MakeSimpleTable(20); const auto reference = OrderOfIteration(t); // Force rehash to the same size. @@ -1870,6 +1913,27 @@ TEST(RawHashSamplerTest, DISABLED_Sample) { 0.01, 0.005); } +TEST(RawHashSamplerTest, DoNotSampleCustomAllocators) { + // Enable the feature even if the prod default is off. + SetHashtablezEnabled(true); + SetHashtablezSampleParameter(100); + + auto& sampler = HashtablezSampler::Global(); + size_t start_size = 0; + start_size += sampler.Iterate([&](const HashtablezInfo&) { ++start_size; }); + + std::vector tables; + for (int i = 0; i < 1000000; ++i) { + tables.emplace_back(); + tables.back().insert(1); + } + size_t end_size = 0; + end_size += sampler.Iterate([&](const HashtablezInfo&) { ++end_size; }); + + EXPECT_NEAR((end_size - start_size) / static_cast(tables.size()), + 0.00, 0.001); +} + #ifdef ADDRESS_SANITIZER TEST(Sanitizer, PoisoningUnused) { IntTable t;