Skip to content

execution/cache: track GenericCache entry count in an atomic, not freelru's all-shard Len - #23521

Closed
AskAlexSharov wants to merge 1 commit into
mainfrom
alex/cache_len_contention_36
Closed

execution/cache: track GenericCache entry count in an atomic, not freelru's all-shard Len#23521
AskAlexSharov wants to merge 1 commit into
mainfrom
alex/cache_len_contention_36

Conversation

@AskAlexSharov

Copy link
Copy Markdown
Collaborator

Every insert into a GenericCache below its growth ceiling ran the grow check through freelru.ShardedLRU.Len, which RLocks every shard in turn (up to GOMAXPROCS*16). That serialised writers on shards they never touch — and since ShardedLRU.Get takes the shard's write lock, readers convoyed behind it too.

Fix: pair each generation of the LRU with an atomic entry count, maintained by the insert path (+1) and the existing OnEvict callback (-1), and read that on the hot path. Len(), the ModeNoOp admission check and maybeGrow's re-check all use it.

BenchmarkGenericCacheParallelPutGrow (added), M4 Max, -cpu 10:

                            │  before     │                after                │
                            │   sec/op    │   sec/op     vs base                │
GenericCacheParallelPutGrow   466.3n ± 2%   163.7n ± 9%  -64.90% (p=0.002 n=6)

The gap widens with core count — the laptop only has 256 shards.

Two tests pin the new invariant (counter == the LRU's real length) across insert / update / delete / lazy stale drop / capacity eviction / grow / Clear, sequentially and under concurrency. Both were verified to fail against a mutated counter.

No TDD cycle: this is a performance refactor with no intended behaviour change; the existing suite is the safety net.

Follow-up, not in this PR: growLRU.Add (grow_lru.go, used by CodeCache) has the same per-add Len().

https://claude.ai/code/session_01WFkAYPPhqPe1NXg41Nph78

…elru's all-shard Len

Every insert into a cache below its growth ceiling ran the grow check through
freelru's ShardedLRU.Len, which RLocks all shards in turn (up to
GOMAXPROCS*16). That serialised writers on shards they never touch, and
ShardedLRU.Get takes the shard's write lock, so readers convoyed behind it too.

Pair each generation of the LRU with an atomic entry count maintained by the
insert path and the OnEvict callback, and read that instead.

BenchmarkGenericCacheParallelPutGrow, M4 Max, -cpu 10: 466.3n -> 163.7n
(-64.9%, p=0.002 n=6). The gap widens with core count.

Claude-Session: https://claude.ai/code/session_01WFkAYPPhqPe1NXg41Nph78
@AskAlexSharov

Copy link
Copy Markdown
Collaborator Author

Superseded by #23522 — same commit, branch renamed to alex/cache_len_contention_37 (main is 3.7; release/3.6 already exists).

@AskAlexSharov
AskAlexSharov deleted the alex/cache_len_contention_36 branch August 23, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant