From d910383b492a2941168ee458ed93abd3c4f1ff37 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Fri, 3 Jan 2025 09:49:48 -0800 Subject: [PATCH] Mix pointers twice in build modes in which the PointerAlignment test is flaky if we mix once. This reverts to the behavior from before a recent hashing change in these build modes. Note that load tests showed a positive impact for that change, but the load tests weren't run in these build modes with lower pointer entropy. PiperOrigin-RevId: 711779528 Change-Id: Ie5a777c9a7e32a1417968ef048c496b17524b8e3 --- absl/hash/hash_test.cc | 13 ++----------- absl/hash/internal/hash.h | 5 +++++ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/absl/hash/hash_test.cc b/absl/hash/hash_test.cc index 7bf5a80cc04..66efd2cf2fc 100644 --- a/absl/hash/hash_test.cc +++ b/absl/hash/hash_test.cc @@ -164,8 +164,6 @@ TEST(HashValueTest, Pointer) { std::make_tuple(&i, ptr, nullptr, ptr + 1, n))); } -// The test is flaky in ASan and on Apple platforms. -#if !defined(ABSL_HAVE_ADDRESS_SANITIZER) && !defined(__APPLE__) TEST(HashValueTest, PointerAlignment) { // We want to make sure that pointer alignment will not cause too many bits to // be stuck. @@ -192,18 +190,11 @@ TEST(HashValueTest, PointerAlignment) { // Limit the scope to the bits we would be using for Swisstable. constexpr size_t kMask = (1 << (kLog2NumValues + 7)) - 1; size_t stuck_bits = (~bits_or | bits_and) & kMask; - // Test that there are less than 3 stuck bits. Sometimes we see stuck_bits + // Test that there are at most 2 stuck bits. Sometimes we see stuck_bits // of 0x3. - size_t stuck_bits_threshold = 3; -#ifdef __ANDROID__ - // On Android, we sometimes see stuck_bits of 0x780 when align is 11520. - stuck_bits_threshold = 5; -#endif - EXPECT_LT(absl::popcount(stuck_bits), stuck_bits_threshold) - << "0x" << std::hex << stuck_bits; + EXPECT_LE(absl::popcount(stuck_bits), 2) << "0x" << std::hex << stuck_bits; } } -#endif // !defined(ABSL_HAVE_ADDRESS_SANITIZER) && !defined(__APPLE__) TEST(HashValueTest, PointerToMember) { struct Bass { diff --git a/absl/hash/internal/hash.h b/absl/hash/internal/hash.h index f70170b122d..797183adeb9 100644 --- a/absl/hash/internal/hash.h +++ b/absl/hash/internal/hash.h @@ -460,6 +460,11 @@ std::enable_if_t::value, H> AbslHashValue(H hash_state, // Due to alignment, pointers tend to have low bits as zero, and the next few // bits follow a pattern since they are also multiples of some base value. The // byte swap in WeakMix helps ensure we still have good entropy in low bits. +#if defined(__APPLE__) || defined(__ANDROID__) || \ + defined(ABSL_HAVE_ADDRESS_SANITIZER) + // In these build modes, pointers may have less entropy so mix pointers twice. + return H::combine(std::move(hash_state), v, v); +#endif return H::combine(std::move(hash_state), v); }