a cache at capacity was weighing every resident key to evict one entry - #9
Merged
Merged
Conversation
Once the memory tier is full, almost every write evicts, so whatever choosing a victim costs is
paid on every write for the life of the cache. Choosing one looked like this:
```rust
let keys = self.memory.keys().cloned().collect::<Vec<_>>();
self.select_eviction_victim(keys)
```
That clones every resident key, scores all of them, groups them, and returns **one** victim -- and
the caller runs it in a `while over_capacity` loop, so a write that has to free several entries
pays that full sweep several times. The cost per write therefore climbs with the number of
resident entries, which is exactly backwards: the fuller and busier the cache, the more each write
costs.
The fix weighs a bounded window of candidates instead, taken least-recently-accessed first, and
falls back to the whole tier when the window turns up nothing evictable (all pinned), so a cache
that can evict at all still evicts.
Measured with `examples/eviction_bench.rs`, which reports both wall time per write and the number
of candidate groups the selector formed per eviction. The second number is the one that settles
the question: it is counted by the cache itself and is immune to load on the machine.
```text
OLD -- weighs every resident key NEW -- bounded candidate window
entries ns/write groups/eviction entries ns/write groups/eviction
1024 2436967 1025.0 1024 285566 512.0
2048 3288870 2049.0 2048 195288 512.0
4096 5863778 4097.0 4096 199734 512.0
8192 10409246 8193.0 8192 203738 512.0
hit rate, working set 4x the cache, 80% of reads on a hot half-cache
1024 82.37 % 1024 82.37 %
4096 81.99 % 4096 81.99 %
```
Groups weighed per eviction is `entries + 1` on the left and a flat 512 on the right: the old
selector inspects the whole resident set, the new one inspects a fixed number of candidates
however large the cache grows. Per write that is 12x at 1024 entries and 51x at 8192, and the
line is still climbing on the left while it is flat on the right, so the distance keeps opening
as a cache fills.
The ladder stops at 8192 because the A/B has to run both arms at the same sizes and the old
selector does not finish the larger ones in a sensible time -- which is the finding, not an
inconvenience.
Hit rate is identical to two decimals on both arms. That is the check that matters here, because
a cheaper selector that quietly evicted better entries would show up as a faster cache that
misses more.
Two details that are load-bearing, and are why this is not simply "scan less":
* The window walks **access** order, not insertion order. An earlier attempt at this walked
insertion order and quietly cost about 11 points of hit rate, because the oldest-inserted entry
is routinely the hottest one.
* The fallback matters for correctness, not just for speed. With every candidate in the window
pinned, a windowed-only selector reports "nothing to evict" and the tier grows past its
capacity. Weighing the whole tier in that case costs no more than weighing everything did
before, and picks exactly the victim the old code would have picked.
The rest of this change brings the tree level with the sources these releases are cut from, which
is how the selector above came to be sitting in one tree and not the other. Included there:
`CacheKeyOrder` gains access tracking (`touch_access` / `iter_access`), the `*_fifo_order` fields
become `*_order` now that they order more than FIFO, and `reset()` on the FIFO and segmented
policies no longer clears the initialized flag -- resetting a cache emptied its index but left the
policy refusing every later write while reporting success, which turned a reset cache into a black
hole.
Verified: 283 unit tests and the doc test pass, and `cargo check --all-targets` is clean against
this crate's own manifest (rocksdb 0.25, thiserror 1) rather than the one the sources came from.
Keeps this branch clean under the Rust version the crate is about to declare.
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.
Once the memory tier is full, almost every write evicts, so whatever choosing a victim costs is
paid on every write for the life of the cache. Choosing one looked like this:
That clones every resident key, scores all of them, groups them, and returns one victim -- and
the caller runs it in a
while over_capacityloop, so a write that has to free several entriespays that full sweep several times. The cost per write therefore climbs with the number of
resident entries, which is exactly backwards: the fuller and busier the cache, the more each write
costs.
The fix weighs a bounded window of candidates instead, taken least-recently-accessed first, and
falls back to the whole tier when the window turns up nothing evictable (all pinned), so a cache
that can evict at all still evicts.
Measured with
examples/eviction_bench.rs, which reports both wall time per write and the numberof candidate groups the selector formed per eviction. The second number is the one that settles
the question: it is counted by the cache itself and is immune to load on the machine.
Groups weighed per eviction is
entries + 1on the left and a flat 512 on the right: the oldselector inspects the whole resident set, the new one inspects a fixed number of candidates
however large the cache grows. Per write that is 12x at 1024 entries and 51x at 8192, and the
line is still climbing on the left while it is flat on the right, so the distance keeps opening
as a cache fills.
The ladder stops at 8192 because the A/B has to run both arms at the same sizes and the old
selector does not finish the larger ones in a sensible time -- which is the finding, not an
inconvenience.
Hit rate is identical to two decimals on both arms. That is the check that matters here, because
a cheaper selector that quietly evicted better entries would show up as a faster cache that
misses more.
Two details that are load-bearing, and are why this is not simply "scan less":
insertion order and quietly cost about 11 points of hit rate, because the oldest-inserted entry
is routinely the hottest one.
pinned, a windowed-only selector reports "nothing to evict" and the tier grows past its
capacity. Weighing the whole tier in that case costs no more than weighing everything did
before, and picks exactly the victim the old code would have picked.
The rest of this change brings the tree level with the sources these releases are cut from, which
is how the selector above came to be sitting in one tree and not the other. Included there:
CacheKeyOrdergains access tracking (touch_access/iter_access), the*_fifo_orderfieldsbecome
*_ordernow that they order more than FIFO, andreset()on the FIFO and segmentedpolicies no longer clears the initialized flag -- resetting a cache emptied its index but left the
policy refusing every later write while reporting success, which turned a reset cache into a black
hole.
Verified: 283 unit tests and the doc test pass, and
cargo check --all-targetsis clean againstthis crate's own manifest (rocksdb 0.25, thiserror 1) rather than the one the sources came from.