From 8675393f2ee8dddf1d75f57da6cebafbf4cbc166 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Sun, 23 Aug 2026 18:36:47 +0700 Subject: [PATCH 1/7] execution/cache: track GenericCache entry count in an atomic, not freelru'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. --- execution/cache/cache_test.go | 49 ++++++++++++ execution/cache/generic_cache.go | 75 ++++++++++++------- .../cache/generic_cache_concurrency_test.go | 46 ++++++++++++ 3 files changed, 143 insertions(+), 27 deletions(-) diff --git a/execution/cache/cache_test.go b/execution/cache/cache_test.go index f0d8a7e02cd..580ad509e09 100644 --- a/execution/cache/cache_test.go +++ b/execution/cache/cache_test.go @@ -1771,3 +1771,52 @@ func BenchmarkPublishVsViewBindLock(b *testing.B) { }) } } + +// The O(1) entry counter that replaced freelru's all-shard Len on the grow +// check must not drift from the LRU's real length on any mutation path. +func TestGenericCache_LenTracksLRU(t *testing.T) { + c := closeOnCleanup(t, NewGenericCacheWithAvg[[]byte](8*datasize.MB, 256, func(v []byte) int { return len(v) }, ModeEvictLRU)) + key := func(i int) []byte { + k := make([]byte, 8) + binary.BigEndian.PutUint64(k, uint64(i)) + return k + } + check := func(phase string) { + t.Helper() + require.Equal(t, c.data.Load().lru.Len(), c.Len(), "entry counter drifted after %s", phase) + } + + for i := range 500 { + c.Put(key(i), []byte("v"), 10) + } + check("inserts") + + for i := range 200 { + c.Put(key(i), []byte("updated"), 20) + } + check("updates") + + for i := range 100 { + c.Delete(key(i)) + } + check("deletes") + + // Floor 15 leaves the txNum-10 entries live and strands the txNum-20 ones, + // which their next read drops. + c.Unwind(15) + for i := 100; i < 500; i++ { + c.Get(key(i)) + } + check("stale drops") + + before := c.data.Load() + for i := 500; i < 4000; i++ { + c.Put(key(i), []byte("v"), 30) + } + require.NotEqual(t, before, c.data.Load(), "grow did not happen") + check("grow") + + c.Clear() + require.Equal(t, 0, c.Len()) + check("clear") +} diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 926c664c30e..25dc4397e41 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -58,6 +58,26 @@ type entry[T any] struct { epoch uint32 // unwind generation the entry was written in } +// lruGen is one generation of the sharded LRU plus an O(1) live-entry count. +// freelru's own Len RLocks every shard, so the grow check — which every insert +// runs while the cache is below its ceiling — cannot use it without serialising +// writers that would otherwise touch disjoint shards. +type lruGen[T any] struct { + lru *freelru.ShardedLRU[uint64, entry[T]] + n atomic.Int64 +} + +func (g *lruGen[T]) len() int { return int(g.n.Load()) } + +// add inserts a key the LRU does not already hold — every call site removes an +// existing one first — so the count rises by one; a capacity eviction inside +// freelru fires OnEvict, which takes it back down. +func (g *lruGen[T]) add(h uint64, e entry[T]) (evicted bool) { + evicted = g.lru.Add(h, e) + g.n.Add(1) + return evicted +} + // GenericCache is a sharded, LRU-evicting bounded cache for key-value // data. Eviction mode is fixed at construction (see policy.go). type GenericCache[T any] struct { @@ -65,7 +85,7 @@ type GenericCache[T any] struct { // held — on a jump-grow (fully copied generation) and on Clear (fresh // empty one) — so no write lands in a retired generation and no reader // sees a partial copy (see maybeGrow, Clear). - data atomic.Pointer[freelru.ShardedLRU[uint64, entry[T]]] + data atomic.Pointer[lruGen[T]] capacityB datasize.ByteSize mode Mode @@ -198,15 +218,17 @@ func newGenericCacheEntries[T any](capacityBytes datasize.ByteSize, capacityEntr // entry. The callback must not feed the evictions metric — it also fires for // intentional Removes — so capacity evictions are counted from Add's evicted // return at the call sites. -func (c *GenericCache[T]) newShards(capacity, shards uint32) *freelru.ShardedLRU[uint64, entry[T]] { +func (c *GenericCache[T]) newShards(capacity, shards uint32) *lruGen[T] { lru, err := freelru.NewShardedWithSize[uint64, entry[T]](shards, capacity, capacity+capacity/4, u64identity) if err != nil { panic(err) } + g := &lruGen[T]{lru: lru} lru.SetOnEvict(func(_ uint64, e entry[T]) { c.currentSize.Add(-int64(e.size)) + g.n.Add(-1) }) - return lru + return g } // maybeGrow jump-resizes the LRU one step larger when it is full, the ceiling @@ -226,7 +248,7 @@ func (c *GenericCache[T]) maybeGrow() { old := c.data.Load() curCap := c.curCap.Load() - if curCap >= c.maxCap || old.Len() < int(curCap) { + if curCap >= c.maxCap || old.len() < int(curCap) { return } newCap := min(curCap*genericCacheGrowFactor, c.maxCap) @@ -251,9 +273,9 @@ func (c *GenericCache[T]) maybeGrow() { c.putStripes[i].Lock() } copied, evicted := 0, 0 - for _, k := range old.Keys() { - if v, ok := old.Get(k); ok { - if next.Add(k, v) { + for _, k := range old.lru.Keys() { + if v, ok := old.lru.Get(k); ok { + if next.add(k, v) { evicted++ } copied++ @@ -320,8 +342,8 @@ func (c *GenericCache[T]) GetWithTxNum(key []byte) (T, uint64, bool) { // snapshot can only cause a safe miss because dropStale rechecks the current // generation before removing it. coh := c.coh.Snapshot() - lru := c.data.Load() - e, ok := lru.Get(h) + gen := c.data.Load() + e, ok := gen.lru.Get(h) if !ok || !bytes.Equal(e.key, key) { c.misses.Add(1) var zero T @@ -369,8 +391,7 @@ func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) // putStriped performs the write under the key's stripe and reports whether the // insert landed in a full LRU with ceiling headroom, i.e. the caller should -// grow. Detection stays on the insert path — Len locks every shard, too costly -// per warm update. +// grow. func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrite bool) bool { h := maphash.Hash(key) valBytes := c.sizeFunc(value) @@ -384,8 +405,8 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit // generation swap and coherence reset, so the stamp cannot belong to a // different generation from the one where the entry lands. ep := c.coh.Epoch() - lru := c.data.Load() - existing, hasExisting := lru.Get(h) + gen := c.data.Load() + existing, hasExisting := gen.lru.Get(h) // Existing key — update by remove-then-add (see newShards for why a size // delta would be wrong). Reuse the stored key buffer to avoid an extra @@ -399,8 +420,8 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit // another stripe over-admits past the budget. Over-stating is safe — at // worst a new key is dropped, which is within "drop new keys when full". c.currentSize.Add(int64(newSize)) - lru.Remove(h) - if lru.Add(h, entry[T]{key: existing.key, val: value, size: newSize, txNum: txNum, epoch: ep}) { + gen.lru.Remove(h) + if gen.add(h, entry[T]{key: existing.key, val: value, size: newSize, txNum: txNum, epoch: ep}) { c.evictions.Add(1) } return false @@ -409,7 +430,7 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit if c.mode == ModeNoOp { // Refuse once full by either bound — freelru would otherwise evict at the // entry-count cap, which ModeNoOp ("drop new keys when full") must not do. - if c.currentSize.Load()+int64(newSize) > int64(c.capacityB) || lru.Len() >= int(c.maxCap) { + if c.currentSize.Load()+int64(newSize) > int64(c.capacityB) || gen.len() >= int(c.maxCap) { c.dropped.Add(1) return false } @@ -419,7 +440,7 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit // The insert lands before the grow (which must run outside the stripe), so // it and any racers until the swap evict at the pre-grow cap — a transient // bounded by the grow window. - needGrow := c.mode != ModeNoOp && curCap < c.maxCap && lru.Len() >= int(curCap) + needGrow := c.mode != ModeNoOp && curCap < c.maxCap && gen.len() >= int(curCap) // In ModeEvictLRU the byte budget is enforced through the entry-count cap, // not a separate currentSize check: capacityEntries is derived from @@ -439,10 +460,10 @@ func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrit // is reserved before the removal (see the update path above). c.currentSize.Add(int64(newSize)) if hasExisting { - lru.Remove(h) + gen.lru.Remove(h) } keyCopy := bytes.Clone(key) - if lru.Add(h, entry[T]{key: keyCopy, val: value, size: newSize, txNum: txNum, epoch: ep}) { + if gen.add(h, entry[T]{key: keyCopy, val: value, size: newSize, txNum: txNum, epoch: ep}) { c.evictions.Add(1) } c.inserts.Add(1) @@ -457,9 +478,9 @@ func (c *GenericCache[T]) Delete(key []byte) { mu := &c.putStripes[h&(putStripeCount-1)] mu.Lock() defer mu.Unlock() - lru := c.data.Load() - if existing, ok := lru.Get(h); ok && bytes.Equal(existing.key, key) { - lru.Remove(h) + gen := c.data.Load() + if existing, ok := gen.lru.Get(h); ok && bytes.Equal(existing.key, key) { + gen.lru.Remove(h) } } @@ -470,9 +491,9 @@ func (c *GenericCache[T]) dropStale(h uint64, key []byte) { mu := &c.putStripes[h&(putStripeCount-1)] mu.Lock() defer mu.Unlock() - lru := c.data.Load() - if e, ok := lru.Get(h); ok && bytes.Equal(e.key, key) && c.coh.IsStale(e.txNum, e.epoch) { - lru.Remove(h) + gen := c.data.Load() + if e, ok := gen.lru.Get(h); ok && bytes.Equal(e.key, key) && c.coh.IsStale(e.txNum, e.epoch) { + gen.lru.Remove(h) } } @@ -533,7 +554,7 @@ func (c *GenericCache[T]) Unwind(unwindToTxNum uint64) { // Len returns the number of entries in the cache. func (c *GenericCache[T]) Len() int { - return c.data.Load().Len() + return c.data.Load().len() } // SizeBytes returns the current size of the cache in bytes. @@ -566,7 +587,7 @@ func (c *GenericCache[T]) PrintStatsAndReset(name string) { "hits", hits, "misses", misses, "hit_rate", hitRate, "inserts", inserts, "evictions", evictions, "dropped", dropped, "stale_evicted", staleEvicted, "epoch", c.coh.Epoch(), - "entries", c.data.Load().Len(), "size_mb", sizeBytes/(1024*1024), + "entries", c.data.Load().len(), "size_mb", sizeBytes/(1024*1024), "capacity_mb", int64(c.capacityB/datasize.MB), "usage_pct", usagePct, ) } diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index a4bdc74a1ea..82dd4217763 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -417,3 +417,49 @@ func TestGenericCache_StatsResetAtomicWithDelete_NoPhantomEvictions(t *testing.T total += c.evictions.Swap(0) require.Zero(t, total, "intentional removals surfaced in the evictions metric") } + +// The entry counter is maintained by the insert path (+1) and freelru's +// OnEvict (-1) instead of by locking every shard, so it must still agree with +// the LRU's real length after concurrent inserts, updates, deletes and +// capacity evictions have raced across several grow steps. +func TestGenericCache_LenCounterUnderConcurrency(t *testing.T) { + c := closeOnCleanup(t, NewGenericCacheWithAvg[[]byte](8*datasize.MB, 256, func(v []byte) int { return len(v) }, ModeEvictLRU)) + + const workers = 8 + const perWorker = 4000 + var wg sync.WaitGroup + for w := range workers { + wg.Go(func() { + k := make([]byte, 8) + for i := range perWorker { + binary.BigEndian.PutUint64(k, uint64(w*perWorker+i)) + c.Put(k, []byte("v"), uint64(i)) + binary.BigEndian.PutUint64(k, uint64(w*perWorker+i/2)) + c.Put(k, []byte("updated"), uint64(i)) + if i%16 == 0 { + binary.BigEndian.PutUint64(k, uint64(w*perWorker+i/4)) + c.Delete(k) + } + } + }) + } + wg.Wait() + + require.Equal(t, c.data.Load().lru.Len(), c.Len(), "entry counter drifted from the LRU") +} + +// Parallel inserts into a cache that is still below its ceiling — the window +// where every insert runs the grow check. +func BenchmarkGenericCacheParallelPutGrow(b *testing.B) { + c := NewGenericCacheWithAvg[[]byte](256*datasize.MB, 96, func(v []byte) int { return len(v) }, ModeEvictLRU) + defer c.Close() + var seq atomic.Uint64 + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + k := make([]byte, 8) + for pb.Next() { + binary.BigEndian.PutUint64(k, seq.Add(1)) + c.Put(k, []byte("0123456789abcdef"), 1) + } + }) +} From 76c7384cdfd60e42d3effe848b174b8bb3a55a1e Mon Sep 17 00:00:00 2001 From: Alex Sharov Date: Mon, 24 Aug 2026 13:30:01 +0700 Subject: [PATCH 2/7] execution/cache: give growLRU the same atomic entry count (#23523) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #23522 — review that first; this PR's own diff is the second commit. `growLRU.Add` ran its grow check through `freelru.ShardedLRU.Len`, which RLocks every shard in turn, on **every add** while the LRU was below its ceiling; `maybeGrow` scanned again inside `resizeMu`. `CodeCache` drives three of these layers (`hashToCode`, `codeHashToCode`, `codeSizeByCodeHash`) straight off the EVM code path. Same fix as #23522: reuse `lruGen`, generalised over the value type, so the count comes from an atomic maintained by `add` and the `OnEvict` callback. `Len()` becomes O(1) too. `putContentLocked` already removes an existing entry before re-adding, so every `Add` reaching freelru is for an absent key — the invariant the counter needs. `BenchmarkGrowLRUParallelAddGrow` (added), M4 Max, `-cpu 10`: ``` │ growLRU_Len │ growLRU_counter │ │ sec/op │ sec/op vs base │ GrowLRUParallelAddGrow 756.90n ± 8% 96.58n ± 13% -87.24% (p=0.002 n=6) ``` `CodeCache.Put` end-to-end does **not** move (500.2n → 497.1n, p=0.394): `addrToHash` / `addrToCodeHash` are `hashicorp/golang-lru` caches behind a single global mutex, and that dominates the put path. This PR removes real CPU time from the content layers; unlocking it end-to-end needs that addr LRU sharded, which is separate work. Two tests pin counter == the LRU's real length: `growLRU` directly across add / remove / eviction / grow / `Purge`, and all three `CodeCache` layers under concurrent puts. Both verified to fail against a mutated counter. No TDD cycle: performance refactor with no intended behaviour change. https://claude.ai/code/session_01WFkAYPPhqPe1NXg41Nph78 --- .../cache/code_cache_concurrency_test.go | 81 +++++++++++++++++++ execution/cache/generic_cache.go | 18 ++--- execution/cache/grow_lru.go | 45 ++++++----- 3 files changed, 115 insertions(+), 29 deletions(-) diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index 88a9c3d3fa1..909c096834e 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -20,6 +20,7 @@ import ( "encoding/binary" "runtime" "sync" + "sync/atomic" "testing" "github.com/c2h5oh/datasize" @@ -193,3 +194,83 @@ func TestCodeCache_ClearFencesStartedPut(t *testing.T) { _, ok := cc.Get(addr) require.False(t, ok, "Clear must remove a write that started in the retiring generation") } + +// The O(1) entry counter that replaced freelru's all-shard Len on growLRU's add +// path must not drift from the LRU's real length on any mutation path. +func TestGrowLRU_LenTracksLRU(t *testing.T) { + var evictions int + g := newGrowLRU[uint64](8*datasize.MB, 16, func(uint64, uint64) { evictions++ }) + defer g.Close() + check := func(phase string) { + t.Helper() + require.Equal(t, g.cur.Load().lru.Len(), g.Len(), "entry counter drifted after %s", phase) + } + // growLRU hashes keys with the identity function, so spread the shard-selection + // bits the way its real maphash/keccak-derived keys do. + key := func(i uint64) uint64 { return i * 0x9E3779B97F4A7C15 } + + for i := range uint64(500) { + g.Add(key(i), i) + } + check("adds") + require.Equal(t, 500, g.Len(), "500 distinct keys must fit below the 1024-slot start capacity") + + for i := range uint64(100) { + g.Remove(key(i)) + } + check("removes") + require.Equal(t, 100, evictions, "the caller's OnEvict must still fire for each removal") + + before := g.cur.Load() + for i := uint64(500); i < 4000; i++ { + g.Add(key(i), i) + } + require.NotEqual(t, before, g.cur.Load(), "grow did not happen") + check("grow") + + g.Purge() + require.Equal(t, 0, g.Len()) + check("purge") +} + +// CodeCache drives three growLRU layers through putContentLocked, which removes +// a stale entry before re-adding it; concurrent puts of distinct code must still +// leave each layer's counter equal to its LRU's real length. +func TestCodeCache_GrowLRULenCounterUnderConcurrency(t *testing.T) { + cc := closeOnCleanup(t, NewCodeCache(8*datasize.MB, 8*datasize.MB)) + + const workers = 8 + const perWorker = 3000 + var wg sync.WaitGroup + for w := range workers { + wg.Go(func() { + addr := make([]byte, 20) + code := make([]byte, 40) + for i := range perWorker { + binary.BigEndian.PutUint64(addr[12:], uint64(w*perWorker+i)) + binary.BigEndian.PutUint64(code, uint64(w*perWorker+i)) + cc.Put(addr, code, uint64(i)) + } + }) + } + wg.Wait() + + require.Equal(t, cc.hashToCode.cur.Load().lru.Len(), cc.hashToCode.Len(), "hashToCode counter drifted") + require.Equal(t, cc.codeHashToCode.cur.Load().lru.Len(), cc.codeHashToCode.Len(), "codeHashToCode counter drifted") + require.Equal(t, cc.codeSizeByCodeHash.cur.Load().lru.Len(), cc.codeSizeByCodeHash.Len(), "codeSizeByCodeHash counter drifted") +} + +// Parallel adds while the LRU is still below its ceiling — the window where +// every add runs the grow check. +func BenchmarkGrowLRUParallelAddGrow(b *testing.B) { + g := newGrowLRU[uint64](256*datasize.MB, 48, nil) + defer g.Close() + var seq atomic.Uint64 + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + n := seq.Add(1) * 0x9E3779B97F4A7C15 + g.Add(n, n) + } + }) +} diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 25dc4397e41..23f64db2539 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -58,22 +58,22 @@ type entry[T any] struct { epoch uint32 // unwind generation the entry was written in } -// lruGen is one generation of the sharded LRU plus an O(1) live-entry count. +// lruGen is one generation of a sharded LRU plus an O(1) live-entry count. // freelru's own Len RLocks every shard, so the grow check — which every insert // runs while the cache is below its ceiling — cannot use it without serialising // writers that would otherwise touch disjoint shards. -type lruGen[T any] struct { - lru *freelru.ShardedLRU[uint64, entry[T]] +type lruGen[V any] struct { + lru *freelru.ShardedLRU[uint64, V] n atomic.Int64 } -func (g *lruGen[T]) len() int { return int(g.n.Load()) } +func (g *lruGen[V]) len() int { return int(g.n.Load()) } // add inserts a key the LRU does not already hold — every call site removes an // existing one first — so the count rises by one; a capacity eviction inside // freelru fires OnEvict, which takes it back down. -func (g *lruGen[T]) add(h uint64, e entry[T]) (evicted bool) { - evicted = g.lru.Add(h, e) +func (g *lruGen[V]) add(h uint64, v V) (evicted bool) { + evicted = g.lru.Add(h, v) g.n.Add(1) return evicted } @@ -85,7 +85,7 @@ type GenericCache[T any] struct { // held — on a jump-grow (fully copied generation) and on Clear (fresh // empty one) — so no write lands in a retired generation and no reader // sees a partial copy (see maybeGrow, Clear). - data atomic.Pointer[lruGen[T]] + data atomic.Pointer[lruGen[entry[T]]] capacityB datasize.ByteSize mode Mode @@ -218,12 +218,12 @@ func newGenericCacheEntries[T any](capacityBytes datasize.ByteSize, capacityEntr // entry. The callback must not feed the evictions metric — it also fires for // intentional Removes — so capacity evictions are counted from Add's evicted // return at the call sites. -func (c *GenericCache[T]) newShards(capacity, shards uint32) *lruGen[T] { +func (c *GenericCache[T]) newShards(capacity, shards uint32) *lruGen[entry[T]] { lru, err := freelru.NewShardedWithSize[uint64, entry[T]](shards, capacity, capacity+capacity/4, u64identity) if err != nil { panic(err) } - g := &lruGen[T]{lru: lru} + g := &lruGen[entry[T]]{lru: lru} lru.SetOnEvict(func(_ uint64, e entry[T]) { c.currentSize.Add(-int64(e.size)) g.n.Add(-1) diff --git a/execution/cache/grow_lru.go b/execution/cache/grow_lru.go index b0e3725028d..282f2728344 100644 --- a/execution/cache/grow_lru.go +++ b/execution/cache/grow_lru.go @@ -38,11 +38,12 @@ import ( // write lost in a retired generation is a benign miss, and an entry whose // removal a racing copy undid serves correct bytes until its stale stamp // drops it on the next read. Do not reuse for mutable-per-key values — those -// need GenericCache's fenced swap. The onEvict-maintained counters are -// approximate across grow windows (a lost write is counted but never -// evicted; a raced removal can subtract twice). +// need GenericCache's fenced swap. The onEvict-maintained counters — this +// type's per-generation entry count and the caller's byte counters — are +// approximate across grow windows (a lost write is counted but never evicted; +// a raced removal can subtract twice). type growLRU[V any] struct { - cur atomic.Pointer[freelru.ShardedLRU[uint64, V]] + cur atomic.Pointer[lruGen[V]] onEvict func(uint64, V) avgBytes int64 @@ -71,26 +72,30 @@ func newGrowLRU[V any](maxBytes datasize.ByteSize, avgBytes uint32, onEvict func return g } -func (g *growLRU[V]) newShards(capacity uint32) *freelru.ShardedLRU[uint64, V] { +func (g *growLRU[V]) newShards(capacity uint32) *lruGen[V] { lru, err := freelru.NewSharded[uint64, V](capacity, u64identity) if err != nil { panic(fmt.Sprintf("growLRU: NewSharded(%d): %s", capacity, err)) } - if g.onEvict != nil { - lru.SetOnEvict(g.onEvict) - } - return lru + gen := &lruGen[V]{lru: lru} + lru.SetOnEvict(func(k uint64, v V) { + gen.n.Add(-1) + if g.onEvict != nil { + g.onEvict(k, v) + } + }) + return gen } -func (g *growLRU[V]) Get(key uint64) (V, bool) { return g.cur.Load().Get(key) } +func (g *growLRU[V]) Get(key uint64) (V, bool) { return g.cur.Load().lru.Get(key) } func (g *growLRU[V]) Add(key uint64, value V) { - lru := g.cur.Load() - if curCap := g.curCap.Load(); curCap < g.maxCap && lru.Len() >= int(curCap) { + gen := g.cur.Load() + if curCap := g.curCap.Load(); curCap < g.maxCap && gen.len() >= int(curCap) { g.maybeGrow() - lru = g.cur.Load() + gen = g.cur.Load() } - lru.Add(key, value) + gen.add(key, value) } func (g *growLRU[V]) maybeGrow() { @@ -98,7 +103,7 @@ func (g *growLRU[V]) maybeGrow() { defer g.resizeMu.Unlock() old := g.cur.Load() curCap := g.curCap.Load() - if curCap >= g.maxCap || old.Len() < int(curCap) { + if curCap >= g.maxCap || old.len() < int(curCap) { return } newCap := min(curCap*genericCacheGrowFactor, g.maxCap) @@ -107,9 +112,9 @@ func (g *growLRU[V]) maybeGrow() { return } next := g.newShards(newCap) - for _, k := range old.Keys() { - if v, ok := old.Get(k); ok { - next.Add(k, v) + for _, k := range old.lru.Keys() { + if v, ok := old.lru.Get(k); ok { + next.add(k, v) } } g.cur.Store(next) @@ -117,8 +122,8 @@ func (g *growLRU[V]) maybeGrow() { g.reserved += delta } -func (g *growLRU[V]) Remove(key uint64) { g.cur.Load().Remove(key) } -func (g *growLRU[V]) Len() int { return g.cur.Load().Len() } +func (g *growLRU[V]) Remove(key uint64) { g.cur.Load().lru.Remove(key) } +func (g *growLRU[V]) Len() int { return g.cur.Load().len() } // Purge empties the LRU and shrinks it back to the start size, returning the // grown budget to the envelope (it regrows on demand). From 16c0c3d3faefd8664e997951575fe61f5f6be97f Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Mon, 24 Aug 2026 18:07:50 +0700 Subject: [PATCH 3/7] execution/cache: fix growLRU count drift on a grow/replace race growLRU's grow is not fenced against writers, so a same-key Remove then Add (putContentLocked's stale-entry refresh) can straddle a generation swap: the Remove lands on the retiring generation while the Add lands on the new one, which a concurrent grow already copied the key into. freelru.Add replaces an existing key in place without firing OnEvict, so the unconditional increment in growLRU's add path double-counted it. Add addIfAbsent, which checks membership before counting, and use it on growLRU's hot path. --- .../cache/code_cache_concurrency_test.go | 36 +++++++++++++++++++ execution/cache/generic_cache.go | 15 ++++++++ execution/cache/grow_lru.go | 2 +- 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index 909c096834e..9b7dc52c847 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -233,6 +233,42 @@ func TestGrowLRU_LenTracksLRU(t *testing.T) { check("purge") } +// growLRU's generation swap is not fenced against writers (see its doc +// comment): a grow can copy a key into the new generation before a same-key +// Remove/Add pair (putContentLocked's stale-entry refresh) observes the swap. +// The Remove then lands on the retired generation while the Add lands on the +// new one, which already holds the copy — freelru.Add replaces it in place +// without firing OnEvict, so the counter must not increment again. +func TestGrowLRU_GrowRaceReplaceDoesNotDoubleCount(t *testing.T) { + g := newGrowLRU[uint64](8*datasize.MB, 16, nil) + defer g.Close() + + h := uint64(7) + g.Add(h, 1) + gen1 := g.cur.Load() + + // Build the next generation exactly like maybeGrow's copy loop, and + // publish it while h is still present in gen1. + newCap := g.curCap.Load() * genericCacheGrowFactor + gen2 := g.newShards(newCap) + for _, k := range gen1.lru.Keys() { + if v, ok := gen1.lru.Get(k); ok { + gen2.add(k, v) + } + } + g.cur.Store(gen2) + g.curCap.Store(newCap) + + // The writer's Remove call raced ahead of the swap and landed on the + // retired generation. + gen1.lru.Remove(h) + // Its Add call now targets gen2, which already holds h from the copy. + g.Add(h, 2) + + require.Equal(t, gen2.lru.Len(), g.Len(), + "counter must not double-count a grow-copied key replaced in place") +} + // CodeCache drives three growLRU layers through putContentLocked, which removes // a stale entry before re-adding it; concurrent puts of distinct code must still // leave each layer's counter equal to its LRU's real length. diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 23f64db2539..c27c581c4ff 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -78,6 +78,21 @@ func (g *lruGen[V]) add(h uint64, v V) (evicted bool) { return evicted } +// addIfAbsent is add's counterpart for a caller that cannot guarantee this +// generation lacks h — growLRU's grow is not fenced against writers (see its +// doc comment), so a same-key Remove/Add pair can land on a generation a +// concurrent grow already copied the key into. freelru.Add replaces an +// existing key in place without firing OnEvict, so the count must rise only +// when the key was actually absent. +func (g *lruGen[V]) addIfAbsent(h uint64, v V) (evicted bool) { + existed := g.lru.Contains(h) + evicted = g.lru.Add(h, v) + if !existed { + g.n.Add(1) + } + return evicted +} + // GenericCache is a sharded, LRU-evicting bounded cache for key-value // data. Eviction mode is fixed at construction (see policy.go). type GenericCache[T any] struct { diff --git a/execution/cache/grow_lru.go b/execution/cache/grow_lru.go index 282f2728344..4c310b93722 100644 --- a/execution/cache/grow_lru.go +++ b/execution/cache/grow_lru.go @@ -95,7 +95,7 @@ func (g *growLRU[V]) Add(key uint64, value V) { g.maybeGrow() gen = g.cur.Load() } - gen.add(key, value) + gen.addIfAbsent(key, value) } func (g *growLRU[V]) maybeGrow() { From 2df32ebf5a632ef0b6e043c9cb7d5cf456500993 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Tue, 25 Aug 2026 08:28:18 +0700 Subject: [PATCH 4/7] execution/cache: refresh a stale entry through one generation, drop the membership probe --- execution/cache/code_cache.go | 7 +- .../cache/code_cache_concurrency_test.go | 67 ++++++++++++++++--- execution/cache/generic_cache.go | 21 +----- execution/cache/grow_lru.go | 28 ++++++-- 4 files changed, 89 insertions(+), 34 deletions(-) diff --git a/execution/cache/code_cache.go b/execution/cache/code_cache.go index 881eaceeaac..dda4869ce94 100644 --- a/execution/cache/code_cache.go +++ b/execution/cache/code_cache.go @@ -208,13 +208,18 @@ func putContentLocked[T any]( counter *atomic.Int64, keyCost int64, ) { + stale := false if existing, ok := lru.Get(h); ok { if txNum, epoch := stamp(existing); !coh.IsStale(txNum, epoch) { return } - lru.Remove(h) // stale — OnEvict decrements counter for the removed entry + stale = true } counter.Add(keyCost + valCost(newEntry)) + if stale { + lru.Replace(h, newEntry) // OnEvict decrements counter for the replaced entry + return + } lru.Add(h, newEntry) // evicts the coldest entry when full; its OnEvict decrements counter } diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index 9b7dc52c847..1f6ac5c92c0 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -234,11 +234,11 @@ func TestGrowLRU_LenTracksLRU(t *testing.T) { } // growLRU's generation swap is not fenced against writers (see its doc -// comment): a grow can copy a key into the new generation before a same-key -// Remove/Add pair (putContentLocked's stale-entry refresh) observes the swap. -// The Remove then lands on the retired generation while the Add lands on the -// new one, which already holds the copy — freelru.Add replaces it in place -// without firing OnEvict, so the counter must not increment again. +// comment): a grow can copy a key into the new generation while a same-key +// refresh (putContentLocked's stale-entry path) is in flight. Replace resolves +// g.cur once so both halves land on the same generation; split across the swap, +// the removal would hit the retired generation and the insert would replace the +// copy in place without firing OnEvict, double-counting the key. func TestGrowLRU_GrowRaceReplaceDoesNotDoubleCount(t *testing.T) { g := newGrowLRU[uint64](8*datasize.MB, 16, nil) defer g.Close() @@ -259,14 +259,20 @@ func TestGrowLRU_GrowRaceReplaceDoesNotDoubleCount(t *testing.T) { g.cur.Store(gen2) g.curCap.Store(newCap) - // The writer's Remove call raced ahead of the swap and landed on the - // retired generation. - gen1.lru.Remove(h) - // Its Add call now targets gen2, which already holds h from the copy. - g.Add(h, 2) + g.Replace(h, 2) require.Equal(t, gen2.lru.Len(), g.Len(), "counter must not double-count a grow-copied key replaced in place") + got, ok := g.Get(h) + require.True(t, ok) + require.Equal(t, uint64(2), got, "the refreshed value must be the one served") + + // A refresh that resolved the retired generation must leave the live count alone. + live := g.Len() + gen1.lru.Remove(h) + gen1.add(h, 3) + require.Equal(t, live, g.Len(), "a write lost in the retired generation must not move the live counter") + require.Equal(t, gen1.lru.Len(), gen1.len(), "the retired generation's own counter must stay exact") } // CodeCache drives three growLRU layers through putContentLocked, which removes @@ -310,3 +316,44 @@ func BenchmarkGrowLRUParallelAddGrow(b *testing.B) { } }) } + +// The three stale drops on CodeCache's read path hold no put stripe, so they +// interleave freely with a striped writer's stale-entry refresh while the LRU +// grows. Whatever the interleaving, the live generation's counter must equal +// its real length: an under-count wedges growth for good, because both grow +// gates read the counter. +func TestGrowLRU_CountExactUnderStripedRefreshAndUnstripedRemove(t *testing.T) { + g := newGrowLRU[uint64](8*datasize.MB, 16, nil) + defer g.Close() + + const keySpace = 4096 + key := func(i uint64) uint64 { return (i % keySpace) * 0x9E3779B97F4A7C15 } + + var stripes [256]sync.Mutex + var wg sync.WaitGroup + for w := range 8 { + wg.Go(func() { + for i := range uint64(20000) { + h := key(uint64(w)*20000 + i) + stripe := &stripes[uint8(h)] + stripe.Lock() + if _, ok := g.Get(h); ok { + g.Replace(h, i) + } else { + g.Add(h, i) + } + stripe.Unlock() + } + }) + } + wg.Go(func() { + for i := range uint64(60000) { + g.Remove(key(i)) + } + }) + wg.Wait() + + gen := g.cur.Load() + require.Equal(t, gen.lru.Len(), gen.len(), "live generation's counter drifted from its real length") + require.NotEqual(t, g.startCap, g.curCap.Load(), "the LRU never grew, so the growth gates were not exercised") +} diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index c27c581c4ff..da244a008fb 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -69,30 +69,15 @@ type lruGen[V any] struct { func (g *lruGen[V]) len() int { return int(g.n.Load()) } -// add inserts a key the LRU does not already hold — every call site removes an -// existing one first — so the count rises by one; a capacity eviction inside -// freelru fires OnEvict, which takes it back down. +// add inserts a key the LRU does not already hold — every call site proves that +// first — so the count rises by one; a capacity eviction inside freelru fires +// OnEvict, which takes it back down. func (g *lruGen[V]) add(h uint64, v V) (evicted bool) { evicted = g.lru.Add(h, v) g.n.Add(1) return evicted } -// addIfAbsent is add's counterpart for a caller that cannot guarantee this -// generation lacks h — growLRU's grow is not fenced against writers (see its -// doc comment), so a same-key Remove/Add pair can land on a generation a -// concurrent grow already copied the key into. freelru.Add replaces an -// existing key in place without firing OnEvict, so the count must rise only -// when the key was actually absent. -func (g *lruGen[V]) addIfAbsent(h uint64, v V) (evicted bool) { - existed := g.lru.Contains(h) - evicted = g.lru.Add(h, v) - if !existed { - g.n.Add(1) - } - return evicted -} - // GenericCache is a sharded, LRU-evicting bounded cache for key-value // data. Eviction mode is fixed at construction (see policy.go). type GenericCache[T any] struct { diff --git a/execution/cache/grow_lru.go b/execution/cache/grow_lru.go index 4c310b93722..3011de526b9 100644 --- a/execution/cache/grow_lru.go +++ b/execution/cache/grow_lru.go @@ -38,10 +38,12 @@ import ( // write lost in a retired generation is a benign miss, and an entry whose // removal a racing copy undid serves correct bytes until its stale stamp // drops it on the next read. Do not reuse for mutable-per-key values — those -// need GenericCache's fenced swap. The onEvict-maintained counters — this -// type's per-generation entry count and the caller's byte counters — are -// approximate across grow windows (a lost write is counted but never evicted; -// a raced removal can subtract twice). +// need GenericCache's fenced swap. Each generation's entry count stays exact +// for that generation — every mutation resolves one generation and decrements +// the one it acted on — which is what the grow gates rely on. The caller's +// onEvict byte counters remain approximate across grow windows: a write lost in +// a retired generation is counted but never evicted, and a removal a racing +// copy undid subtracts twice. type growLRU[V any] struct { cur atomic.Pointer[lruGen[V]] onEvict func(uint64, V) @@ -89,13 +91,29 @@ func (g *growLRU[V]) newShards(capacity uint32) *lruGen[V] { func (g *growLRU[V]) Get(key uint64) (V, bool) { return g.cur.Load().lru.Get(key) } +// Add inserts a key this LRU does not hold. Callers reach it through a put +// stripe they have held since their own miss, and the read paths only ever +// remove, so nothing can have inserted key in between and no grow copy can +// have carried it into the generation this resolves. func (g *growLRU[V]) Add(key uint64, value V) { gen := g.cur.Load() if curCap := g.curCap.Load(); curCap < g.maxCap && gen.len() >= int(curCap) { g.maybeGrow() gen = g.cur.Load() } - gen.addIfAbsent(key, value) + gen.add(key, value) +} + +// Replace refreshes a key this LRU already holds. Resolving the generation once +// is what keeps the count exact: a Remove and an Add that resolve g.cur +// separately can straddle an unfenced grow, landing the removal on the retired +// generation and the insert on a new one that already holds the copied key -- +// which freelru replaces in place without firing OnEvict. The entry count does +// not change, so this never needs to grow. +func (g *growLRU[V]) Replace(key uint64, value V) { + gen := g.cur.Load() + gen.lru.Remove(key) + gen.add(key, value) } func (g *growLRU[V]) maybeGrow() { From 489fb3163ab524f9931fec541e31bc2ae1b42ef2 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Wed, 26 Aug 2026 12:25:23 +0700 Subject: [PATCH 5/7] execution/cache: fold growLRU Add and Replace into one Put that displaces Add assumed its caller had proven the key absent. It had not: an unfenced grow copies a key into the next generation, a read-path Remove drops it from the still-current old one, and the publish can land between the caller's miss and Add's own load -- so freelru replaced the copy in place, firing no OnEvict, while the count still rose. The generation then carried an entry that was not there, and the grow gate tripped early. Add and Replace only ever differed in the grow check, so they are one Put that removes before it stores. The removal costs a lookup and buys an exact count and a byte counter that no longer keeps a displaced copy. --- execution/cache/cache_test.go | 43 +++++++++++++++++++ execution/cache/code_cache.go | 18 +++----- .../cache/code_cache_concurrency_test.go | 18 ++++---- execution/cache/grow_lru.go | 24 ++++------- 4 files changed, 67 insertions(+), 36 deletions(-) diff --git a/execution/cache/cache_test.go b/execution/cache/cache_test.go index 580ad509e09..4299d1699a5 100644 --- a/execution/cache/cache_test.go +++ b/execution/cache/cache_test.go @@ -1820,3 +1820,46 @@ func TestGenericCache_LenTracksLRU(t *testing.T) { require.Equal(t, 0, c.Len()) check("clear") } + +// TestGrowLRU_PutOfPresentKeyKeepsCount pins that storing a key the generation +// already holds does not raise the entry count. freelru replaces such a key in +// place and fires no OnEvict, so a count raised on the way in never comes back +// down and the grow gate trips on entries that are not there. +func TestGrowLRU_PutOfPresentKeyKeepsCount(t *testing.T) { + t.Run("same key twice", func(t *testing.T) { + g := newGrowLRU[int](1*datasize.MB, 8, nil) + defer g.Close() + + g.Put(1, 10) + g.Put(1, 20) + + require.Equal(t, 1, g.Len()) + v, ok := g.Get(1) + require.True(t, ok) + require.Equal(t, 20, v) + }) + + // A grow copies a key into the next generation, a read-path Remove then + // drops it from the still-current old one, and a writer that missed on old + // stores it after the publish -- into a generation that already holds it. + t.Run("key carried in by a racing grow copy", func(t *testing.T) { + var evicted int + g := newGrowLRU[int](1*datasize.MB, 8, func(uint64, int) { evicted++ }) + defer g.Close() + + g.Put(1, 10) + old := g.cur.Load() + + next := g.newShards(g.curCap.Load()) + next.add(1, 10) // the grow's copy + old.lru.Remove(1) + g.cur.Store(next) // the publish the writer's miss straddles + + evictedBefore := evicted + g.Put(1, 20) + + require.Equal(t, 1, g.Len()) + require.Equal(t, 1, evicted-evictedBefore, + "the copy the store displaced must be evicted, not dropped silently") + }) +} diff --git a/execution/cache/code_cache.go b/execution/cache/code_cache.go index dda4869ce94..26be20bcb2e 100644 --- a/execution/cache/code_cache.go +++ b/execution/cache/code_cache.go @@ -191,10 +191,10 @@ type CodeCache struct { // putContentLocked is the shared insert path for the content-addressed code layers // (hashToCode, codeHashToCode, codeSizeByCodeHash). Each is a freelru.ShardedLRU // of per-key-immutable entries carrying a (txNum, epoch) stamp: a live entry is -// kept (its bytes/size are invariant for a given key), a stale one is removed -// (its OnEvict decrements counter) so the fresh entry can replace it, and once -// the entry-count cap is reached freelru.Add evicts the coldest entry (whose -// OnEvict decrements counter) rather than freezing. counter tracks resident +// kept (its bytes/size are invariant for a given key), a stale one is displaced +// by Put (its OnEvict decrements counter), and once the entry-count cap is +// reached freelru evicts the coldest entry (whose OnEvict decrements counter) +// rather than freezing. counter tracks resident // bytes as a stat; the hard bound is the LRU's entry cap. stamp/valCost are // non-capturing so passing them allocates nothing on the put path. The caller // holds the key's put stripe. @@ -208,19 +208,15 @@ func putContentLocked[T any]( counter *atomic.Int64, keyCost int64, ) { - stale := false if existing, ok := lru.Get(h); ok { if txNum, epoch := stamp(existing); !coh.IsStale(txNum, epoch) { return } - stale = true } counter.Add(keyCost + valCost(newEntry)) - if stale { - lru.Replace(h, newEntry) // OnEvict decrements counter for the replaced entry - return - } - lru.Add(h, newEntry) // evicts the coldest entry when full; its OnEvict decrements counter + // Put covers both a fresh key and a stale one: whatever it displaces is + // evicted, and its OnEvict takes counter back down. + lru.Put(h, newEntry) } func codeEntryStamp(e codeEntry) (uint64, uint32) { return e.txNum, e.epoch } diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index 1f6ac5c92c0..a1f784d0d1e 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -81,7 +81,7 @@ func TestCodeCache_ByteCheckRejectsForeignKeyHash(t *testing.T) { foreign := make([]byte, 32) copy(foreign, realHash) foreign[0] ^= 0xff // different 32-byte key - cc.codeHashToCode.Add(maphash.Hash(foreign), codeEntry{code: code, keyHash: hash32(realHash), txNum: 1, epoch: cc.coh.Epoch()}) + cc.codeHashToCode.Put(maphash.Hash(foreign), codeEntry{code: code, keyHash: hash32(realHash), txNum: 1, epoch: cc.coh.Epoch()}) // The stored entry's keyHash is realHash, not foreign — Get must reject it. _, ok = cc.GetByCodeHash(foreign) @@ -151,7 +151,7 @@ func TestCodeCache_ClearRacingPut_EpochAlias(t *testing.T) { // Model a writer that sampled the epoch before Clear and published after // the relevant layers were purged. cc.addrToHash.Add(common.BytesToAddress(addr), versionedAddressID{addrID: codeID, txNum: 200, epoch: preClearEpoch}) - cc.hashToCode.Add(codeID, codeEntry{code: code, txNum: 200, epoch: preClearEpoch}) + cc.hashToCode.Put(codeID, codeEntry{code: code, txNum: 200, epoch: preClearEpoch}) cc.Unwind(150) _, ok := cc.Get(addr) @@ -210,7 +210,7 @@ func TestGrowLRU_LenTracksLRU(t *testing.T) { key := func(i uint64) uint64 { return i * 0x9E3779B97F4A7C15 } for i := range uint64(500) { - g.Add(key(i), i) + g.Put(key(i), i) } check("adds") require.Equal(t, 500, g.Len(), "500 distinct keys must fit below the 1024-slot start capacity") @@ -223,7 +223,7 @@ func TestGrowLRU_LenTracksLRU(t *testing.T) { before := g.cur.Load() for i := uint64(500); i < 4000; i++ { - g.Add(key(i), i) + g.Put(key(i), i) } require.NotEqual(t, before, g.cur.Load(), "grow did not happen") check("grow") @@ -244,7 +244,7 @@ func TestGrowLRU_GrowRaceReplaceDoesNotDoubleCount(t *testing.T) { defer g.Close() h := uint64(7) - g.Add(h, 1) + g.Put(h, 1) gen1 := g.cur.Load() // Build the next generation exactly like maybeGrow's copy loop, and @@ -259,7 +259,7 @@ func TestGrowLRU_GrowRaceReplaceDoesNotDoubleCount(t *testing.T) { g.cur.Store(gen2) g.curCap.Store(newCap) - g.Replace(h, 2) + g.Put(h, 2) require.Equal(t, gen2.lru.Len(), g.Len(), "counter must not double-count a grow-copied key replaced in place") @@ -312,7 +312,7 @@ func BenchmarkGrowLRUParallelAddGrow(b *testing.B) { b.RunParallel(func(pb *testing.PB) { for pb.Next() { n := seq.Add(1) * 0x9E3779B97F4A7C15 - g.Add(n, n) + g.Put(n, n) } }) } @@ -338,9 +338,9 @@ func TestGrowLRU_CountExactUnderStripedRefreshAndUnstripedRemove(t *testing.T) { stripe := &stripes[uint8(h)] stripe.Lock() if _, ok := g.Get(h); ok { - g.Replace(h, i) + g.Put(h, i) } else { - g.Add(h, i) + g.Put(h, i) } stripe.Unlock() } diff --git a/execution/cache/grow_lru.go b/execution/cache/grow_lru.go index 3011de526b9..16f760a641b 100644 --- a/execution/cache/grow_lru.go +++ b/execution/cache/grow_lru.go @@ -91,27 +91,19 @@ func (g *growLRU[V]) newShards(capacity uint32) *lruGen[V] { func (g *growLRU[V]) Get(key uint64) (V, bool) { return g.cur.Load().lru.Get(key) } -// Add inserts a key this LRU does not hold. Callers reach it through a put -// stripe they have held since their own miss, and the read paths only ever -// remove, so nothing can have inserted key in between and no grow copy can -// have carried it into the generation this resolves. -func (g *growLRU[V]) Add(key uint64, value V) { +// Put stores a key, growing first when the generation is full. Both the removal +// and the store resolve one generation, and the removal is not redundant on a +// caller that just missed: an unfenced grow can copy the key into the next +// generation, a read-path Remove can drop it from the still-current old one, and +// the publish can land between that miss and this load. freelru would then +// replace the copy in place without firing OnEvict, leaving the count and the +// caller's byte counter carrying an entry that is no longer there. +func (g *growLRU[V]) Put(key uint64, value V) { gen := g.cur.Load() if curCap := g.curCap.Load(); curCap < g.maxCap && gen.len() >= int(curCap) { g.maybeGrow() gen = g.cur.Load() } - gen.add(key, value) -} - -// Replace refreshes a key this LRU already holds. Resolving the generation once -// is what keeps the count exact: a Remove and an Add that resolve g.cur -// separately can straddle an unfenced grow, landing the removal on the retired -// generation and the insert on a new one that already holds the copied key -- -// which freelru replaces in place without firing OnEvict. The entry count does -// not change, so this never needs to grow. -func (g *growLRU[V]) Replace(key uint64, value V) { - gen := g.cur.Load() gen.lru.Remove(key) gen.add(key, value) } From 9424fba9841e0401632abf0b1c7555fa1695d5e7 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Wed, 26 Aug 2026 21:51:59 +0700 Subject: [PATCH 6/7] execution/cache: make the CodeCache counter test exercise what it claims It wrote only hashToCode, at a budget where the ceiling equals the start capacity so no grow ever ran, with distinct keys so no stale refresh ran. The other two layers' assertions compared 0 to 0. Now all three layers are written through PutWithCodeHash over a repeating key space, an unwinder makes the repeats stale, and the code budget leaves room to grow. Also drop a dead if/else whose arms were identical, and retire the Replace naming folded away in the previous commit. --- .../cache/code_cache_concurrency_test.go | 79 +++++++++++++------ 1 file changed, 57 insertions(+), 22 deletions(-) diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index a1f784d0d1e..9bf1af23e9d 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -235,11 +235,11 @@ func TestGrowLRU_LenTracksLRU(t *testing.T) { // growLRU's generation swap is not fenced against writers (see its doc // comment): a grow can copy a key into the new generation while a same-key -// refresh (putContentLocked's stale-entry path) is in flight. Replace resolves -// g.cur once so both halves land on the same generation; split across the swap, -// the removal would hit the retired generation and the insert would replace the -// copy in place without firing OnEvict, double-counting the key. -func TestGrowLRU_GrowRaceReplaceDoesNotDoubleCount(t *testing.T) { +// refresh (putContentLocked's stale-entry path) is in flight. Put resolves +// g.cur once so its removal and its store land on the same generation; split +// across the swap, the removal would hit the retired generation and the store +// would replace the copy in place without firing OnEvict, double-counting it. +func TestGrowLRU_GrowRacePutDoesNotDoubleCount(t *testing.T) { g := newGrowLRU[uint64](8*datasize.MB, 16, nil) defer g.Close() @@ -275,36 +275,75 @@ func TestGrowLRU_GrowRaceReplaceDoesNotDoubleCount(t *testing.T) { require.Equal(t, gen1.lru.Len(), gen1.len(), "the retired generation's own counter must stay exact") } -// CodeCache drives three growLRU layers through putContentLocked, which removes -// a stale entry before re-adding it; concurrent puts of distinct code must still -// leave each layer's counter equal to its LRU's real length. +// CodeCache drives all three growLRU layers through putContentLocked, whose +// stale path displaces the resident entry. The key space repeats so puts land on +// resident keys, an unwinder makes those repeats stale so the displacing path +// runs, and the code budget leaves room above the start capacity so the grow +// gates fire. Each layer's counter must still equal its LRU's real length. func TestCodeCache_GrowLRULenCounterUnderConcurrency(t *testing.T) { - cc := closeOnCleanup(t, NewCodeCache(8*datasize.MB, 8*datasize.MB)) + // 64MB over avgCodeEntryBytes puts the two code layers' ceiling well above + // genericCacheStartCapacity; at 8MB it lands below and they never grow. + cc := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 8*datasize.MB)) + // Floor 0, so every later Unwind turns the whole resident set stale on its + // epoch bump alone. + cc.Unwind(0) const workers = 8 const perWorker = 3000 + const keySpace = 4096 + + var unwinder sync.WaitGroup + done := make(chan struct{}) + unwinder.Go(func() { + for { + select { + case <-done: + return + default: + } + cc.Unwind(0) + runtime.Gosched() + } + }) + var wg sync.WaitGroup for w := range workers { wg.Go(func() { addr := make([]byte, 20) code := make([]byte, 40) + codeHash := make([]byte, 32) for i := range perWorker { - binary.BigEndian.PutUint64(addr[12:], uint64(w*perWorker+i)) - binary.BigEndian.PutUint64(code, uint64(w*perWorker+i)) - cc.Put(addr, code, uint64(i)) + k := uint64(w*perWorker+i) % keySpace + binary.BigEndian.PutUint64(addr[12:], k) + binary.BigEndian.PutUint64(code, k) + binary.BigEndian.PutUint64(codeHash, k) + cc.PutWithCodeHash(addr, code, codeHash, uint64(i)) } }) } wg.Wait() - - require.Equal(t, cc.hashToCode.cur.Load().lru.Len(), cc.hashToCode.Len(), "hashToCode counter drifted") - require.Equal(t, cc.codeHashToCode.cur.Load().lru.Len(), cc.codeHashToCode.Len(), "codeHashToCode counter drifted") - require.Equal(t, cc.codeSizeByCodeHash.cur.Load().lru.Len(), cc.codeSizeByCodeHash.Len(), "codeSizeByCodeHash counter drifted") + close(done) + unwinder.Wait() + + for _, layer := range []struct { + name string + lru *growLRU[codeEntry] + }{ + {"hashToCode", cc.hashToCode}, + {"codeHashToCode", cc.codeHashToCode}, + } { + require.Equal(t, layer.lru.cur.Load().lru.Len(), layer.lru.Len(), "%s counter drifted", layer.name) + require.Greater(t, layer.lru.curCap.Load(), layer.lru.startCap, "%s never grew", layer.name) + } + require.Equal(t, cc.codeSizeByCodeHash.cur.Load().lru.Len(), cc.codeSizeByCodeHash.Len(), + "codeSizeByCodeHash counter drifted") + require.Greater(t, cc.codeSizeByCodeHash.curCap.Load(), cc.codeSizeByCodeHash.startCap, + "codeSizeByCodeHash never grew") } // Parallel adds while the LRU is still below its ceiling — the window where // every add runs the grow check. -func BenchmarkGrowLRUParallelAddGrow(b *testing.B) { +func BenchmarkGrowLRUParallelPutGrow(b *testing.B) { g := newGrowLRU[uint64](256*datasize.MB, 48, nil) defer g.Close() var seq atomic.Uint64 @@ -337,11 +376,7 @@ func TestGrowLRU_CountExactUnderStripedRefreshAndUnstripedRemove(t *testing.T) { h := key(uint64(w)*20000 + i) stripe := &stripes[uint8(h)] stripe.Lock() - if _, ok := g.Get(h); ok { - g.Put(h, i) - } else { - g.Put(h, i) - } + g.Put(h, i) stripe.Unlock() } }) From a1a4637a6b330891542f88c2a3d087438543c366 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Thu, 27 Aug 2026 10:43:21 +0700 Subject: [PATCH 7/7] execution/cache: trim the comments, and put growLRU's doc back on growLRU --- execution/cache/cache_test.go | 7 ++- execution/cache/code_cache.go | 11 ++-- .../cache/code_cache_concurrency_test.go | 27 ++++------ execution/cache/grow_lru.go | 50 ++++++++----------- 4 files changed, 39 insertions(+), 56 deletions(-) diff --git a/execution/cache/cache_test.go b/execution/cache/cache_test.go index 77acd6475ba..0a5e94f3bdb 100644 --- a/execution/cache/cache_test.go +++ b/execution/cache/cache_test.go @@ -1830,10 +1830,9 @@ func TestGenericCache_LenTracksLRU(t *testing.T) { check("clear") } -// TestGrowLRU_PutOfPresentKeyKeepsCount pins that storing a key the generation -// already holds does not raise the entry count. freelru replaces such a key in -// place and fires no OnEvict, so a count raised on the way in never comes back -// down and the grow gate trips on entries that are not there. +// Storing a key the generation already holds must not raise the count: freelru +// replaces it in place and fires no OnEvict, so the rise never comes back down +// and the grow gate trips on entries that are not there. func TestGrowLRU_PutOfPresentKeyKeepsCount(t *testing.T) { t.Run("same key twice", func(t *testing.T) { g := newGrowLRU[int](1*datasize.MB, 8, nil) diff --git a/execution/cache/code_cache.go b/execution/cache/code_cache.go index 26be20bcb2e..8504a506f31 100644 --- a/execution/cache/code_cache.go +++ b/execution/cache/code_cache.go @@ -191,11 +191,10 @@ type CodeCache struct { // putContentLocked is the shared insert path for the content-addressed code layers // (hashToCode, codeHashToCode, codeSizeByCodeHash). Each is a freelru.ShardedLRU // of per-key-immutable entries carrying a (txNum, epoch) stamp: a live entry is -// kept (its bytes/size are invariant for a given key), a stale one is displaced -// by Put (its OnEvict decrements counter), and once the entry-count cap is -// reached freelru evicts the coldest entry (whose OnEvict decrements counter) -// rather than freezing. counter tracks resident -// bytes as a stat; the hard bound is the LRU's entry cap. stamp/valCost are +// kept (its bytes/size are invariant for a given key), and whatever Put displaces +// — a stale entry, or the coldest one once the cap is reached — decrements +// counter through OnEvict. counter tracks resident bytes as a stat; the hard +// bound is the LRU's entry cap. stamp/valCost are // non-capturing so passing them allocates nothing on the put path. The caller // holds the key's put stripe. func putContentLocked[T any]( @@ -214,8 +213,6 @@ func putContentLocked[T any]( } } counter.Add(keyCost + valCost(newEntry)) - // Put covers both a fresh key and a stale one: whatever it displaces is - // evicted, and its OnEvict takes counter back down. lru.Put(h, newEntry) } diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index 9bf1af23e9d..06b6d8920f4 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -233,12 +233,10 @@ func TestGrowLRU_LenTracksLRU(t *testing.T) { check("purge") } -// growLRU's generation swap is not fenced against writers (see its doc -// comment): a grow can copy a key into the new generation while a same-key -// refresh (putContentLocked's stale-entry path) is in flight. Put resolves -// g.cur once so its removal and its store land on the same generation; split -// across the swap, the removal would hit the retired generation and the store -// would replace the copy in place without firing OnEvict, double-counting it. +// A grow can copy a key into the new generation while a same-key refresh is in +// flight. Put resolves g.cur once so its removal and its store land on the same +// generation; split across the swap, the store would replace the copy in place +// without firing OnEvict and double-count it. func TestGrowLRU_GrowRacePutDoesNotDoubleCount(t *testing.T) { g := newGrowLRU[uint64](8*datasize.MB, 16, nil) defer g.Close() @@ -275,11 +273,9 @@ func TestGrowLRU_GrowRacePutDoesNotDoubleCount(t *testing.T) { require.Equal(t, gen1.lru.Len(), gen1.len(), "the retired generation's own counter must stay exact") } -// CodeCache drives all three growLRU layers through putContentLocked, whose -// stale path displaces the resident entry. The key space repeats so puts land on -// resident keys, an unwinder makes those repeats stale so the displacing path -// runs, and the code budget leaves room above the start capacity so the grow -// gates fire. Each layer's counter must still equal its LRU's real length. +// Each layer's counter must equal its LRU's real length while all three are +// driven through putContentLocked's displacing path. The fixture repeats keys +// and keeps unwinding them so that path runs, with budget left to grow into. func TestCodeCache_GrowLRULenCounterUnderConcurrency(t *testing.T) { // 64MB over avgCodeEntryBytes puts the two code layers' ceiling well above // genericCacheStartCapacity; at 8MB it lands below and they never grow. @@ -356,11 +352,10 @@ func BenchmarkGrowLRUParallelPutGrow(b *testing.B) { }) } -// The three stale drops on CodeCache's read path hold no put stripe, so they -// interleave freely with a striped writer's stale-entry refresh while the LRU -// grows. Whatever the interleaving, the live generation's counter must equal -// its real length: an under-count wedges growth for good, because both grow -// gates read the counter. +// CodeCache's read-path stale drops hold no put stripe, so they interleave +// freely with a striped refresh while the LRU grows. The counter must still +// equal the real length: both grow gates read it, so an under-count wedges +// growth for good. func TestGrowLRU_CountExactUnderStripedRefreshAndUnstripedRemove(t *testing.T) { g := newGrowLRU[uint64](8*datasize.MB, 16, nil) defer g.Close() diff --git a/execution/cache/grow_lru.go b/execution/cache/grow_lru.go index 8a6a87da967..3eca24fecf2 100644 --- a/execution/cache/grow_lru.go +++ b/execution/cache/grow_lru.go @@ -27,26 +27,9 @@ import ( "github.com/erigontech/erigon/common/cachebudget" ) -// growLRU is a uint64-keyed sharded LRU that starts small and jump-resizes ×4 -// toward a byte-budget ceiling as it fills, funding each step from the shared -// cachebudget envelope. It exists so a cache with a small working set never -// pre-commits its full configured capacity — the same demand-growth the state -// caches use — reused across the CodeCache's content and size layers. -// -// Generation swaps (maybeGrow, Purge) are not fenced against writers — safe -// only for content-addressed layers, where a key's payload never changes: a -// write lost in a retired generation is a benign miss, and an entry whose -// removal a racing copy undid serves correct bytes until its stale stamp -// drops it on the next read. Do not reuse for mutable-per-key values — those -// need GenericCache's fenced swap. Each generation's entry count stays exact -// for that generation — every mutation resolves one generation and decrements -// the one it acted on — which is what the grow gates rely on. The caller's -// onEvict byte counters remain approximate across grow windows: a write lost in -// a retired generation is counted but never evicted, and a removal a racing -// copy undid subtracts twice. // lruGen is one generation of a sharded LRU plus an O(1) live-entry count. -// freelru's own Len locks every shard, which serialises readers against each -// other on a path that only wanted a number. +// freelru's own Len locks every shard, which serialises readers on a path that +// only wanted a number. type lruGen[V any] struct { lru *freelru.ShardedLRU[uint64, V] n atomic.Int64 @@ -54,15 +37,27 @@ type lruGen[V any] struct { func (g *lruGen[V]) len() int { return int(g.n.Load()) } -// add inserts a key the LRU does not already hold -- Put removes first, so it -// always does -- and the count rises by one; a capacity eviction inside freelru -// fires OnEvict, which takes it back down. +// add requires a key the LRU does not hold — Put removes first — so the count +// can rise unconditionally; a capacity eviction fires OnEvict, which lowers it. func (g *lruGen[V]) add(h uint64, v V) (evicted bool) { evicted = g.lru.Add(h, v) g.n.Add(1) return evicted } +// growLRU is a uint64-keyed sharded LRU that starts small and jump-resizes ×4 +// toward a byte-budget ceiling as it fills, funding each step from the shared +// cachebudget envelope. It exists so a cache with a small working set never +// pre-commits its full configured capacity — the same demand-growth the state +// caches use — reused across the CodeCache's content and size layers. +// +// Generation swaps (maybeGrow, Purge) are not fenced against writers — safe +// only for content-addressed layers, where a key's payload never changes: a +// write lost in a retired generation is a benign miss, and an entry whose +// removal a racing copy undid serves correct bytes until its stale stamp +// drops it on the next read. Do not reuse for mutable-per-key values — those +// need GenericCache's fenced swap. Each generation's own entry count is exact; +// the caller's onEvict byte counters stay approximate across grow windows. type growLRU[V any] struct { cur atomic.Pointer[lruGen[V]] onEvict func(uint64, V) @@ -110,13 +105,10 @@ func (g *growLRU[V]) newShards(capacity uint32) *lruGen[V] { func (g *growLRU[V]) Get(key uint64) (V, bool) { return g.cur.Load().lru.Get(key) } -// Put stores a key, growing first when the generation is full. Both the removal -// and the store resolve one generation, and the removal is not redundant on a -// caller that just missed: an unfenced grow can copy the key into the next -// generation, a read-path Remove can drop it from the still-current old one, and -// the publish can land between that miss and this load. freelru would then -// replace the copy in place without firing OnEvict, leaving the count and the -// caller's byte counter carrying an entry that is no longer there. +// Put stores a key, growing first when the generation is full. The Remove is not +// redundant after a caller's miss: an unfenced grow can leave the key present in +// the generation this Put lands on, and freelru's Add would replace it in place +// without firing OnEvict, stranding the count and the caller's byte counter. func (g *growLRU[V]) Put(key uint64, value V) { gen := g.cur.Load() if curCap := g.curCap.Load(); curCap < g.maxCap && gen.len() >= int(curCap) {