Skip to content

Commit cf2b48c

Browse files
ezbrcopybara-github
authored andcommitted
Perform stronger mixing on 32-bit platforms and enable the LowEntropyStrings test.
Also avoid the conditional use of intrinsic 128-bit integers in Mix(), which is no longer needed. There is no generated code difference for x86-64 clang. PiperOrigin-RevId: 780592497 Change-Id: I2e0b05a78521f42e3eebcb7e99e8387c34fe7e33
1 parent ea50280 commit cf2b48c

File tree

2 files changed

+4
-30
lines changed

2 files changed

+4
-30
lines changed

absl/hash/hash_test.cc

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,11 +1266,6 @@ TEST(PrecombineLengthMix, ShortStringCollision) {
12661266
// Test that we don't cause excessive collisions on the hash table for
12671267
// doubles in the range [-1024, 1024]. See cl/773069881 for more information.
12681268
TEST(SwisstableCollisions, DoubleRange) {
1269-
#ifdef GOOGLE_UNSUPPORTED_OS_LOONIX
1270-
// TODO(b/424834054): make this test pass on Loonix.
1271-
GTEST_SKIP() << "Test fails on Loonix.";
1272-
#endif
1273-
12741269
absl::flat_hash_set<double> set;
12751270
for (double t = -1024.0; t < 1024.0; t += 1.0) {
12761271
set.insert(t);
@@ -1282,12 +1277,6 @@ TEST(SwisstableCollisions, DoubleRange) {
12821277
// Test that for each pair of adjacent bytes in a string, if there's only
12831278
// entropy in those two bytes, then we don't have excessive collisions.
12841279
TEST(SwisstableCollisions, LowEntropyStrings) {
1285-
if (sizeof(size_t) < 8) {
1286-
// TODO(b/424834054): make this test pass on 32-bit platforms. We need to
1287-
// make 32-bit Mix() stronger.
1288-
GTEST_SKIP() << "Test fails on 32-bit platforms";
1289-
}
1290-
12911280
constexpr char kMinChar = 0;
12921281
constexpr char kMaxChar = 64;
12931282
// These sizes cover the different hashing cases.
@@ -1302,7 +1291,7 @@ TEST(SwisstableCollisions, LowEntropyStrings) {
13021291
set.insert(s);
13031292
ASSERT_LT(HashtableDebugAccess<decltype(set)>::GetNumProbes(set, s),
13041293
64)
1305-
<< size << " " << b;
1294+
<< "size: " << size << "; bit: " << b;
13061295
}
13071296
}
13081297
}

absl/hash/internal/hash.h

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -955,24 +955,9 @@ inline uint64_t PrecombineLengthMix(uint64_t state, size_t len) {
955955
}
956956

957957
ABSL_ATTRIBUTE_ALWAYS_INLINE inline uint64_t Mix(uint64_t lhs, uint64_t rhs) {
958-
// For 32 bit platforms we are trying to use all 64 lower bits.
959-
if constexpr (sizeof(size_t) < 8) {
960-
uint64_t m = lhs * rhs;
961-
return m ^ absl::byteswap(m);
962-
}
963-
// absl::uint128 is not an alias or a thin wrapper around the intrinsic.
964-
// We use the intrinsic when available to improve performance.
965-
// TODO(b/399425325): Try to remove MulType since compiler seem to generate
966-
// the same code with just absl::uint128.
967-
// See https://gcc.godbolt.org/z/s3hGarraG for details.
968-
#ifdef ABSL_HAVE_INTRINSIC_INT128
969-
using MulType = __uint128_t;
970-
#else // ABSL_HAVE_INTRINSIC_INT128
971-
using MulType = absl::uint128;
972-
#endif // ABSL_HAVE_INTRINSIC_INT128
973-
// Though the 128-bit product on AArch64 needs two instructions, it is
974-
// still a good balance between speed and hash quality.
975-
MulType m = lhs;
958+
// Though the 128-bit product needs multiple instructions on non-x86-64
959+
// platforms, it is still a good balance between speed and hash quality.
960+
absl::uint128 m = lhs;
976961
m *= rhs;
977962
return Uint128High64(m) ^ Uint128Low64(m);
978963
}

0 commit comments

Comments
 (0)