test: re-measure eviction quality — the README's hit-rate win was a capacity artifact - #23
Closed
ddsha441981 wants to merge 2 commits into
Closed
ddsha441981 wants to merge 2 commits into
ddsha441981 wants to merge 2 commits into
Conversation
The README claimed the highest hit rate of four caches, ~0.9 points over lru. Re-ran that workload with every cache given the same 16,384 entries and the claim does not survive: TypedPulseMap 96.76% vs lru 96.77%, a tie at four figures with 0.01% run-to-run stddev. The old harness built ShardedPulseMap::new(capacity / 64) — 16 shards x 256 buckets x 4 slots = 16,384 nominal — while lru, Moka and QuickCache each got exactly 10,000. lru gains +0.94 points on the same budget, which is the entire margin. The bigger finding is the one the audit was not looking for. The concurrent and sharded paths score 95.60% and 95.57%, 1.16 points below the raw path, because ConcurrentPulseMap::get pushes access events into the AccessBuffer and AccessBuffer::drain has no caller in the crate — so eviction priority is driven by inserts alone. Not inferred: a TypedPulseMap read through peek, which skips the priority update by design, lands on the identical 95.60% +/- 0.04%. Eviction selection is the same find_evict_target call in both maps, and find_free_or_expired vs find_free_slot is inert with TTL off, so the read path is the only variable left. 686198e (v0.6.1) added the table and the example while get still called on_access directly; its 96.73% matches today's raw path. 74a512a (v0.6.2, "66% GET p99 improvement") moved get onto the buffer. The table predates the change that invalidated it. No behaviour change here. Wiring the drain touches sync.rs, which v0.6.5 is validating, so this PR measures and documents; the fix belongs in the release that can afford to move that file. The buffer's module doc, which claimed it "is drained during insert() operations", now says what actually happens and names the two ways out.
The book still ranked PulseMap above LRU on hit rate (four stars to three) and said nothing about which API a reader gets that from. At equal capacity it is a tie, and the concurrent and sharded paths are 1.16 points below the raw path. Same correction as the README, in the page someone actually reads when choosing a policy.
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
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.
Re-ran the workload behind the README's
## Eviction Quality (Hit Rate, Not Speed)table with every cache given the same capacity. The headline claim does not survive, and a second, larger problem turned up underneath it.New
examples/eviction_quality_audit.rs— same workload as Scenario D ofquality_and_readheavy.rs(Zipf 1.3 over 100,000 keys, 2M accesses, get-then-insert-on-miss, 5 seeded trials, every variant sees the identical access sequence within a trial). ~5 s to run:1. The ~0.9-point win over
lruwas a capacity artifactquality_cachesbuildsShardedPulseMap::new(capacity / 64)=new(156). Each shard rounds156 → 256buckets (next_power_of_two), so the map is 16 × 256 × 4 = 16,384 nominal slots — whilelru, Moka and QuickCache were each handed exactly 10,000.Give
lruthe same 16,384 and it gains +0.94 points (95.83% → 96.77%). That is the whole margin. At equal capacityTypedPulseMap96.76% ± 0.01% againstlru96.77% ± 0.01% is a tie at four significant figures.PulseMap capacity is always
pow2 buckets × 4, so a round 10,000 was never reachable — 16,384 is the nearest budget both sides can be given, which is what the new harness does.2. On the concurrent and sharded paths, reads never reach the eviction policy — 1.16 points
ConcurrentPulseMapscores 95.60% where the raw path scores 96.76%.Proven rather than inferred, by control: a
TypedPulseMapread throughpeek— which deliberately skips the priority update — lands on the identical 95.60% ± 0.04%. Eviction selection is the samefind_evict_targetcall in both maps, and thefind_free_or_expired/find_free_slotdifference is inert with TTL disabled. The read path is the only variable left.Root cause:
AccessBuffer::drainhas no caller anywhere in the crate.ConcurrentPulseMap::getpushes access events into the buffer instead of updating theMetaWordinline; nothing drains them, so eviction priority is driven by inserts alone and the buffer fills once (4096 events) and rejects every push after that. The module doc claimed "the buffer is drained duringinsert()operations" — corrected here to state what actually happens and name the two ways out: wire the drain intoinsert_internal, or delete the module and its loom tests.Sharding is not the culprit:
ShardedPulseMap95.57% vsConcurrentPulseMap95.60% is −0.02 points, inside the noise.Where it entered
686198e(2026-08-05, v0.6.1) added both the README table and the example, whenConcurrentPulseMap::getstill calledon_accessdirectly. Its 96.73% matches today's raw-path 96.76%.74a512a(2026-08-11, v0.6.2, "atomic MetaWord + deferred access buffer — 66% GET p99 improvement") movedgetonto the buffer.git merge-base --is-ancestor 686198e 74a512a→ yes. The table predates the change that invalidated it and was never re-measured.Harness validation
It reproduces the figure it was meant to reproduce:
lruat 10,000 gives 95.83% ± 0.01%, matching the README exactly.What changed in the docs
README.md— the table now lists all three APIs with their capacities and whether reads reach the policy; the two corrections are stated plainly; Scenario D says "tie withlru" instead of "Highest hit rate of all 4 caches"; the closing "PulseMap's cache hit rate was still the highest of the four" is replaced with the measured position. Moka's and QuickCache's figures are marked unverified — this run did not re-measure them, and their old numbers came from the same mismatched comparison.CHANGELOG.md— appended inside the existing## [Unreleased] — v0.6.5.Scope
No behaviour change. Wiring the drain touches
src/sync.rs, which is exactly what v0.6.5 is validating — changing it and validating it in the same release is not worth much. This PR measures and documents; the fix belongs in the release that can afford to move that file.One thing left open, stated rather than quietly ignored: the scan-resistance table further down the README (1000/1000 hot keys surviving) was produced by a harness that is not in the repo. Its wording ("survivors counted with
peek") points at the raw path, which this audit shows does apply reads to the policy — but that is inference, not a measurement, and it has not been re-run here.