perf: in-memory and sharded store speedups; promote on cache_set overwrite - #299
Open
jaemk wants to merge 9 commits into
Open
perf: in-memory and sharded store speedups; promote on cache_set overwrite#299jaemk wants to merge 9 commits into
cache_set overwrite#299jaemk wants to merge 9 commits into
Conversation
…ed per-op paths Adds criterion coverage for the paths that were unmeasured: O(n) sweeps (`evict`/`retain`/`retain_latest`/`key_order`/`iter_order`/`iter`) at N=10k, the `refresh_on_hit` hit path, `cache_set` over a live key with and without `on_evict`, whole-cache `clear`/`len`/`metrics`, single-threaded `cache_get` for all six sharded stores, an 8-thread expiry storm where every read is a lazy eviction, an 8-thread `len`/`metrics` poll, sharded `build()` time, a large-value LRU group, and `Instant::now()` itself. Sweep and clear benchmarks use `iter_batched` so each sample rebuilds a populated cache and measures the sweep rather than an already-swept one. Every entry-removing op gets a `String`-key variant alongside the `usize` one, since key-clone and re-hash costs are invisible with `Copy` keys.
`TtlCache` and `TtlSortedCache` read the clock twice on several paths: once to test liveness and again to recompute an expiry or judge a displaced entry. On this machine `Instant::now()` costs ~19ns against a ~9ns hash-and-probe, so the redundant read dominated. Both stores now sample once per operation and thread it through, including the async get-or-set families. `retain` samples once per pass rather than once per entry, matching `evict`, which already did. `LruCache` sweeps cloned every doomed key and then re-hashed it to remove the entry. `LRUList` gains `iter_indices` and `drain_into`, and `LruCache` gains `remove_index`, `drain_all`, and `pop_raw_with_hash`, so `retain`, `retain_silent`, and `cache_clear_with_on_evict` work from slot indices instead. `cache_clear_with_on_evict` no longer hashes at all. The four order methods pre-size their `Vec` from the live count. `TtlSortedCache::evict` detached the expired prefix with a counted range scan followed by one `pop_first` per entry. It now uses a single `split_off` and drains the detached tree by value, with no rebalancing. `set_inner` returns the stamp it indexed so `set_and_get_mut` needs neither a second key clone nor a re-lookup, and the occupied branch reuses the stored key `Arc`. `StripedCounter` gains `increment_mut` for the `&mut self` read paths, which skips the thread-local lookup and the atomic RMW that shared access requires. `ExpiringCache` wrapped an `UnboundCache` it bypassed for every read and write, carrying two `StripedCounter`s it never incremented. It now holds its `HashMap` directly. Sharded stores cache the default shard count, which parses several /proc files per build, and drop the read guard before bumping hit/miss counters. Measured against a pristine build: `UnboundCache` hit -44%, `LruCache` `cache_clear_with_on_evict` 2.67x, `TtlSortedCache::evict` 1.70x, `TtlCache` overwrite -32 to -38%, `TtlCache` refresh-on-hit -30 to -35%, `retain` on String keys -35%. Behavior changes, all making a variant agree with its siblings: - The `get_or_set` family on `TtlSortedCache` no longer sweeps an expired entry before running the factory. A cancelled or panicking factory now leaves the entry in place and fires no `on_evict`, matching the sync variants, `TtlCache`, and `LruTtlCache`. On success the callback fires after the factory rather than before. - For a key type whose `Eq` ignores part of its payload, `TtlSortedCache` reports the first-inserted key to `on_evict` and `cache_remove_entry` rather than the most recent, matching `HashMap::insert`. - `TtlSortedCache::retain_latest` counts an eviction before firing `on_evict`, so a panicking callback no longer removes an entry without counting it.
…ne-pass sweeps `ShardedLruTtlCache::cache_get` and `ShardedExpiringLruCache::cache_get` did two full hash-and-probe lookups per read hit, both inside the exclusive write lock: `cache_peek` to test liveness, then `cache_get` to fetch. They now use `get_if`, falling through to `pop_raw` only on a miss to tell expired from absent, the shape `cache_get_with_expiry_status` already used. Combined with hoisting the clock read out of the critical section, four-thread read throughput goes up 84% on `ShardedLruTtlCache` and 16% on `ShardedExpiringLruCache`. The expiry-capable sharded stores each kept one process-wide eviction counter in their `Arc<Inner>`, contended from every core and sharing a cache line with `shards`, `shard_mask`, `ttl_nanos` and `refresh`, all read on every operation. Eviction counting moves to a per-shard counter alongside the existing hit and miss counters, summed on read. Capacity and non-capacity evictions stay disjoint. `evict` on the map-backed sharded stores cloned every expired key into a `Vec` and then re-hashed each one to remove it. It is now a single `extract_if` pass, with a `retain` plus length delta when no callback is set. That is 3.2x on `ShardedTtlCache` and 3.0x on `ShardedExpiringCache` with `String` keys. `cache_clear_with_on_evict` across the LRU family walks the recency chain with `drain_all` instead of cloning every key and re-hashing it: 2.4x. `available_parallelism` parses several /proc files and ran on every sharded cache construction. Caching it drops `build()` by 92 to 99%. `ExpiringLruCache::cache_set` cloned the key on every insert whenever `on_evict` was set, though the clone was only used on the rare expired-displacement branch. It now takes the displaced entry from the store instead. Behavior changes: - `ExpiringLruCache::cache_set` fires `on_evict` with the stored key rather than the caller's key, matching `LruTtlCache`. Observable for key types whose `Eq` ignores part of the payload. - The sharded TTL and LRU-TTL stores decide expiry against the caller's own clock sample, taken before the shard lock is acquired. Under contention an entry that crosses its expiry while the caller queues is served as live. This stays inside the documented lazy-expiry contract, which never promised prompt removal. Also records the intended `max_size`-bounded default shard count as a design note; the builders are not wired to it yet.
…motion Assert both link directions after promoting an already-head entry and the only entry, via a new `order_reversed` helper that walks the `prev` chain. A ring whose `prev`/`next` links disagree (e.g. a `move_to_front` that skips `unlink`) corrupts into a cycle that never reaches the `OCCUPIED` sentinel, which turns forward iteration into an unbounded loop; these assertions catch that before it can happen.
`cache_set` and `cache_set_returning_entry` over an existing key now move that entry to most-recently-used instead of replacing it in place. A write counts as an access, so an overwrite behaves like a fresh insertion. This applies to `LruCache`, `LruTtlCache`, `ExpiringLruCache`, `ShardedLruCache`, `ShardedLruTtlCache`, and `ShardedExpiringLruCache`, and it changes which entry a capacity eviction selects in overwrite-heavy workloads: the overwritten key survives longer and another key becomes the victim. The sharded LRU-TTL and expiring-LRU stores previously promoted only on their `on_evict` branch (via a `get_or_set_with_if` helper) and not on the no-callback branch, so attaching a purely observational `on_evict` changed eviction order. Both branches now go through the promoting primitive and the helper is removed; the divergence is gone. `cache_peek`, `cache_peek_with_expiry_status`, and `cache_contains` remain non-promoting, so a write and a peek stay distinguishable. Inserting a new key still goes to the front via `push_front` and is unchanged. Add `specs/design/0038`, the LRU-7 behavior statement in store-lru.md, and certification tests covering promotion, the eviction-victim shift, head and sole-entry `move_to_front`, and sharded semantics/concurrency.
3.4.0 emits plain `rust` code fences and drops the hidden `# pub fn main` lines that 3.3.3 kept. CI installs the latest cargo-readme, so `check/readme` failed against the 3.3.3-generated file. No source doc comments changed.
…deflaked tests - `ttl_sorted::evict_at` no-callback branch: drain the expired prefix into a Vec and count evictions per removal so a value `Drop` panic leaves `map` and `keys` in lockstep instead of orphaning rows - narrow the before-lock clock-sample contract in `specs/store-sharded.md` to the TTL-family stores; the `Expires`-family decides expiry under the write lock; add `ShardedLruTtlCache` to the matching CHANGELOG entry - mark the spec 0037 default shard cap as planned, not yet wired, in the spec and design record; fix the `default_shard_count_for_capacity` count-vs-capacity doc - document promote-on-overwrite on the `Cached::cache_set` trait doc; drop `cache_clear_with_on_evict` from the per-shard eviction-counter docs and the stale global-counter bench comment - pre-size `doomed_indices` in `lru::retain` - raise TTLs in the refresh-on-hit perf and sharded-semantics tests, make the sharded flip-stress test assert real evictions, and cover the factory-error miss count for `cache_try_get_or_set_with_mut`
… zero-margin peek-refresh test - `ConcurrentCached::cache_set`: a displaced expired entry is filtered to `None` and delivered to `on_evict`, not returned, so a returned `Some` is always live; add the promote-on-overwrite note. - `store-expiring.md` EXPIRE-7 and `store-lru.md` LRU-6: `on_evict` fires on an overwrite only when the displaced entry has already expired. A plain `LruCache` overwrite returns the old value and fires no callback. - `TtlSortedCache::set` doc: size-limit trimming runs on every `set`, independent of the `.evict()` expiry-sweep opt-in. - `peek_and_contains_do_not_refresh_the_expiry`: shorten the final sleep so the read lands between the original and refreshed deadlines, so a peek that wrongly refreshes expiry now fails the test.
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
1 participant
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.
Performance work across the in-memory and sharded stores, plus a behavior change to
cache_setrecency.Breaking
cache_set(andcache_set_returning_entry) over an existing key now promote that key to most-recently-used instead of replacing it in place. AffectsLruCache,LruTtlCache,ExpiringLruCache,ShardedLruCache,ShardedLruTtlCache, andShardedExpiringLruCache, and changes which entry a capacity eviction selects in overwrite-heavy workloads.cache_peek/cache_peek_with_expiry_status/cache_containsstay non-promoting. Seespecs/design/0038-cache-set-promotes-on-overwrite.md.on_evictdivergence on the sharded LRU-TTL and expiring-LRU stores: the callback and no-callback write branches now share one promoting primitive, so configuring an observationalon_evictno longer changes eviction order.Changed
ExpiringLruCache::cache_setfireson_evictwith the stored key, matchingLruTtlCache, and drops an unconditional key clone from every set.TtlSortedCachereuses the stored key on overwrite, itsget_or_setfamily no longer removes an expired entry before running the initializer, andretain_latestcounts an eviction before firingon_evict.Tests
move_to_frontring integrity, sharded semantics and concurrency, stored-key lifecycle, and the LRU-TTL / expiring-LRU parity.Redis tests require a live server (
CACHED_REDIS_CONNECTION_STRING); CI runs--all-features.