feat(hash): rework hash benchmark - fix latency, add bounded-range throughput - #259
Merged
Conversation
BmHash64Latency previously walked keys by keys[hash & mask], a functional iteration that collapses into a short rho-cycle: it re-hashed only the few keys on the cycle and read as near-zero whenever the cycle hit an empty/tiny key. Replace it with two fixed, documented key-length distributions given as inverse-CDF control points (Short-Identifier, Web-URL). Keys are sampled once with a fixed seed and SHARED across every algorithm in a run, so the length multiset is reproducible and identical for all algorithms; a plain sequential sweep (no hash in the index) then hashes them. Registered as one benchmark per scenario: BmHash64Latency<algo>/<dist>. String building uses absl::StrCat/StrJoin (adds @abseil-cpp//absl/strings). Report generator + README rendering for the new two-scenario latency come in a follow-up PR after re-measuring on clean main.
to the Short-Identifier distribution. Added {1.0, 4096} to the Web-URL
distribution.Refactored LatencyKeys() generation lambda to loop $K-1$
times and append the explicit ceiling length at the end of each set.
Fab-Cat
approved these changes
Jul 14, 2026
Two separately-named latency benchmarks each reported as a lone value with no grouping. Register one family per algo with the scenarios as Args instead (BmHash64Latency<algo>/0, /1) so it is grouped table data like the throughput families; emit the Arg-index -> distribution-name legend as the latency_dists context.
Replace the distribution-aggregate BmHash64Latency with BmHash64Throughput: for each (distribution, upper bound) it hashes the realistic [0..L] length mix truncated and renormalized to that bound, reporting bytes/s -- an (upper-bound -> throughput) curve. Two documented distributions given as inverse-CDF tables (Short <=128 B, Web <=4096 B); key sets built once with a fixed seed and shared byte-for-byte across algorithms; one anchor key pinned at each bound so the boundary is always represented. Sequential sweep (no hash-indexed walk), so no rho-cycle collapse. The exact-length BmHash64 / BmHash128 stay as the per-length latency view.
Replace the stale latency_dists Arg-index legend with throughput_dists: the full
inverse-CDF per distribution ("Short=pct:len,...;Web=..."), so a dataset records
exactly which length mix produced its BmHash64Throughput numbers (provenance;
the benchmark names carry only the bound lengths, not the weights).
helly25
changed the title
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.
Summary
Fixes the
//mbo/hash:hash_benchmarkmixed-length measurement (which produced unreliable, non-reproducible latency numbers) and reframes it. Benchmark-only, by design: the report generator + README rendering land in a follow-up PR after re-measuring on cleanmain.Terminology this PR settles on:
BmHash64/BmHash128<algo>/<len>— unchanged.BmHash64Throughput.The old bug
BmHash64Latencywalked keys bykeys[hash & mask]— a functional iterationhash_{n+1} = f(hash_n)that collapses into a short rho-cycle, re-hashing only a few keys and reading as near-zero whenever the cycle hit an empty/tiny key (e.g.fnv1aat0.27 ns). Removed.New: throughput over upper-bounded length ranges
BmHash64Throughput<algo>/<Short|Web>:<bound>— hashes a realistic[0..L]length mix, truncated and renormalized (kept buckets rescaled to 100%) to each upper boundL, reporting bytes/s → an(upper-bound length -> throughput)curve.Lmax= 128 B (two L1 cache lines / SSO & AVX-512 transition) and 4096 B (one virtual page).keys[counter & mask], no hash-indexed walk) → no rho-cycle; the unpredictable length order still defeats the size-dispatch branch predictor, so the branch-misprediction cost is measured (not modelled).absl::StrCat/absl::StrJoin(adds@abseil-cpp//absl/strings).throughput_distscontext (the full per-distribution inverse-CDF) so every dataset records exactly which mix produced its numbers.Verification
Both distributions produce smooth, monotonic, anomaly-free, reproducible curves across all algorithms (no
0.27collapses);bytes/srises with the bound as larger keys enter the mix.Follow-up (not this PR)
Re-measure on merged
main, then a second PR: report parser/distillforBmHash64Throughput, the latency (exact-length) + throughput (bounded-range) tables/charts, and the README methodology + values.