feat(hash): faster constexpr hash (mbo::hash::mh) + hardening & tests - #208
Merged
Conversation
Add mbo::hash::GetHash64 / GetHash128 and the Hash128 type (namespace mbo::hash::mh), now the default behind mbo::hash::GetHash*, and deprecate mbo::hash::simple::GetHash. Implementation: - Dual-lane rotate-multiply core with full-width odd multipliers and a MurmurHash3 fmix64 finalize (strong avalanche; ~2x simple's throughput on >=1KB inputs). - Endian-independent little-endian byte loads (portable across platforms). - Small-input (<=32B) fast path for GetHash64: single lane + one finalize so short keys don't pay the 128-bit fold overhead. - Hash128 gains dependency-free AbslHashValue / AbslStringify. Testing: - Templated test/benchmark framework (hash_test_util.h) that drives multiple algorithms and detects 64- vs 128-bit ones; covers constexpr==runtime, empty/null, distinctness, collisions, and avalanche on both code paths. - google_benchmark target (length-bucketed) to substantiate throughput. Also: include/NOLINT/IWYU cleanups, accurate GetHash docs + stability note (values not stable across versions / not for persistence), CHANGELOG + README, and bump module to 0.13.0.
Deriving the mangle seed straight from __DATE__/__TIME__ produced a full 64-bit, effectively unbounded seed that changed every compile, baking a different constant into every build and defeating reproducible builds / build caches. Now the date/time hash only selects a bucket via `% kMangleSeedCount` (default 16), and the bucket is expanded into a full-width constant so the whole hash is still mangled. This bounds the number of possible outputs while keeping some per-build variation; a hermetic build (pinned macros) collapses to one bucket. Adds tests for the seed-independent mangle properties.
GetHash was hard-wired to the default 64-bit hash. Make it a defaulted template `GetHash<Hash64Fn = &mh::GetHash64>(data, seed)` so the same build-seed mangle can wrap any implementation matching `Hash64Fn = uint64_t(*)(std::string_view, uint64_t) noexcept`. Because default arguments don't apply through a function pointer, the pluggable contract takes the seed explicitly and GetHash forwards it (default seed hoisted to the shared `mh::kDefaultSeed`). Existing `GetHash(data)` calls are unchanged. Adds a pluggability test.
Promote GetHash's seed from a runtime default argument to a second non-type template parameter (default mh::kDefaultSeed), matching the hash-function parameter. GetHash(data) is now a clean single-argument call; the seed is customized at compile time via GetHash<&Fn, Seed>(data).
Adds an explicit, build-wide switch MBO_HASH_MANGLE (default 1). Defining it to 0 makes HashMangle the identity, so GetHash == GetHash64 -- fully reproducible values for reproducible builds / golden tests. Deliberately NOT tied to -O level or NDEBUG: HashMangle is inline constexpr, so a per-config value would be an ODR violation and would make debug and release builds disagree (a worse footgun than the mangle). A team wanting opt to disable it sets -DMBO_HASH_MANGLE=0 in that config explicitly. Adds hash_no_mangle_test (built with the flag off) so the disabled path stays covered in CI.
Fab-Cat
approved these changes
Jul 2, 2026
This was referenced Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardens and tests the new hash work, and makes it the default.
What
mbo::hash::GetHash64/GetHash128+Hash128type (namespacembo::hash::mh), now the default behindmbo::hash::GetHash*.mbo::hash::simple::GetHashis deprecated (kept assimple::GetHash64).Implementation
fmix64finalize. Strong avalanche (verified ~0.50), ~2×simple's throughput on ≥1 KB.bit_cast).GetHash64: single lane + one finalize so short keys don't pay the 128-bit fold overhead (16 B: 9.8 GiB/s vs simple 6.8; 1 B improved 2.9→1.7 ns).Hash128gets dependency-freeAbslHashValue/AbslStringify(no abseil dep added tohash_cc).Testing
hash_test_util.h): algorithm descriptors + aHasHash128/kHashBitstrait that detects 64- vs 128-bit algorithms.hash_test.ccrunsTYPED_TESTs over{simple, mh}— constexpr==runtime, empty/null, distinctness, collisions, and avalanche on both code paths. The 128-bit test auto-skips for the 64-bit-onlysimple.hash_benchmark(google_benchmark,tags=["manual"]): length-bucketed throughput to substantiate the perf claims.Fixes rolled in (from the review)
Missing
<bit>/<cstddef>/<string_view>includes, malformed NOLINT, IWYU private pragmas on impl headers, accurateGetHashdoc + stability note (values are not stable across versions and not for persistence/crypto), and CHANGELOG/README reconciliation ("deprecated", not "removed").Perf note (reviewer FYI)
For 1–4 byte keys
simpleis still marginally faster (inherentfmix64finalize cost);mhwins or ties at typical key sizes (≥16 B) and is ~2× faster on large inputs. Hash values are intentionally not stable across versions.