Skip to content

Commit

Permalink
Mix pointers twice in build modes in which the PointerAlignment test …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
ezbr authored and copybara-github committed Jan 3, 2025
1 parent f339ea3 commit d910383
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 11 deletions.
13 changes: 2 additions & 11 deletions absl/hash/hash_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions absl/hash/internal/hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,11 @@ std::enable_if_t<std::is_pointer<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);
}

Expand Down

0 comments on commit d910383

Please sign in to comment.