diff --git a/execution/cache/cache_test.go b/execution/cache/cache_test.go index e70d80cf713..6e2009d0cba 100644 --- a/execution/cache/cache_test.go +++ b/execution/cache/cache_test.go @@ -18,6 +18,7 @@ package cache import ( "bytes" + "encoding/binary" "sync" "testing" @@ -31,6 +32,12 @@ import ( ) // Test helpers +func closeOnCleanup[T interface{ Close() }](tb testing.TB, c T) T { + tb.Helper() + tb.Cleanup(c.Close) + return c +} + func makeAddr(i int) []byte { addr := make([]byte, 20) addr[19] = byte(i) @@ -56,7 +63,7 @@ func makeValue(i int) []byte { // ============================================================================= func TestDomainCache_NewWithByteCapacity(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) // 1MB + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) // 1MB require.NotNil(t, c) assert.Equal(t, 0, c.Len()) assert.Equal(t, int64(0), c.SizeBytes()) @@ -64,7 +71,7 @@ func TestDomainCache_NewWithByteCapacity(t *testing.T) { } func TestDomainCache_GetPut(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) addr := makeAddr(1) value := makeValue(1) @@ -83,7 +90,7 @@ func TestDomainCache_GetPut(t *testing.T) { } func TestDomainCache_PutUpdateValue(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) addr := makeAddr(1) value1 := []byte{1, 2, 3, 4, 5, 6, 7, 8} // 8 bytes @@ -104,7 +111,7 @@ func TestDomainCache_PutCapacityLimit_NoOpMode(t *testing.T) { // full, new keys are silently dropped. Counted via the dropped metric. // Entry overhead is 20 (addr key) + 3 (value) + 24 = 47 bytes per entry. // Two entries take 94 bytes; cap at 100 leaves no room for a third. - c := NewDomainCacheMode(100, ModeNoOp) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeNoOp)) c.Put(makeAddr(1), makeValue(1), 0) c.Put(makeAddr(2), makeValue(2), 0) @@ -155,7 +162,7 @@ func TestDomainCache_PutEvictsWhenFull_EvictMode(t *testing.T) { } func TestDomainCache_Delete(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) addr := makeAddr(1) c.Put(addr, makeValue(1), 0) @@ -169,7 +176,7 @@ func TestDomainCache_Delete(t *testing.T) { } func TestDomainCache_Clear(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) c.Put(makeAddr(1), makeValue(1), 0) c.Put(makeAddr(2), makeValue(2), 0) @@ -180,7 +187,7 @@ func TestDomainCache_Clear(t *testing.T) { } func TestDomainCache_PrintStatsAndReset(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) // Generate some hits and misses c.Put(makeAddr(1), makeValue(1), 0) @@ -196,7 +203,7 @@ func TestDomainCache_PrintStatsAndReset(t *testing.T) { } func TestDomainCache_PrintStatsAndReset_NoOps(t *testing.T) { - c := NewDomainCacheMode(100, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(100, ModeEvictLRU)) // No operations - should handle zero total gracefully c.PrintStatsAndReset("test") } @@ -210,14 +217,14 @@ func TestDomainCache_ImplementsInterface(t *testing.T) { // ============================================================================= func TestCodeCache_NewDefaultCodeCache(t *testing.T) { - c := NewDefaultCodeCache() + c := closeOnCleanup(t, NewDefaultCodeCache()) require.NotNil(t, c) assert.Equal(t, 0, c.Len()) assert.Equal(t, 0, c.CodeLen()) } func TestCodeCache_GetPut(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) addr := makeAddr(1) code := makeCode(1) @@ -237,7 +244,7 @@ func TestCodeCache_GetPut(t *testing.T) { } func TestCodeCache_PutEmptyCode(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) addr := makeAddr(1) c.Put(addr, []byte{}, 0) @@ -248,7 +255,7 @@ func TestCodeCache_PutEmptyCode(t *testing.T) { } func TestCodeCache_CodeDeduplication(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) code := makeCode(1) addr1 := makeAddr(1) @@ -288,7 +295,7 @@ func TestCodeCache_AddrCapacityLimit(t *testing.T) { return []byte{0x60, byte(i >> 8), byte(i)} } - c := NewCodeCache(1024*1024, 1024*28) // 1MB code, ~1024 addr LRU entries + c := closeOnCleanup(t, NewCodeCache(1024*1024, 1024*28)) // 1MB code, ~1024 addr LRU entries for i := range 1100 { c.Put(wideAddr(i), wideCode(i), 0) } @@ -322,7 +329,7 @@ func TestCodeCache_AddrCapacityLimit(t *testing.T) { func TestCodeCache_CodeCapacityLimit(t *testing.T) { // Tiny byte budget → a 1-entry code layer cap. Successive distinct codes // LRU-evict the coldest rather than freezing the layer. - c := NewCodeCache(25, 1024*1024) // 25 bytes code, 1MB addr + c := closeOnCleanup(t, NewCodeCache(25, 1024*1024)) // 25 bytes code, 1MB addr c.Put(makeAddr(1), makeCode(1), 0) c.Put(makeAddr(2), makeCode(2), 0) @@ -342,7 +349,7 @@ func TestCodeCache_CodeCapacityLimit(t *testing.T) { } func TestCodeCache_Delete(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) addr := makeAddr(1) code := makeCode(1) @@ -358,7 +365,7 @@ func TestCodeCache_Delete(t *testing.T) { } func TestCodeCache_Clear(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) c.Put(makeAddr(1), makeCode(1), 0) c.Put(makeAddr(2), makeCode(2), 0) @@ -371,7 +378,7 @@ func TestCodeCache_Clear(t *testing.T) { } func TestCodeCache_PrintStatsAndReset(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) c.Put(makeAddr(1), makeCode(1), 0) c.Get(makeAddr(1)) // hit @@ -382,13 +389,13 @@ func TestCodeCache_PrintStatsAndReset(t *testing.T) { } func TestCodeCache_PrintStatsAndReset_NoOps(t *testing.T) { - c := NewCodeCache(100, 200) + c := closeOnCleanup(t, NewCodeCache(100, 200)) // No operations - should handle zero total gracefully c.PrintStatsAndReset() } func TestCodeCache_GetMissingCode(t *testing.T) { - c := NewCodeCache(1024*1024, 1024*1024) // 1MB each + c := closeOnCleanup(t, NewCodeCache(1024*1024, 1024*1024)) // 1MB each // Manually set addr mapping without code (simulates capacity limit scenario) addr := makeAddr(1) @@ -413,7 +420,7 @@ func TestCodeCache_ImplementsInterface(t *testing.T) { // ============================================================================= func TestStateCache_NewStateCache(t *testing.T) { - c := NewStateCache(10, 20, 30, 40) + c := closeOnCleanup(t, NewStateCache(10, 20, 30, 40)) require.NotNil(t, c) // Account, Storage, Code, Commitment should be initialized @@ -427,7 +434,7 @@ func TestStateCache_NewStateCache(t *testing.T) { } func TestStateCache_NewDefaultStateCache(t *testing.T) { - c := NewDefaultStateCache() + c := closeOnCleanup(t, NewDefaultStateCache()) require.NotNil(t, c) assert.NotNil(t, c.GetCache(kv.AccountsDomain)) @@ -436,7 +443,7 @@ func TestStateCache_NewDefaultStateCache(t *testing.T) { } func TestStateCache_GetPut_Account(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) addr := makeAddr(1) value := makeValue(1) @@ -454,7 +461,7 @@ func TestStateCache_GetPut_Account(t *testing.T) { } func TestStateCache_GetPut_Storage(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) key := make([]byte, 52) // addr(20) + slot(32) copy(key, makeAddr(1)) @@ -468,7 +475,7 @@ func TestStateCache_GetPut_Storage(t *testing.T) { } func TestStateCache_GetPut_Code(t *testing.T) { - c := NewStateCache(1*datasize.MB, 1*datasize.MB, 1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewStateCache(1*datasize.MB, 1*datasize.MB, 1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) code := makeCode(1) @@ -480,7 +487,7 @@ func TestStateCache_GetPut_Code(t *testing.T) { } func TestStateCache_GetPut_UnsupportedDomain(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) // ReceiptDomain is not supported c.Put(kv.ReceiptDomain, makeAddr(1), makeValue(1), 0) @@ -490,7 +497,7 @@ func TestStateCache_GetPut_UnsupportedDomain(t *testing.T) { } func TestStateCache_Delete(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) addr := makeAddr(1) c.Put(kv.AccountsDomain, addr, makeValue(1), 0) @@ -504,7 +511,7 @@ func TestStateCache_Delete(t *testing.T) { // caches deleted keys via Put(key, nil); if Get treats that as "not found", // the caller unnecessarily falls through to the DB on every read. func TestStateCache_PutEmpty_ThenGet_IsCacheHit(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) key := make([]byte, 52) // addr(20) + slot(32) key[0] = 0x1d @@ -519,7 +526,7 @@ func TestStateCache_PutEmpty_ThenGet_IsCacheHit(t *testing.T) { // Same test for []byte{} (zero-length but non-nil). func TestStateCache_PutEmptySlice_ThenGet_IsCacheHit(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) key := make([]byte, 52) key[0] = 0x1d @@ -533,14 +540,14 @@ func TestStateCache_PutEmptySlice_ThenGet_IsCacheHit(t *testing.T) { } func TestStateCache_Delete_UnsupportedDomain(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) // Should not panic c.Delete(kv.ReceiptDomain, makeAddr(1)) } func TestStateCache_Clear(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) c.Put(kv.AccountsDomain, makeAddr(1), makeValue(1), 0) c.Put(kv.StorageDomain, makeAddr(2), makeValue(2), 0) @@ -558,7 +565,7 @@ func TestStateCache_Clear(t *testing.T) { } func TestStateCache_GetCache_OutOfBounds(t *testing.T) { - c := NewStateCache(100, 100, 100, 100) + c := closeOnCleanup(t, NewStateCache(100, 100, 100, 100)) // Domain >= DomainLen should return nil cache := c.GetCache(kv.DomainLen) @@ -573,7 +580,7 @@ func TestStateCache_GetCache_OutOfBounds(t *testing.T) { // ============================================================================= func TestDomainCache_ConcurrentAccess(t *testing.T) { - c := NewDomainCacheMode(10000, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(10000, ModeEvictLRU)) done := make(chan bool) @@ -598,7 +605,7 @@ func TestDomainCache_ConcurrentAccess(t *testing.T) { } func TestCodeCache_ConcurrentAccess(t *testing.T) { - c := NewCodeCache(1000, 1000) + c := closeOnCleanup(t, NewCodeCache(1000, 1000)) done := make(chan bool) @@ -627,7 +634,7 @@ func TestCodeCache_ConcurrentAccess(t *testing.T) { // ============================================================================= func TestStateCache_DomainIsolation(t *testing.T) { - c := NewStateCache(1*datasize.MB, 1*datasize.MB, 1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewStateCache(1*datasize.MB, 1*datasize.MB, 1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) accountData := []byte("account") @@ -688,7 +695,7 @@ func makeDiffKey(baseKey []byte, step uint64) string { // Entries stamped at/below the unwind point survive (warm hot set kept); entries // above it from the now-dead epoch are dropped lazily on read. func TestUnwind_KeepsBelowFloor_EvictsAbove(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) below := makeAddr(1) above := makeAddr(2) c.Put(below, makeValue(1), 50) // predates the unwind @@ -709,7 +716,7 @@ func TestUnwind_KeepsBelowFloor_EvictsAbove(t *testing.T) { // so an entry stamped at exactly that txNum is dead-fork state and must be // evicted — the drop rule is txNum >= floor, not txNum > floor. func TestUnwind_EvictsEntryAtFloor(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) atFloor := makeAddr(1) belowFloor := makeAddr(2) c.Put(atFloor, makeValue(1), 100) // first txNum of the first unwound block @@ -729,7 +736,7 @@ func TestUnwind_EvictsEntryAtFloor(t *testing.T) { // SAME txNum as the dead fork's write. The epoch — not the txNum — distinguishes // them, so the dead entry reads stale and the re-written one reads valid. func TestUnwind_ReusedTxNumDisambiguatedByEpoch(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) k := makeAddr(1) c.Put(k, makeValue(1), 150) // dead fork, epoch 0 @@ -748,7 +755,7 @@ func TestUnwind_ReusedTxNumDisambiguatedByEpoch(t *testing.T) { // dead epoch above the floor and reads stale no matter how far execution // advances afterwards (there is no rising high-water mark to re-validate it). func TestUnwind_StragglerNeverResurrects(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) straggler := makeAddr(1) c.Put(straggler, makeValue(1), 150) // epoch 0 @@ -765,7 +772,7 @@ func TestUnwind_StragglerNeverResurrects(t *testing.T) { // A second, shallower unwind must not resurrect entries a deeper earlier unwind // invalidated (floor only moves down). func TestUnwind_FloorOnlyMovesDown(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) k := makeAddr(1) c.Put(k, makeValue(1), 70) // epoch 0 @@ -777,7 +784,7 @@ func TestUnwind_FloorOnlyMovesDown(t *testing.T) { } func TestDomainCache_PutIfAbsent(t *testing.T) { - c := NewDomainCacheMode(1*datasize.KB, ModeEvictLRU) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.KB, ModeEvictLRU)) addr := makeAddr(1) fresh := []byte("fresh") stale := []byte("stale") @@ -812,7 +819,7 @@ func TestDomainCache_PutIfAbsent(t *testing.T) { } func TestCodeCache_PutIfAbsentKeepsLiveAddrBinding(t *testing.T) { - cc := NewCodeCache(1*datasize.MB, 1*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) fresh := []byte{0xaa, 1, 2, 3} stale := []byte{0xbb, 4, 5, 6} @@ -837,7 +844,7 @@ func TestCodeCache_PutIfAbsentKeepsLiveAddrBinding(t *testing.T) { } func TestCodeCache_PutWithCodeHashIfAbsent(t *testing.T) { - cc := NewCodeCache(1*datasize.MB, 1*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) fresh := []byte{0xaa, 1, 2, 3} stale := []byte{0xbb, 4, 5, 6} @@ -865,12 +872,14 @@ func TestCodeCache_PutWithCodeHashIfAbsent(t *testing.T) { // check (absent), lose the CPU to the authoritative writer's insert, then // clobber it — the prefetch-vs-flush staleness this cache guards against. func TestDomainCache_PutIfAbsentAtomicWithPut(t *testing.T) { - c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) - addr := makeAddr(1) + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) fresh := []byte("fresh") stale := []byte("stale") + addr := make([]byte, 20) for round := range 20000 { - c.Delete(addr) + // Full-width round: the race only has teeth on a never-seen key, and + // makeAddr would truncate it to a byte. + binary.BigEndian.PutUint64(addr[1:], uint64(round)) var wg sync.WaitGroup wg.Add(2) go func() { defer wg.Done(); c.Put(addr, fresh, 20) }() @@ -881,3 +890,69 @@ func TestDomainCache_PutIfAbsentAtomicWithPut(t *testing.T) { require.Equal(t, fresh, v, "round %d: PutIfAbsent raced past a concurrent Put", round) } } + +// A Delete racing an update-in-place put must not double-subtract the +// displaced entry's size: freelru's OnEvict subtracts it for the Remove, and +// put's update delta subtracts it again unless the two writers share the +// key's stripe. +func TestDomainCache_DeleteAtomicWithPut_NoSizeDrift(t *testing.T) { + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) + addr := makeAddr(1) + v1 := []byte("value-one") + v2 := []byte("value-two") + for round := range 20000 { + c.Put(addr, v1, 10) + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(addr, v2, 20) }() + go func() { defer wg.Done(); c.Delete(addr) }() + wg.Wait() + c.Delete(addr) + require.Zero(t, c.SizeBytes(), "round %d: size accounting drifted", round) + } +} + +// The lazy stale-drop inside GetWithTxNum removes entries; an unstriped +// Remove racing put's read-modify-write double-subtracts the displaced +// entry's size. Exactly one live entry remains after every round, so drift +// shows as a size mismatch. +func TestDomainCache_StaleDropAtomicWithPut_NoSizeDrift(t *testing.T) { + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) + addr := makeAddr(1) + v1 := []byte("value-one") + v2 := []byte("value-two") + wantSize := int64(len(addr) + len(v1) + 24) + for round := range 20000 { + c.Put(addr, v1, 10) + c.Unwind(5) + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(addr, v2, 20) }() + go func() { defer wg.Done(); c.Get(addr) }() + wg.Wait() + require.Equal(t, wantSize, c.SizeBytes(), "round %d: size accounting drifted", round) + } +} + +// A Clear racing a put must not leave phantom bytes: unless Clear excludes +// writers via the put stripes, a put that loaded the retiring generation +// lands its entry where no reader sees it and adds the entry's size after +// Clear zeroed the counter — inflating SizeBytes for an invisible entry. +func TestDomainCache_ClearAtomicWithPut_NoSizeDrift(t *testing.T) { + c := closeOnCleanup(t, NewDomainCacheMode(1*datasize.MB, ModeEvictLRU)) + addr := makeAddr(1) + v1 := []byte("value-one") + entrySize := int64(len(addr) + len(v1) + 24) + for round := range 20000 { + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(addr, v1, 10) }() + go func() { defer wg.Done(); c.Clear() }() + wg.Wait() + wantSize := int64(0) + if _, ok := c.Get(addr); ok { + wantSize = entrySize + } + require.Equal(t, wantSize, c.SizeBytes(), "round %d: size accounting drifted", round) + } +} diff --git a/execution/cache/code_cache_codehash_test.go b/execution/cache/code_cache_codehash_test.go index dd7feee771a..a57664f1025 100644 --- a/execution/cache/code_cache_codehash_test.go +++ b/execution/cache/code_cache_codehash_test.go @@ -34,7 +34,7 @@ func makeCodeHash(i int) []byte { } func TestCodeCache_GetByCodeHash_HitAfterPut(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) code := []byte{0x60, 0x80, 0x60, 0x40, 0x52} // small contract preamble codeHash := makeCodeHash(0xab) @@ -61,7 +61,7 @@ func TestCodeCache_GetByCodeHash_HitAfterPut(t *testing.T) { func TestCodeCache_GetByCodeHash_DistinctAddrsSameCode(t *testing.T) { // The point of codeHashToCode: many addresses sharing one codeHash all hit a // single entry once any one of them has been populated. - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) code := []byte{0x60, 0x80, 0x60, 0x40, 0x52} codeHash := makeCodeHash(0xcd) @@ -82,7 +82,7 @@ func TestCodeCache_GetByCodeHash_DistinctAddrsSameCode(t *testing.T) { } func TestCodeCache_PutWithCodeHash_EmptyHashOrCodeIsNoOp(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) addr := makeAddr(1) code := []byte{0x60, 0x00} @@ -101,7 +101,7 @@ func TestCodeCache_PutWithCodeHash_EvictsColdestWhenFull(t *testing.T) { // Tiny byte budget → a 1-entry freelru cap. The second put must EVICT the // coldest entry (LRU), not freeze the layer: the newest code is retrievable // and the oldest is gone. - c := NewCodeCache(8, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(8, 1*datasize.MB)) c.PutWithCodeHash(makeAddr(1), []byte{1, 2, 3, 4}, makeCodeHash(1), 0) c.PutWithCodeHash(makeAddr(2), []byte{5, 6, 7, 8}, makeCodeHash(2), 0) @@ -112,7 +112,7 @@ func TestCodeCache_PutWithCodeHash_EvictsColdestWhenFull(t *testing.T) { } func TestCodeCache_CodeSize_PopulatedAlongsideBytes(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) code := []byte{0x60, 0x80, 0x60, 0x40, 0x52, 0x60, 0x10} codeHash := makeCodeHash(0xee) @@ -129,7 +129,7 @@ func TestCodeCache_CodeSize_PopulatedAlongsideBytes(t *testing.T) { } func TestCodeCache_CodeSize_DirectPutAndGet(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) codeHash := makeCodeHash(0xff) // Direct Put without going through the bytes layer. @@ -141,7 +141,7 @@ func TestCodeCache_CodeSize_DirectPutAndGet(t *testing.T) { } func TestCodeCache_CodeSize_EmptyHashOrNegativeIsNoOp(t *testing.T) { - c := NewCodeCache(1*datasize.MB, 1*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(1*datasize.MB, 1*datasize.MB)) c.PutCodeSizeByCodeHash(nil, 100, 0) c.PutCodeSizeByCodeHash(makeCodeHash(1), -1, 0) _, ok := c.GetCodeSizeByCodeHash(makeCodeHash(1)) @@ -153,7 +153,7 @@ func TestCodeCache_CodeSize_EmptyHashOrNegativeIsNoOp(t *testing.T) { // ============================================================================= func BenchmarkCodeCache_GetByCodeHash_Hit(b *testing.B) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(b, NewCodeCache(64*datasize.MB, 16*datasize.MB)) code := bytes.Repeat([]byte{0x5b}, 2048) // 2 KiB typical contract size codeHash := makeCodeHash(0x11) c.PutWithCodeHash(makeAddr(1), code, codeHash, 0) @@ -168,7 +168,7 @@ func BenchmarkCodeCache_GetByCodeHash_Hit(b *testing.B) { } func BenchmarkCodeCache_GetByCodeHash_Miss(b *testing.B) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(b, NewCodeCache(64*datasize.MB, 16*datasize.MB)) missHash := makeCodeHash(0x22) b.ResetTimer() @@ -181,7 +181,7 @@ func BenchmarkCodeCache_GetByCodeHash_Miss(b *testing.B) { // path. Compare against GetByCodeHash to verify the codeHashToCode lookup is at least // as fast (one map probe vs two: addr→hash then hash→code). func BenchmarkCodeCache_Get_AddrLevel_Hit(b *testing.B) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(b, NewCodeCache(64*datasize.MB, 16*datasize.MB)) code := bytes.Repeat([]byte{0x5b}, 2048) addr := makeAddr(1) c.PutWithCodeHash(addr, code, makeCodeHash(0x33), 0) @@ -200,7 +200,7 @@ func BenchmarkCodeCache_Get_AddrLevel_Hit(b *testing.B) { // Without codeHashToCode every fresh addr would pay a file read. With codeHashToCode every // caller that already knows the hash hits one shared entry. func BenchmarkCodeCache_GetByCodeHash_ManyAddrs_OneCode(b *testing.B) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(b, NewCodeCache(64*datasize.MB, 16*datasize.MB)) code := bytes.Repeat([]byte{0x5b}, 2048) codeHash := makeCodeHash(0x44) c.PutWithCodeHash(makeAddr(1), code, codeHash, 0) // populate once @@ -221,7 +221,7 @@ func BenchmarkCodeCache_GetByCodeHash_ManyAddrs_OneCode(b *testing.B) { // content-addressed codeHash→code, and the size layer — not just the addr // layer. The code's value is invariant for a hash, but its existence is not. func TestCodeCache_Unwind_DropsUnwoundCodeEverywhere(t *testing.T) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := makeAddr(1) code := bytes.Repeat([]byte{0x60}, 64) @@ -254,7 +254,7 @@ func TestCodeCache_Unwind_DropsUnwoundCodeEverywhere(t *testing.T) { // TestCodeCache_Unwind_BelowFloorSurvives verifies code deployed below the // unwind floor (still live after the unwind) stays warm on all layers. func TestCodeCache_Unwind_BelowFloorSurvives(t *testing.T) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := makeAddr(2) code := bytes.Repeat([]byte{0x61}, 32) @@ -274,7 +274,7 @@ func TestCodeCache_Unwind_BelowFloorSurvives(t *testing.T) { // on the live fork (current epoch) after an unwind makes it discoverable again, // even though a stale entry at the same txNum was left behind. func TestCodeCache_Unwind_RedeployRevives(t *testing.T) { - c := NewCodeCache(64*datasize.MB, 16*datasize.MB) + c := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := makeAddr(3) code := bytes.Repeat([]byte{0x62}, 48) diff --git a/execution/cache/code_cache_concurrency_test.go b/execution/cache/code_cache_concurrency_test.go index 37565a26595..9cf56b7508f 100644 --- a/execution/cache/code_cache_concurrency_test.go +++ b/execution/cache/code_cache_concurrency_test.go @@ -17,6 +17,7 @@ package cache import ( + "encoding/binary" "sync" "testing" @@ -35,7 +36,7 @@ import ( // goroutine that actually inserts accounts the size, so the counters must equal // exactly one entry regardless of how many concurrent Puts raced. func TestCodeCache_ConcurrentPutSameCode_NoSizeDrift(t *testing.T) { - cc := NewCodeCache(64*datasize.MB, 16*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := make([]byte, 20) addr[0] = 0xab @@ -69,7 +70,7 @@ func TestCodeCache_ConcurrentPutSameCode_NoSizeDrift(t *testing.T) { // an entry whose stored keyHash differs from the requested codeHash is treated // as a miss, so a 64-bit maphash collision can never serve the wrong code. func TestCodeCache_ByteCheckRejectsForeignKeyHash(t *testing.T) { - cc := NewCodeCache(64*datasize.MB, 16*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) code := []byte("contract A bytecode") realHash := crypto.Keccak256(code) @@ -97,7 +98,7 @@ func TestCodeCache_ByteCheckRejectsForeignKeyHash(t *testing.T) { // OnEvict-maintained byte counter must never drift negative under concurrency. func TestCodeCache_ConcurrentDistinctPuts_RespectCap(t *testing.T) { const codeCap = 4 * datasize.KB - cc := NewCodeCache(codeCap, 16*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(codeCap, 16*datasize.MB)) const workers = 128 var wg sync.WaitGroup @@ -124,13 +125,13 @@ func TestCodeCache_ConcurrentDistinctPuts_RespectCap(t *testing.T) { // authoritative Put must win over a conditional prefetch put in every // interleaving. func TestCodeCache_PutIfAbsentAtomicWithPut(t *testing.T) { - cc := NewCodeCache(64*datasize.MB, 16*datasize.MB) + cc := closeOnCleanup(t, NewCodeCache(64*datasize.MB, 16*datasize.MB)) addr := make([]byte, 20) addr[0] = 0xcd fresh := []byte{0xaa, 1, 2, 3} stale := []byte{0xbb, 4, 5, 6} for round := range 20000 { - cc.Delete(addr) + binary.BigEndian.PutUint64(addr[1:], uint64(round)) var wg sync.WaitGroup wg.Add(2) go func() { defer wg.Done(); cc.Put(addr, fresh, 20) }() diff --git a/execution/cache/coherence/coherence.go b/execution/cache/coherence/coherence.go index 44f50c80d8a..6410f69793d 100644 --- a/execution/cache/coherence/coherence.go +++ b/execution/cache/coherence/coherence.go @@ -64,9 +64,30 @@ func (g *Gen) load() *gen { } // IsStale reports whether an entry stamped (txNum, epoch) reflects dead-fork -// state after an unwind. +// state after an unwind, judged by the live coherence state. func (g *Gen) IsStale(txNum uint64, epoch uint32) bool { + return g.Snapshot().IsStale(txNum, epoch) +} + +// Snapshot is an immutable (epoch, floor) pair for judging entries against +// the coherence state captured at a chosen point — e.g. before loading a +// cache generation, so a concurrent Clear's re-init (fresh epoch, lifted +// floor) cannot revalidate a dead entry captured from the retiring +// generation. +type Snapshot struct { + epoch uint32 + floor uint64 +} + +// Snapshot returns the current (epoch, floor) pair. +func (g *Gen) Snapshot() Snapshot { s := g.load() + return Snapshot{epoch: s.epoch, floor: s.floor} +} + +// IsStale reports whether an entry stamped (txNum, epoch) reflects dead-fork +// state under this snapshot. +func (s Snapshot) IsStale(txNum uint64, epoch uint32) bool { return epoch != s.epoch && txNum >= s.floor } diff --git a/execution/cache/generic_cache.go b/execution/cache/generic_cache.go index 4d261f686f7..2dc3435ed37 100644 --- a/execution/cache/generic_cache.go +++ b/execution/cache/generic_cache.go @@ -18,8 +18,11 @@ package cache import ( "bytes" + "math/bits" + "runtime" "sync" "sync/atomic" + "time" "github.com/c2h5oh/datasize" "github.com/elastic/go-freelru" @@ -59,9 +62,10 @@ type entry[T any] struct { // 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 { - // data is the sharded LRU, replaced wholesale on a jump-grow. Load it once per - // operation; a write racing a resize may land in the LRU about to be replaced - // and be dropped — a benign miss (the value is re-read from the domain). + // data is the sharded LRU, replaced wholesale only with every put stripe + // 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]]] capacityB datasize.ByteSize mode Mode @@ -80,6 +84,14 @@ type GenericCache[T any] struct { resizeMu sync.Mutex reservedBytes int64 + // shardCount is the live generation's freelru shard count, bounded by + // shardCeil (freelru's own GOMAXPROCS-derived choice). Left to freelru, a + // grown generation could pick more, smaller shards and evict entries during + // the migration copy; instead shards double across grows only while + // per-shard capacity does not shrink (see maybeGrow). Mutated under resizeMu. + shardCount uint32 + shardCeil uint32 + currentSize atomic.Int64 // enveloped is set only when the cache draws from the shared envelope (via @@ -101,15 +113,28 @@ type GenericCache[T any] struct { hits atomic.Uint64 misses atomic.Uint64 inserts atomic.Uint64 - evictions atomic.Uint64 + evictions atomic.Uint64 // capacity evictions only, counted from Add's evicted return (see newShards) dropped atomic.Uint64 - staleEvicted atomic.Uint64 // entries dropped lazily on read after an unwind + staleEvicted atomic.Uint64 // stale entries detected on read after an unwind; dropped unless a racing put revived them sizeFunc func(T) int } func u64identity(k uint64) uint32 { return uint32(k) } +func nextPow2(v uint32) uint32 { + if v <= 1 { + return 1 + } + return 1 << bits.Len32(v-1) +} + +// initialShardCount starts a lineage at ~64 entries per shard (freelru's own +// small-cache geometry), bounded by ceil. +func initialShardCount(capacity, ceil uint32) uint32 { + return min(nextPow2(capacity/64), ceil) +} + const ( // genericCacheStartCapacity is the slot count a jump-grow cache is born with. // A cache whose working set never exceeds it (a test fixture) stays this small @@ -164,30 +189,48 @@ func newGenericCacheEntries[T any](capacityBytes datasize.ByteSize, capacityEntr sizeFunc: sizeFunc, } c.curCap.Store(capacityEntries) + c.shardCeil = nextPow2(uint32(runtime.GOMAXPROCS(0) * 16)) + c.shardCount = initialShardCount(capacityEntries, c.shardCeil) // Before any unwind every entry predates the (nonexistent) floor, so all // reads are valid; the floor only drops once an unwind happens. c.coh.Init() - c.data.Store(c.newShards(capacityEntries)) + c.data.Store(c.newShards(capacityEntries, c.shardCount)) return c } -// newShards builds a sharded LRU of the given capacity with this cache's evict -// callback wired, so currentSize follows capacity-driven eviction and Remove. -func (c *GenericCache[T]) newShards(capacity uint32) *freelru.ShardedLRU[uint64, entry[T]] { - lru, err := freelru.NewSharded[uint64, entry[T]](capacity, u64identity) +// newShards builds a sharded LRU of the given capacity and shard count (see +// shardCount; the 1.25 slack mirrors freelru.NewSharded, and per-shard sizes +// stay large enough that freelru's internal shard clamp never overrides the +// count) with this cache's evict callback wired. The callback is the sole +// subtractor of currentSize — every removal (capacity eviction, Remove) +// accounts through it. Freelru picks eviction victims per shard (hash bits +// 16+), which the put stripes (bits 0-7) don't cover, so any subtraction +// computed outside the callback races a cross-stripe eviction of the same +// 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]] { + lru, err := freelru.NewShardedWithSize[uint64, entry[T]](shards, capacity, capacity+capacity/4, u64identity) if err != nil { panic(err) } lru.SetOnEvict(func(_ uint64, e entry[T]) { c.currentSize.Add(-int64(e.size)) - c.evictions.Add(1) }) return lru } // maybeGrow jump-resizes the LRU one step larger when it is full, the ceiling // hasn't been reached, and the shared envelope can fund the step. Otherwise the -// LRU keeps its size and freelru evicts within it. Called with no lock held. +// LRU keeps its size and freelru evicts within it. Must not be called with a +// stripe held (it takes them all). +// +// The copy runs with every put stripe held: writers (and the striped +// stale-drop) are excluded, so no write can land in the generation being +// retired and a conditional put never sees a mid-resize gap it could fill +// with a stale value; readers stay on the retiring generation until the swap +// and never miss. Grows are a handful of steps per cache lifetime, so the +// writer stall is a bounded one-off. func (c *GenericCache[T]) maybeGrow() { c.resizeMu.Lock() defer c.resizeMu.Unlock() @@ -202,15 +245,41 @@ func (c *GenericCache[T]) maybeGrow() { if !cachebudget.Global.Reserve(delta) { return } - next := c.newShards(newCap) + // Shards double with capacity only while per-shard capacity does not + // shrink. The selection bits nest across power-of-two counts, so each new + // shard receives a subset of exactly one old shard and the copy below can + // never overfill one — freelru's own geometry for the larger capacity + // would pick more, smaller shards and evict during the copy. + perShardOld := (curCap + c.shardCount - 1) / c.shardCount + shards := c.shardCount + for shards*2 <= c.shardCeil && (newCap+shards*2-1)/(shards*2) >= perShardOld { + shards *= 2 + } + start := time.Now() + next := c.newShards(newCap, shards) // allocate before excluding writers + fenceStart := time.Now() + for i := range c.putStripes { + c.putStripes[i].Lock() + } + copied, evicted := 0, 0 for _, k := range old.Keys() { if v, ok := old.Get(k); ok { - next.Add(k, v) + if next.Add(k, v) { + evicted++ + } + copied++ } } c.data.Store(next) c.curCap.Store(newCap) + c.shardCount = shards + for i := range c.putStripes { + c.putStripes[i].Unlock() + } + c.evictions.Add(uint64(evicted)) c.reservedBytes += delta + log.Debug("[cache] jump-grow", "fromSlots", curCap, "toSlots", newCap, "shards", shards, "copied", copied, "evicted", evicted, + "alloc", fenceStart.Sub(start), "fenced", time.Since(fenceStart)) } // DomainCache wraps GenericCache[[]byte] to implement the Cache interface. @@ -255,6 +324,15 @@ func (c *GenericCache[T]) Get(key []byte) (T, bool) { // maxStep — the same coherence the BranchCache read applies for commitment. func (c *GenericCache[T]) GetWithTxNum(key []byte) (T, uint64, bool) { h := maphash.Hash(key) + // Snapshot coherence before loading the generation: judged against the live + // state instead, a Clear landing between the load and the staleness check + // re-inits coherence (fresh epoch, lifted floor) and revalidates a dead + // entry captured from the retiring generation. Paired with Clear re-initing + // only after its swap, an old-generation entry is always judged by a + // pre-init snapshot that still carries the unwind. A live entry judged by a + // pre-Clear snapshot only degrades to a miss (dropStale re-checks and keeps + // it). + coh := c.coh.Snapshot() lru := c.data.Load() e, ok := lru.Get(h) if !ok || !bytes.Equal(e.key, key) { @@ -270,8 +348,8 @@ func (c *GenericCache[T]) GetWithTxNum(key []byte) (T, uint64, bool) { // dead block — e.g. an EIP-4788 beacon-root write in the block-begin system // tx — and must be dropped; >= not > (the surviving block's last txNum is // floor-1, so this never drops a live entry). - if c.coh.IsStale(e.txNum, e.epoch) { - lru.Remove(h) + if coh.IsStale(e.txNum, e.epoch) { + c.dropStale(h, key) c.staleEvicted.Add(1) c.misses.Add(1) var zero T @@ -296,28 +374,49 @@ func (c *GenericCache[T]) PutIfAbsent(key []byte, value T, txNum uint64) { } func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) { + if c.putStriped(key, value, txNum, overwrite) { + // Grow outside the stripe — maybeGrow takes every stripe. + c.maybeGrow() + } +} + +// 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. +func (c *GenericCache[T]) putStriped(key []byte, value T, txNum uint64, overwrite bool) bool { h := maphash.Hash(key) valBytes := c.sizeFunc(value) newSize := len(key) + valBytes + 24 - ep := c.coh.Epoch() mu := &c.putStripes[h&(putStripeCount-1)] mu.Lock() defer mu.Unlock() + // Sample the epoch under the stripe: Clear resets the epoch counter inside + // the fence, so a stamp read outside could alias a future epoch and let a + // dead-fork entry survive a later unwind. + ep := c.coh.Epoch() lru := c.data.Load() existing, hasExisting := lru.Get(h) - // Existing key — update in place. Reuse the stored key buffer to - // avoid an extra allocation; the freshly-decoded value replaces the - // old one. + // 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 + // allocation; the freshly-decoded value replaces the old one. if hasExisting && bytes.Equal(existing.key, key) { if !overwrite && !c.coh.IsStale(existing.txNum, existing.epoch) { - return + return false } - lru.Add(h, entry[T]{key: existing.key, val: value, size: newSize, txNum: txNum, epoch: ep}) - c.currentSize.Add(int64(newSize - existing.size)) - return + // Reserve the new size before the removal: the byte counter must never + // transiently under-state usage, or a concurrent ModeNoOp admission on + // 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}) { + c.evictions.Add(1) + } + return false } if c.mode == ModeNoOp { @@ -325,16 +424,16 @@ func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) // 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) { c.dropped.Add(1) - return + return false } } - // ModeEvictLRU: grow toward the ceiling before inserting into a full LRU, so a - // busy cache expands into its budget rather than evicting at the start size. - if curCap := c.curCap.Load(); c.mode != ModeNoOp && curCap < c.maxCap && lru.Len() >= int(curCap) { - c.maybeGrow() - lru = c.data.Load() - } + curCap := c.curCap.Load() + // 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) + // In ModeEvictLRU the byte budget is enforced through the entry-count cap, // not a separate currentSize check: capacityEntries is derived from // capacityB (capacityB/avgBytesPerEntry, see NewGenericCache / @@ -348,35 +447,56 @@ func (c *GenericCache[T]) put(key []byte, value T, txNum uint64, overwrite bool) // balcache.go / db/state/cache.go accept. // hasExisting here means a 64-bit maphash collision (different key, same - // hash): freelru.Add replaces the colliding entry in place WITHOUT firing - // OnEvict, so subtract the displaced size now — otherwise currentSize drifts - // up by it permanently. + // hash): remove the colliding entry first so OnEvict accounts for it — + // freelru.Add would replace it in place without firing OnEvict. The size + // is reserved before the removal (see the update path above). + c.currentSize.Add(int64(newSize)) if hasExisting { - c.currentSize.Add(-int64(existing.size)) + lru.Remove(h) } keyCopy := common.Copy(key) - lru.Add(h, entry[T]{key: keyCopy, val: value, size: newSize, txNum: txNum, epoch: ep}) - c.currentSize.Add(int64(newSize)) + if lru.Add(h, entry[T]{key: keyCopy, val: value, size: newSize, txNum: txNum, epoch: ep}) { + c.evictions.Add(1) + } c.inserts.Add(1) + return needGrow } -// Delete removes the data for the given key. +// Delete removes the data for the given key. Runs under the key's put stripe +// so the check-then-remove is atomic against same-key puts and excluded from +// generation swaps (maybeGrow, Clear), which fence via the stripes. func (c *GenericCache[T]) Delete(key []byte) { h := maphash.Hash(key) + 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) } } +// dropStale removes key's entry under its put stripe: the re-check keeps an +// entry a concurrent put revived, and the stripe keeps the removal out of +// generation swaps. +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) + } +} + // Clear removes all entries from the cache. It also resets the (epoch, // unwindFloor) coherence pair: with no entries left, no stale (txNum, epoch) // can survive, so a fresh floor keeps subsequent Puts at the live epoch // serviceable. Mirrors CodeCache.Clear (which already did this — the two had -// drifted). +// drifted). The counter reset and the generation swap run with every put +// stripe held — like maybeGrow's — so a racing put can neither land in the +// retired generation nor add its size after the reset. func (c *GenericCache[T]) Clear() { - c.currentSize.Store(0) - c.coh.Init() // Shrink back to the start size and return the grown budget to the envelope, // keeping the cache adaptive across fork-validation/reset (it regrows on // demand). A no-op Purge would leave the grown slot array resident. @@ -386,8 +506,23 @@ func (c *GenericCache[T]) Clear() { cachebudget.Global.Release(c.reservedBytes - int64(c.startCap)*c.avgEntryBytes) c.reservedBytes = int64(c.startCap) * c.avgEntryBytes } + shards := initialShardCount(c.startCap, c.shardCeil) + next := c.newShards(c.startCap, shards) // allocate before excluding writers + for i := range c.putStripes { + c.putStripes[i].Lock() + } + c.currentSize.Store(0) + c.shardCount = shards c.curCap.Store(c.startCap) - c.data.Store(c.newShards(c.startCap)) + c.data.Store(next) + // Re-init coherence only after the swap: paired with GetWithTxNum's + // snapshot-before-load ordering, an entry captured from the retiring + // generation is then always judged by pre-init coherence that still + // carries the unwind. + c.coh.Init() + for i := range c.putStripes { + c.putStripes[i].Unlock() + } } // Close returns this cache's envelope reservation so later caches can grow into diff --git a/execution/cache/generic_cache_concurrency_test.go b/execution/cache/generic_cache_concurrency_test.go index 389f06b5521..0639c8e1131 100644 --- a/execution/cache/generic_cache_concurrency_test.go +++ b/execution/cache/generic_cache_concurrency_test.go @@ -19,9 +19,14 @@ package cache import ( "encoding/binary" "sync" + "sync/atomic" "testing" + "time" "github.com/c2h5oh/datasize" + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/maphash" ) // TestGenericCache_ConcurrentPutAcrossGrow guards the jump-grow data race: @@ -32,7 +37,7 @@ import ( // -race, this must stay clean. func TestGenericCache_ConcurrentPutAcrossGrow(t *testing.T) { // Budget well above the start size (1024 slots) so maybeGrow fires repeatedly. - c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + c := closeOnCleanup(t, NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU)) const workers = 8 const perWorker = 20_000 @@ -51,3 +56,380 @@ func TestGenericCache_ConcurrentPutAcrossGrow(t *testing.T) { } wg.Wait() } + +// A same-key put serialized by its stripe must never be undone by a grow: with +// copy-then-swap migration, a writer that loaded the old generation before the +// swap landed its write in the abandoned generation, and the migrated (older) +// value resurfaced as live — a stale serve, not a benign miss. The writer +// self-verifies each put and a reader checks the hot key's monotonically +// increasing value never goes backward. +func TestGenericCache_PutNotLostAcrossGrow(t *testing.T) { + value := func(n uint64) []byte { + b := make([]byte, 8) + binary.BigEndian.PutUint64(b, n) + return b + } + for round := range 50 { + c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + hot := []byte("hot-key") + c.Put(hot, value(0), 1) + + stop := make(chan struct{}) + var regressed atomic.Bool + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + for n := uint64(1); ; n++ { + select { + case <-stop: + return + default: + } + c.Put(hot, value(n), n) + if v, ok := c.Get(hot); ok { + if got := binary.BigEndian.Uint64(v); got < n { + regressed.Store(true) + return + } + } + } + }() + go func() { + defer wg.Done() + last := uint64(0) + for { + select { + case <-stop: + return + default: + } + if v, ok := c.Get(hot); ok { + if n := binary.BigEndian.Uint64(v); n < last { + regressed.Store(true) + return + } else { + last = n + } + } + } + }() + + // Cross the grow threshold so maybeGrow swaps the generation while the + // hot-key writer runs. + key := make([]byte, 8) + for i := range 3 * genericCacheStartCapacity { + binary.BigEndian.PutUint64(key, uint64(1+i)) + c.Put(key, []byte{1}, 1) + } + + close(stop) + wg.Wait() + c.Close() + require.False(t, regressed.Load(), "round %d: a striped put was lost across a grow (older value resurfaced)", round) + } +} + +// A conditional put must keep deferring to a live entry across a grow. The +// vulnerable writer class: a put of a brand-new key that lands in the +// retiring generation after the copy snapshotted Keys() is lost on the swap, +// and a follow-up PutIfAbsent finds the key absent and installs its stale +// value as live. With the fence the put either lands pre-fence (and is +// migrated — Keys() is taken with every stripe held) or lands in the new +// generation; either way the conditional put defers. +// +// A writer hammers fresh keys while the grow swaps generations; every key +// that straddled the swap is then probed with a stale conditional put. The +// grow is forced by lowering curCap over a lightly-populated cache, so +// capacity eviction cannot explain a missing key. +func TestGenericCache_PutIfAbsentDefersAcrossGrow(t *testing.T) { + fresh := []byte("fresh-value") + stale := []byte("stale-value") + for round := range 100 { + c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + key := make([]byte, 8) + for i := range 256 { + binary.BigEndian.PutUint64(key, uint64(1+i)) + c.Put(key, []byte{1}, 1) + } + before := c.data.Load() + c.curCap.Store(uint32(c.Len())) + + var candidates [][]byte + stop := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; ; j++ { + select { + case <-stop: + return + default: + } + k := make([]byte, 9) + k[0] = 0xfe + binary.BigEndian.PutUint64(k[1:], uint64(j)) + c.Put(k, fresh, 10) + candidates = append(candidates, k) + if c.data.Load() != before { + return + } + } + }() + + binary.BigEndian.PutUint64(key, 0) + c.Put(key, []byte{1}, 1) // insert at the lowered cap → triggers the grow + close(stop) + wg.Wait() + + for _, k := range candidates { + c.PutIfAbsent(k, stale, 5) + } + for i, k := range candidates { + v, ok := c.Get(k) + require.True(t, ok, "round %d: candidate %d missing", round, i) + require.Equal(t, fresh, v, + "round %d: candidate %d: PutIfAbsent installed a stale value over a put lost in the retiring generation", round, i) + } + c.Close() + } +} + +// A ModeNoOp admission must never observe the byte counter mid-update: the +// update path removes the old entry before adding the new one, and a +// concurrent insert on another stripe that reads the transient dip passes the +// budget check and lands over capacity — breaking "drop new keys when full" +// with a key that should never have been admitted. The counter is reserved +// before the removal, so the budget is transiently over-stated (at worst +// dropping a new key) and never under-stated. +func TestGenericCache_ModeNoOpAdmissionAtomicWithUpdate(t *testing.T) { + a := []byte("key-a-aaaaaaaaaaaaaa") + var b []byte + for i := 0; ; i++ { + cand := []byte("key-b-bbbbbbbbbbbbb" + string(rune('a'+i%26))) + if maphash.Hash(a)&(putStripeCount-1) != maphash.Hash(cand)&(putStripeCount-1) { + b = cand + break + } + } + v := []byte("valuevalu") // entry size 20+9+24 = 53: the budget fits exactly one entry + c := newGenericCacheEntries(datasize.ByteSize(53), 8, func(v []byte) int { return len(v) }, ModeNoOp) + c.Put(a, v, 1) + for round := range 200000 { + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(a, v, 2) }() + go func() { defer wg.Done(); c.Put(b, v, 1) }() + wg.Wait() + if _, ok := c.Get(b); ok { + t.Fatalf("round %d: ModeNoOp admitted a key past a full budget (SizeBytes=%d, capacityB=%d)", + round, c.SizeBytes(), c.CapacityBytes()) + } + } +} + +// A grow must migrate every entry. Left to pick its own geometry per +// generation, freelru chooses more, smaller shards as capacity rises, and a +// new shard that overfills during the copy silently evicts — keys clustered +// on the shard-selection bits vanish across a "grow", and a follow-up +// conditional put can install a stale value in the hole. Seeding writes the +// clustered keys after the pad so they are the newest in their shard and +// cannot be seeding-eviction victims; only the migration can lose them. +func TestGenericCache_GrowMigrationLossless(t *testing.T) { + c := NewGenericCacheWithAvg[[]byte](4*datasize.MB, 256, func(v []byte) int { return len(v) }, ModeEvictLRU) + defer c.Close() + + // Keys sharing hash bits 16-23 land in one shard of any generation with up + // to 256 shards. + target := (maphash.Hash([]byte("cluster-seed")) >> 16) & 255 + var clustered [][]byte + for i := 0; len(clustered) < 24; i++ { + k := make([]byte, 8) + binary.BigEndian.PutUint64(k, uint64(i)) + if (maphash.Hash(k)>>16)&255 == target { + clustered = append(clustered, k) + } + } + pad := make([]byte, 9) + for j := 0; c.Len() < genericCacheStartCapacity-len(clustered); j++ { + binary.BigEndian.PutUint64(pad[1:], uint64(j)) + c.Put(pad, []byte{1}, 1) + } + for _, k := range clustered { + c.Put(k, []byte("fresh"), 10) + } + for j := 1 << 20; c.Len() < genericCacheStartCapacity; j++ { + binary.BigEndian.PutUint64(pad[1:], uint64(j)) + c.Put(pad, []byte{1}, 1) + if j > 1<<21 { + t.Fatal("seeding could not fill the cache to the grow threshold") + } + } + before := c.data.Load() + c.Put([]byte("grow-trigger"), []byte{1}, 1) + require.NotEqual(t, before, c.data.Load(), "grow did not happen") + + lost := 0 + for _, k := range clustered { + if _, ok := c.Get(k); !ok { + lost++ + } + } + require.Zero(t, lost, "grow migration evicted clustered entries: per-shard capacity shrank across the swap") +} + +// A capacity eviction is a size-subtracting writer the put stripes cannot +// serialize: freelru picks its victim per shard (hash bits 16+), so an insert +// on one stripe can evict a key whose own update — on another stripe — is +// between its Get and Add; delta accounting against the pre-eviction size then +// double-subtracts. Capacity 1 collapses freelru to a single shard, making any +// two keys same-shard; the keys are chosen to differ in their put stripe. Each +// hit leaks negative size; drift accumulates and shows after the settle +// deletes. +func TestGenericCache_CapacityEvictionAtomicWithPut_NoSizeDrift(t *testing.T) { + c := newGenericCacheEntries(1*datasize.MB, 1, func(v []byte) int { return len(v) }, ModeEvictLRU) + a := makeAddr(1) + var b []byte + for i := 2; ; i++ { + b = makeAddr(i) + if maphash.Hash(a)&(putStripeCount-1) != maphash.Hash(b)&(putStripeCount-1) { + break + } + } + v := []byte("value-one") + for range 100000 { + c.Put(b, v, 10) + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Put(a, v, 10) }() // insert → evicts b (cap 1) + go func() { defer wg.Done(); c.Put(b, v, 20) }() // same-key update path + wg.Wait() + } + c.Delete(a) + c.Delete(b) + require.Zero(t, c.SizeBytes(), "capacity eviction raced the update-path delta") +} + +// A put samples the coherence epoch and then contends for its stripe; a Clear +// that wins the stripe first resets the epoch counter, so the put would stamp +// a pre-Clear epoch onto an entry landing in the post-Clear generation. Once +// a later unwind re-reaches that epoch value, the entry aliases the live +// epoch and serves dead-fork state despite its txNum being at or above the +// floor. +// +// The test holds the key's stripe to park Clear on it (before the reset, +// which runs inside the fence) and then the put behind it; waits beyond 1ms +// put the mutex in starvation mode, so unlocking hands the stripe FIFO to +// Clear first. +func TestGenericCache_ClearRacingPut_EpochAlias(t *testing.T) { + c := NewGenericCache[[]byte](64*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + defer c.Close() + c.Unwind(300) // epoch 0 -> 1 + + key := []byte("epoch-alias-key") + mu := &c.putStripes[maphash.Hash(key)&(putStripeCount-1)] + mu.Lock() + + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Clear() }() + time.Sleep(5 * time.Millisecond) + go func() { defer wg.Done(); c.Put(key, []byte("dead-fork-value"), 200) }() + time.Sleep(5 * time.Millisecond) + mu.Unlock() + wg.Wait() + + c.Unwind(150) // epoch 0 -> 1 again, floor 150 + + _, ok := c.Get(key) + require.False(t, ok, "entry at txNum 200 outlived an unwind to 150: its pre-Clear epoch stamp aliases the live epoch") +} + +// A reader that captures a dead (unwind-invalidated) entry from the retiring +// generation must not have it revalidated by Clear's coherence re-init: +// judged against the post-Init state (fresh epoch, lifted floor), the entry +// passes IsStale and dead-fork state is served. Coherence is snapshotted +// before the generation load, so an old-generation entry is always judged by +// coherence that still carries the unwind. +// +// The reader gates on the fence reaching the key's stripe — the last one the +// sweep locks — so its Get lands next to the Init that follows. +func TestGenericCache_ClearRacingGet_DeadEntryStaysDead(t *testing.T) { + var key []byte + for i := 0; ; i++ { + k := make([]byte, 8) + binary.BigEndian.PutUint64(k, uint64(i)) + if maphash.Hash(k)&(putStripeCount-1) == putStripeCount-1 { + key = k + break + } + } + dead := []byte("dead-fork-value") + c := NewDomainCacheMode(1*datasize.MB, ModeEvictLRU) + defer c.Close() + for round := range 2000 { + c.Put(key, dead, 200) + c.Unwind(150) // the entry is dead-fork state; it must never be served again + var served atomic.Bool + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); c.Clear() }() + go func() { + defer wg.Done() + mu := &c.putStripes[putStripeCount-1] + for range 1 << 16 { + if mu.TryLock() { + mu.Unlock() + continue + } + break + } + for range 4 { + if _, ok := c.Get(key); ok { + served.Store(true) + return + } + } + }() + wg.Wait() + require.False(t, served.Load(), + "round %d: Clear revalidated an unwind-invalidated entry for a concurrent reader", round) + } +} + +// The evictions counter must carry capacity evictions only. Routing +// intentional removals through it — decrement-compensated or netted against a +// removal counter at print time — races a concurrent stats reset: the swap +// straddles the paired updates, underflowing the counter or reporting phantom +// evictions that a later interval cannot retract. A Delete hammer with zero +// capacity pressure must therefore never surface a nonzero count, concurrent +// resets included. +func TestGenericCache_StatsResetAtomicWithDelete_NoPhantomEvictions(t *testing.T) { + c := NewGenericCache[[]byte](1*datasize.MB, func(v []byte) int { return len(v) }, ModeEvictLRU) + defer c.Close() + key := []byte("metrics-key") + stop := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for { + select { + case <-stop: + return + default: + } + c.Put(key, []byte{1}, 1) + c.Delete(key) + } + }() + total := uint64(0) + for range 1_000_000 { + total += c.evictions.Swap(0) + } + close(stop) + wg.Wait() + total += c.evictions.Swap(0) + require.Zero(t, total, "intentional removals surfaced in the evictions metric") +} diff --git a/execution/cache/grow_lru.go b/execution/cache/grow_lru.go index b2455f5f5c1..b0e3725028d 100644 --- a/execution/cache/grow_lru.go +++ b/execution/cache/grow_lru.go @@ -33,8 +33,14 @@ import ( // pre-commits its full configured capacity — the same demand-growth the state // caches use — reused across the CodeCache's content and size layers. // -// A write racing a resize may land in the LRU about to be replaced and be -// dropped; that is a benign cache miss (the value is re-read from the DB). +// 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. The onEvict-maintained 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]] onEvict func(uint64, V)