execution/cache, execution/commitment, db/state: consolidate cache stack (freelru + persistent code cache) - #22154
Conversation
…ack on freelru + add persistent code cache Converge the in-memory caches onto freelru.ShardedLRU and fold in the #22120 review findings, then add the persistent (MDBX-backed) code cache. freelru everywhere: - CodeCache content layers (hashToCode/codeHashToCode/codeSizeByCodeHash): maphash.Map -> freelru.ShardedLRU, so a full layer LRU-evicts the coldest entry instead of freezing and refusing new contracts (#22120 finding 1); OnEvict keeps the byte counters honest; keyHash collision + coherence.Gen invalidation preserved. - BranchCache tail: maphash.ShardedLRU -> freelru.ShardedLRU. #22120 review fixes: - putContent over-cap back-out goes through the accounted removal so a racing stale-drop can't double-subtract (finding 3). - account cache entry clamp 1<<22 -> 1<<24 so the configured 1 GB budget is actually reachable instead of capping residency at ~384 MB (finding 2). - trim comment-policy violations in domain_shared.go (finding 8). Persistent code cache (CodeStore): otter in-mem tier over a persistent MDBX TblCodeCache backing, both holding decompressed code keyed by keccak(code). Read-through in stateObject.Code (via the reader's CodeStore()), write-through on the CodeDomain flush at Commit, pruned in the forkchoice prune cycle. Gated by USE_CODE_STORE (default on). otter is the deliberate exception to the freelru-everywhere rule for the in-mem tier. go build, go vet, golangci-lint clean; cache/state/execctx/commitment tests green (incl. a new two-tier CodeStore test); -race clean on the cache package.
Comment-only cleanup (no behavior change) across the files this branch touches, per CLAUDE.md / .claude/rules/comments.md: drop issue/PR-reference narration, incident/rebase history, client name-drops, "NOTE:"/"WRONG choice" shouting, and "safety net" scope narration, keeping each comment's one-line technical "why". Pre-existing bare TODOs are left for a dedicated pass.
ada5ac0 to
b5c5261
Compare
The persistent CodeStore produced INVALID canonical blocks on reorg/unwind (BAL mismatch, wrong trie root) with it enabled. Root causes, all fixed here: - The app-level read-through in stateObject.Code resolved code by the stateObject's snapshot so.data.CodeHash, which can lag an in-block code change. Serve the code store from SD.GetCode instead, keyed off the mem-first codeHashForAddr resolution (reflects in-block writes), so the hash lookup is reorg-safe. The read-through is removed. - The cold-path SetMem stored (codeHash-from-AccountsDomain, code-from- CodeDomain) pairs, which parallel exec can observe momentarily inconsistent, poisoning the store with a hash->wrong-code entry that later flipped a SetCode prev-code comparison and the BAL. The store is now populated only by the flush callback (self-consistent keccak(v)->v). - The flush-callback code-store MDBX write ran mid-flushMem; defer it to after flushMem so it no longer interleaves with the in-progress domain flush. Removes the now-dead SetMem, ReaderV3.CodeStore and temporalGetter.CodeStore plumbing.
13ab6da to
fcfc99b
Compare
There was a problem hiding this comment.
Pull request overview
This PR consolidates and extends Erigon’s execution caching stack by standardizing in-memory eviction on freelru, introducing a persistent (MDBX-backed) code cache, and adding a “trunk-pin” BranchCache tiering + adaptive pin controller to improve cross-block commitment-trie locality.
Changes:
- Add a two-tier persistent
CodeStore(otter in-mem + MDBXTblCodeCache) and wire it into execmodule/SharedDomains code reads and prune cycles. - Rework
BranchCacheinto multiple tiers (resident account trunk, pinned per-contract storage trunks, freelru LRU tail) with preload/adaptive pinning support and new metrics. - Convert
CodeCachecontent-addressed layers tofreelru-backed LRUs and raise freelru slot-count safety ceilings to allow configured byte budgets to be reachable.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Adds github.com/maypok86/otter/v2 dependency for the in-memory CodeStore tier. |
| go.sum | Adds checksums for the new otter dependency. |
| execution/state/state_object.go | Minor formatting change near stateObject.Code() (no functional change shown in diff). |
| execution/stagedsync/rawdbreset/reset_stages.go | Clears commitment BranchCache during exec reset and warns when it cannot. |
| execution/execmodule/set_head.go | Wires CodeStore into the SharedDomains used by SetHead. |
| execution/execmodule/forkchoice.go | Wires CodeStore into forkchoice SD lifecycles; evicts persistent code cache during prune. |
| execution/execmodule/exec_module.go | Constructs CodeStore (gated by USE_CODE_STORE) and wires it into validation SDs; comment refactors. |
| execution/commitment/warmuper.go | Adds process-level counters for warmer branch-read outcomes. |
| execution/commitment/trunk_pin_metrics.go | Adds metrics emission for pinned/adaptive/preload-related BranchCache activity. |
| execution/commitment/preload.go | Adds resumable serial BFS contract storage-trunk preload logic. |
| execution/commitment/preload_ranges.go | Adds key-range helpers for contract trunk scans while minimizing imports. |
| execution/commitment/preload_parallel.go | Adds resumable wave-BFS preload using batched file-only branch resolution. |
| execution/commitment/preload_parallel_test.go | Adds extensive tests for the parallel preload and range helpers. |
| execution/commitment/hex_patricia_hashed.go | Adds counters for disk-load events during trie compute paths. |
| execution/commitment/commitmentdb/commitment_context.go | Adds diagnostic probing hooks to sample SD/tx layers and tag cache lineage. |
| execution/commitment/branch_cache.go | Major BranchCache redesign: resident trunk tiers, pinned storage trunks, freelru tail, miss hooks, new stats. |
| execution/commitment/branch_cache_test.go | Updates/adds tests for new BranchCache tiers and behaviors. |
| execution/commitment/adaptive_pin.go | Introduces adaptive pin controller driven by BranchCache miss pressure and preloads. |
| execution/cache/state_cache.go | Raises freelru slot-array ceiling to avoid silently capping configured byte budgets. |
| execution/cache/generic_cache.go | Raises freelru slot-array ceiling for generic caches to match domain cache sizing. |
| execution/cache/code_store.go | Adds CodeStore implementation (otter + MDBX) with stats and eviction. |
| execution/cache/code_store_test.go | Adds tests validating CodeStore two-tier behavior and eviction. |
| execution/cache/code_cache.go | Migrates code content layers to freelru LRUs; adds eviction accounting via OnEvict; updates insert path. |
| execution/cache/code_cache_concurrency_test.go | Updates concurrency test expectations for eviction-driven behavior. |
| execution/cache/code_cache_codehash_test.go | Updates tests to assert eviction (no freeze) for codeHash layer under tiny caps. |
| execution/cache/cache_test.go | Updates CodeCache capacity/eviction expectations after switching to freelru-backed content layers. |
| db/state/execctx/domain_shared.go | Adds CodeStore plumbing, write-through persistence on CodeDomain flush, and adaptive pin hook on Commit. |
| db/kv/tables.go | Adds new persistent table TblCodeCache to chaindata table set. |
| common/dbg/experiments.go | Adds USE_CODE_STORE experiment flag (default on). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (s *CodeStore) Evict(tx kv.RwTx) error { | ||
| if s == nil || s.tableCapBytes == 0 || uint64(s.tableSizeBytes.Load()) <= s.tableCapBytes { | ||
| return nil | ||
| } | ||
| c, err := tx.RwCursor(kv.TblCodeCache) |
There was a problem hiding this comment.
Addressed in 0950d78: Evict seeds tableSizeBytes from sumTableBytes(tx) on its first call (via a tableSeeded CAS), so a backing that already exceeds the cap is pruned rather than growing unbounded across restarts.
| sd.adaptivePinController.SetParallelMode(factory, provider) | ||
| sd.adaptivePinController.OnBlockComplete(ctx, sd.txNum, reader) | ||
| sd.adaptivePinController.SetParallelMode(nil, nil) |
There was a problem hiding this comment.
Addressed in 0950d78: the parameter is now txNum and the field is promotedAtTxNum (logged as txNum), so the diagnostics no longer mislabel a txNum as a block number.
| // BranchCache stores commitment-trie branch data: | ||
| // | ||
| // - Bounded LRU tail with configurable capacity (eviction is well-defined, | ||
| // suitable for long-lived caching across many Process calls without | ||
| // unbounded memory growth). | ||
| // - Single pinned slot for the root branch (always hottest, always present | ||
| // once populated, never subject to LRU eviction). Compact prefix of | ||
| // length 0 (or single-byte "no-key" form) targets this slot. |
There was a problem hiding this comment.
Addressed in fe86259: the type doc is trimmed to two sentences with no sub-heading or bullet list, per the comment policy.
| // Wiping the commitment table leaves the aggregator's in-memory branchCache | ||
| // referencing trie nodes that no longer exist on disk. A subsequent from-0 | ||
| // re-exec then reads those stale nodes when computing block 0's commitment | ||
| // and produces a wrong trie root under parallel exec. | ||
| // Drop the cache so it repopulates from the freshly-wiped table. |
There was a problem hiding this comment.
Addressed in c692bba: condensed to a single sentence.
| // Chain whenever a currentContext exists, not only when head-extending | ||
| // (header.ParentHash == head): fork-payload caching needs the parent link | ||
| // too — see the two-role breakdown below. | ||
| // | ||
| // Chain the validation SD to the latest in-memory canonical generation: | ||
| // e.currentContext when present, otherwise the newest in-flight commit | ||
| // generation (gate item 2 — the prior FCU cleared currentContext and | ||
| // handed its SD to the background commit). | ||
| // |
There was a problem hiding this comment.
Addressed: the CodeStore construction is now a plain 3-line if dbg.UseCodeStore { ... } with no comment block.
- CodeStore.Evict: tableSizeBytes starts at 0 each process start, so a persistent TblCodeCache that already exceeds the cap would never be pruned (unbounded growth across restarts). Seed it once from the table via sumTableBytes, in the same key+value byte units the eviction loop decrements. - BranchCache.Get: only count a pinned hit/miss when the prefix actually routes to a pinned storage trunk; account-trie and tail-only lookups no longer inflate pinnedMisses. - AdaptivePinController: the value threaded through OnBlockComplete is a txNum (that is what SharedDomains has at commit), not a block number; rename the parameter, the write-only promotedAtBlock field, and the log keys to txNum so the diagnostics are accurate. No behaviour change.
Address the #22154 review's comment-length notes: condense the BranchCache doc, the reset_stages branchCache-clear note, and the ValidateChain SetParent note to their load-bearing invariants. The design-doc detail (concurrency walkthrough, responsibility split, disk counters, two-role breakdown, cherry- pick note) moves out of source per the comment policy. No behaviour change.
Bring the cache stack current with main (parallel/streaming commitment correctness fixes #22184/#22113, nibblized-keccak cache #22185, erigondb.toml commitment referencing #21452, trie io.Writer trace #21859, etc.). One conflict in commitment_context.go's trieContext: keep both this branch's probeSd/probeTx (adaptive trunk-pin probe) and main's traceW (io.Writer trace).
…genesis The trunk-pin BranchCache eagerly allocated a 65536-entry (~512KB) depth-4 account-trie array per aggregator. In production this is one process-wide cache so the cost is trivial, but execution/tests spins up tens of thousands of ephemeral aggregators via GenesisToBlock, turning that eager alloc into GBs of churn (amplified ~10x under -race), long enough to hit the CI runner-reclaim window. - branch_cache.go: d4 is now an atomic.Pointer allocated lazily via CAS on the first depth-4 write; shallow test tries never pay for it. trunkSlot takes a forWrite flag so reads/invalidations don't allocate. - db/state: add AggOpts.DisableBranchCache(); GenesisToBlock opts out entirely (one-shot, no cross-block reuse).
putContent did a membership check, then accounted bytes, then inserted as three separate steps. freelru has no LoadOrStore, so two goroutines Putting the same cold code both missed the check, both added to the byte counter, and the second Add overwrote the first — leaving one resident entry but a counter inflated by 2x. The drift is permanent and compounds, wedging the byte-based stat. Serialize the check-account-insert per key hash with a striped mutex so distinct keys still put in parallel.
The BranchCache eagerly allocated a production-sized LRU tail (50k entries, ~3.5MB) at construction. In production there is one process-wide cache so the cost is paid once, but tests build one aggregator per test file for isolation (thousands in execution/tests), turning that eager tail into GBs of allocation churn — enough under -race to OOM the CI runner (peak RSS ~49GB vs ~23GB with the cache stack off). Add AggOpts.BranchCacheTailBudget: a byte budget translated to a tail entry count via TailCapacityForBudget. NewTest uses a small budget (256KB) so test aggregators keep the cache fully functional — the static resident trunk and the LRU tail are both exercised — while the tail stays cheap (test workloads don't rely on tail retention). Production is unchanged (default 50k entries). Measured execution/tests -race peak RSS: 49GB -> 22GB.
…churn The trunk-pin cache stack allocated its per-instance structures eagerly at full production size. In production there is one process-wide cache so the cost is paid once, but the execution-test suite spins up thousands of short-lived caches over shallow tries, turning the eager allocation into churn — which the race detector shadows into an OOM on the 16GB -race runner. The cache is memory-neutral without -race (measured); this makes the footprint demand-driven so it stays small when the workload is small, full when it isn't, with the same code in tests and production. - Trunk tiers d2/d3/d4 (the dense 2KB/32KB/512KB arrays) allocate on first write at that depth, via CompareAndSwap, like d4 already did. A shallow trie pays ~160B instead of 35KB. - The pinned per-contract map allocates on the first pin instead of eagerly. - The LRU tail allocates on first spill and jump-grows toward its cap under a shared, memory-derived budget (estimate.TotalMemory) rather than pre-reserving the full 50k-entry freelru; removes the earlier test-specific tail budget. - onCacheMiss no longer allocates per miss: Load-first (no discarded atomic.Uint64) and ContractHashFromPrefix decodes the leading 64 nibbles straight out of the compact bytes instead of materializing the hex expansion. - HashSort pre-sizes its key arena to min(pending keys, batch) so small commitments (genesis, small blocks) don't reserve the full 3.84MB scratch.
…bound app caches with one shared memory envelope The cache stack pre-allocated each cache to its full configured byte budget at construction, so a process that builds many cache instances (the execution-test suite) committed hundreds of MB per instance and could exhaust a constrained runner. Make the app caches adaptive to their environment instead of turning them off for tests. - common/cachebudget: one process-wide envelope = TotalMemory/32 (cgroup- and GOMEMLIMIT-aware), the single knob for aggregate cache residency. Reserve is all-or-nothing per growth step; Take is the unconditional initial slice; Release returns bytes on teardown/clear. - execution/cache: GenericCache jump-grows — starts at 1024 slots and resizes ×4 toward the byte-budget ceiling, funding each step from the envelope; a cache with a small working set never pre-commits its budget. Account/storage (NewGenericCacheWithAvg, per-domain avg) and the code cache route through the envelope; Close returns the reservation, Clear shrinks back to the start size. - execution/commitment: the BranchCache LRU tail draws from the same envelope (dropping the separate tail budget). The trunk is fixed-size by design, so it adapts by active-instance count instead — full depth-4 residency for a handful of caches (production), shallow (depth-2, deeper branches spill to the tail) once many are live (the test suite). Close decrements the instance count. - db/state, execmoduletester: wire Close so the envelope and instance count track real concurrency; the test harness now uses the production default cache (the jump-grow keeps it small) rather than a hand-tuned test-only size.
The CodeCache pre-allocated its content-addressed layers to full capacity at construction — most notably the fixed 1M-entry codeSizeByCodeHash (~52MB) plus the code LRUs — so every per-fixture CodeCache in the execution-test suite committed tens of MB up front. Held for each fixture's lifetime, their concurrent sum (amplified by the race detector) was the remaining driver of the execution-tests -race OOM after the state caches were already bounded. Add a generic growLRU[V] (start small, jump-resize ×4 toward a byte-budget ceiling, funding each step from the shared cachebudget envelope, shrink on Purge, release on Close) and route hashToCode, codeHashToCode, and codeSizeByCodeHash through it. A cache over few contracts stays KB-sized; a busy one grows into its budget. Per-instance and content-addressed as before — the addr layers and the keyHash-collision gate are unchanged, so correctness holds. Under a runner-like cap (14GB, 4 vCPU, no swap, -race) the recursive execution/tests now completes where it previously OOM-killed.
|
Follow-up item 2 ("Remove the GetCodeHash over-refresh", after #21536) now has a consolidated tracking issue: #22225. It captures the full next-PR scope — the GetCodeHash watermark-gated per-field refresh (the lever for the warm-extcodehash outlier and the 5/6-rank contract cells in the perf table above), plus the read-set double-probe collapse, the cellPool |
…n cut Design record for the deferred item-1 fix: move the batch resource cut into the apply loop (the only place that sees both exec and commitment progress), cancel both with a clean-exit cause vs error, and commit at a consistent boundary. Captures the requirement that the calculator may be more than one block ahead (fully BAL-driven mode) and the resulting flush-boundary crux: when commitment is folded ahead of exec's state at cut time, the excess folds must not persist past exec's position. Includes the required test matrix (unit cut-selection, clean-vs-error cause, orphan regression incl. many-blocks-ahead, drain ordering, hive + tip gates). Not implemented — deferred behind #22154.
yperbasis
left a comment
yperbasis
left a comment
There was a problem hiding this comment.
Reviewed at 2ca5333. Findings by severity; the two critical ones are consensus-safety.
Critical
-
execution/cache/code_store.go:70—GetByHashinserts the zero-copy MDBX slice fromtx.GetOneinto the process-lifetime otter tier (and returns it) without cloning.GetOnememory is invalid once the tx ends (db/kv/kv_interface.go:360), so a later mem-tier hit serves recycled pages as bytecode — wrong execution/state root, or a segfault on remap. Needs a copy beforemem.Set/return (the write path already copies, only the read-through re-cache is affected). -
execution/commitment/preload_parallel.go:122,preload.go:101— pins are stampedtxN=0, whichcoherence.IsStale(txNum >= floor) treats as unwind-immune, yet the pinned bytes come from mutable latest state (ttx.GetLatest/TblCommitmentValsoverlay). The intended backstop —sd.branchCache.Invalidate([]byte(diff.Key))inSharedDomains.Unwind(db/state/execctx/domain_shared.go:667) — is a no-op:DomainEntryDiff.Keystill carries its 8-byte inverted-step suffix, so it matches no cache tier. After a reorg the pinned tier serves dead-fork branches ahead of mem/DB/files → wrong trie root until demote/restart. The comments atdomain_shared.go:658and:1009promise the opposite behavior.
High
-
db/state/execctx/domain_shared.go:220— theAdaptivePinControlleris per-SharedDomains, but every production SD lives for exactly oneCommit(fresh SD per FCU / per batch), so each controller gets exactly oneOnBlockComplete: demotion (needs 5 consecutive calls on the same controller) and extension are unreachable, andSD.Closeneither demotes nor unbinds. Every pin (up to 8×4MB + a ~512KB depth-4 trunk skeleton per contract, per commit) is orphaned permanently in the aggregator-lifetime pinned tier, which never evicts and sits outside the cachebudget envelope — monotonic growth across a sync. Builder/fork-validation SDs also clobber the single miss-callback slot, zeroing the canonical controller's miss signal. -
execution/commitment/preload_parallel.go:172—Runloops forever when cumulative pin cost hitsstepCapexactly on a wave's last item with file-misses remaining:pin()setsbudgetHitonly on strict overshoot,fileBudget == 0defers everything, and the identical frontier is re-queued at the same depth with no other exit (no progress check, no ctx,nextDepthfrozen). It runs synchronously insideCommitholding the controller mutex and the write tx. -
execution/cache/code_cache.go:53— the content layers' byte cap became entry caps ofbudget/4096, but code runs up to 24,576B and is stored in bothhashToCodeandcodeHashToCode: worst case ~6GiB resident while cachebudget is charged 1GiB. LRU-evict is the right call (finding 1 of #22120), but it needs a real byte bound — actual-size accounting or a weigher, ascode_store.godoes with otter in this same PR. The deleted assertion inTestCodeCache_ConcurrentDistinctPuts_RespectCappinned exactly this invariant.
Medium
-
execution/cache/generic_cache.go:336— the ModeNoOp refuse guard comparesLen() >= maxCap, but the LRU is permanently at the 1024 start size (growth is mode-gated), so both refuse arms are unreachable and a "noop" cache silently runs as a 1024-entry LRU (droppedstays 0,evictionsclimbs) — invalidating the diagnostic baseline. Compare againstcurCap. -
execution/cache/code_store.go:109— the firstEvictper process cursor-walks the entire ≤1GBTblCodeCache(keys and values) inside the prune RwTx, holding the aggregator commit gate and, in background-prune mode, the pipeline semaphore the next FCU needs. Seed fromtx.BucketSize(O(1), accurate enough for the 90% heuristic) or persist the running counter. -
execution/commitment/branch_cache_tail.go/execution/cache/grow_lru.go/generic_cache.go— the jump-grow LRU is implemented three times and has already drifted:tailLRUhas noClose, soAggregator.Close(Clear+Close) leaks the tail's 256KiB envelope reservation per closed aggregator. Consolidate ongrowLRU(already generic over V); the tail getsClosefor free. -
execution/commitment/adaptive_pin.go—ctxis threaded throughOnBlockComplete→ promote/extend but nothing ever checks it (var _ = context.Background // reserved for cancellationat line 400), so preloads on the commit path are uncancellable despite the signatures. Wire it or drop the params. Same file:promotedAtTxNumis write-only andPromotedContracts()has no callers. -
db/state/execctx/domain_shared.go:1038—Commitre-implements the 8-byte inverted-step decoding ofTblCommitmentValsvalues owned by db/state, and the dbBranches provider swallows cursor errors into an empty/partial map, which downgrades DB-overlaid keys to "file-only" and pins stale file values as authoritative. Related altitude gap: CodeStore cap enforcement lives only in the forkchoice call site whilePutByHashalready has the tx, running size, and cap; and the 256MB otter tier doesn't reserve from the envelopebudget.godocuments as covering the code caches.
Low
- Hot path:
storageRoutepaysCompactToHex+ a 32-byte repack on every storage-prefix Get/Put/Invalidate before checkingpinned == nil— probe with the zero-allocContractHashFromPrefixfirst. Each flushed code blob is keccak-hashed and copied twice perCommit(flush callback +PutCodeWithHash), with no dedup across identical bytecode. The "parallel" preload resolver is a serial per-key loop (and a naive fan-out would race on the sharedDomainRoTxreader state — needs per-worker views if parallelized). - Duplication / dead code:
commitment.NextSubtreeduplicateskv.NextSubtree(the "imports minimal" comment is moot — the package already imports db/kv); the byte→nibbles loop now exists three times in the package (expandNibbles,ContractNibbles, inline inpreload.go);u64identis the third copy of the identity hash;mxPreloadDurationSecondsTotal/mxPreloadBytesTotalare registered but never incremented;PreloadContractTrunkand bothContractHash()getters have no callers; the six defaults are duplicated betweenDefaultAdaptivePinControllerConfigand the constructor clamps. - Env gates:
DISABLE_ADAPTIVE_PINis an inlinedbg.EnvBoolre-read (and re-warned) on every SD construction;BRANCH_CACHE_TRUNK_DISABLEis rawos.Getenv != "", so=falsestill disables and theERIGON_prefix doesn't work. Register both inexperiments.golikeUseCodeStore. AlsoUSE_CODE_STORE=falseafter running default-on strands the table — nothing ever drops or shrinksTblCodeCachewhen the store is nil. - Comments: "(added separately)" at
branch_cache.go:117and:503is stale — the adaptive layer lands in this PR; the(squeeze.go, trie_reader_integration_test.go, …)file inventory at:300and the// Concurrency:sub-heading at:55are the comment-policy patterns CLAUDE.md strips.
Verified non-issues (checked, not raising): the storageRoute/PinEntry check-then-act is serialized today by the controller mutex plus MDBX's single writer (though LoadOrStore would be cheap insurance); the misses map growth is bounded to ~2 SD lifetimes; the reset_stages.go assert/Warn is pre-existing behavior made loud, not a demotion.
Clean auto-merge, no conflicts. Build + unit tests (cache, commitment, state/vio, stagedsync) green; the typed-vio API change did not break the cache-stack callers.
…ell[T] + sync.Pool) (erigontech#21536) ## execution/state: typed versionedio read/write surface Types the versionedio surface end-to-end — `WriteCell[T]`, `VersionedRead[T]` / `VersionedWrite[T]`, typed per-path `ReadSet` / `WriteSet`, and `accounts.Code` — and removes the `any` boxing those (now base-typed) values make unnecessary on the read and validation paths. > **Scope — this PR is now a single type refactor.** It has been reduced to the typing change alone: it restores `main`'s logic verbatim and only changes types. The read-map pool, the exec-loop rework, and every other behavioural change are deferred to follow-up PRs. The intent is that it reviews as "`main`'s logic, retyped" — nothing else to reason about. ### Why this is deliberately mechanical This is a broad, disruptive change across sensitive parallel-exec logic. It is kept **mechanical** to derisk: it restores `main`'s logic and only changes types. It is **one step of a journey**, not the whole thing — the logical order is **change types now → remove state later**. ### The journey The endgame is for the **EVM interpreter to hold the typed write handlers directly, with the versionmap as the direct source of all EVM reads** — removing the IBS / `stateObject` intermediary, which is redundant for parallel execution. The `stateObject` does **two jobs**: caching read **deserialization** (bytes → object) and staging **local writes**. Removing it relocates both — read-decode caching into a **fully-available object state cache**, write staging into the versionmap's typed write handlers. So the prerequisite for the endgame is **reads returning objects, not bytes**, which needs that complete object cache (the State Cache line of work, erigontech#21380 / erigontech#21386). This PR makes the versionedio surface typed and alloc-free so it is *ready* to become that direct source. In the end state the **state cache has a dual role**: - **(a)** it reduces disk IO + deserialization cost (decoded objects are cached), and - **(b)** it is a **pool of allocated objects for the VM**, reducing GC churn (objects are reused rather than re-allocated per access). ### Benchmark evidence (`vio_exec_alloc_bench_test.go`) **1. De-boxing is a real read-path win** — same versionMap lookup, boxed (`Read()` → `ReadResult.value any`) vs typed (`ReadX`): | read | before (boxed) | after (typed) | |---|---|---| | balance (`uint256.Int`) | 33 ns, **32 B, 1 alloc** | 17 ns, **0 B, 0 alloc** | | nonce (>255) | 26 ns, **8 B, 1 alloc** | 16 ns, **0 B, 0 alloc** | | codehash (interned) | 0 alloc | 0 alloc | | storage (already typed) | — | 0 alloc | **2. But the dominant cost is the `stateObject`, not versionedio.** Alloc profile of an exec-shaped read loop (4 typed reads/tx): | allocator | share | |---|---| | `IntraBlockState.Reset` | 54% | | `getStateObject` | 40% (cum) | | `newTransientStorage` | 11% | | `readAccountData` (decode) | 6% | | **versionedio** | **does not appear** | ~99% of per-tx allocation is the `stateObject` lifecycle — the intermediary this PR prepares to remove. **The headline allocation win lands in that later step, not here.** ### Review follow-ups - **Read-map pool** — dropped. It was orphaned when the PR was reduced to "main + typing" (the block-end `Release()` lived in the stripped exec-loop rework); re-wiring would reintroduce that divergence and the empty-BAL risk. - **Validation tests** — restored `TestValidateRead_SDStaleness_InvalidatesPreDestructRead`, `…_RevivalKeepsReadValid`, `…_PriorAccountCreation_DetectedViaIncarnationPath`; added a `*VersionedWrite[T]` pool-reuse test. The two SD-revival bug-fixes are covered by `TestDeleteRecreateSlots*` under parallel exec. - **Comments / dead code** — trimmed stale docstrings and removed dead code (`mapRes`, `mapStorageValOK`, `mapResCodeBytes`, the `destructedVersion` field, uncalled `MarkNewReadsInternal` / `SnapshotVersionedReadKeys`, "Commit 2b/E" codenames). ### Behavior change vs main (disclosed per review) Commit `8814383304` is a real behavior change, not a verbatim restore of main's logic: empty-code writes now `DomainDel` the `CodeDomain` entry where main skips nil-code writes entirely. This fixes a main-side bug — clearing an account's code (e.g. an `eth_simulateV1` `stateOverride` of `"code":"0x"`, or a 7702 delegation clear) left the stale prior code in `CodeDomain`, inconsistent with the emptied `codeHash`, tripping the `ERIGON_ASSERT` commitment check (`INVALID`). Both write paths now key the code write off an explicit "code changed" signal and route empty code to a delete. Tracked separately for backport assessment in erigontech#22204. A second, smaller delta: the BAL codePath recovery now **skips** (increments `codePathRecoveryHashMismatch` + emits a `log.Warn`) when the recovered bytes don't hash to the emitted codeHash, whereas main unconditionally re-emitted the recovered code. The new behavior is deliberately safer — main silently persisted bytes that mismatched their hash — but it is a real delta from "main's logic verbatim", disclosed here for completeness. ### Explicitly deferred to the next PR (GetCodeHash over-refresh) This PR is a type refactor only — by design it does **not** restructure hot-path read/validation flow or change ownership semantics. The two lines of work are **split on purpose to keep this PR reviewable**: bundling the structural rework in would make it too big and would turn a mechanical retype into a risky functional change. The structural changes raised in review below are therefore **out of scope here** and will land in the immediate follow-up: the *"Remove the GetCodeHash over-refresh"* exec-path fix listed as follow-up 2 in erigontech#22154, which is sequenced to come directly after this PR. They are grouped there because they touch the same read/validation and write-ownership surface. - **`GetCodeHash` over-refresh** — `GetCodeHash` reads `CodeHashPath` 2–3× through the `GetCodeHash → versionedReadCore → getStateObject` nesting, plus a full-account `refreshVersionedAccount` and a per-read `SelfDestructPath` probe. The next PR cuts this to a watermark-gated, per-field refresh — the lever for the **warm-extcodehash** outlier and the contract first-touch / warm-call cells. - **Read-set-hit double map probe** — `versionedReadCore` calls `getHeader` for the version-gating decision and the typed wrapper then re-fetches the same cell for its value. Collapsing this to a single typed fetch means changing `getHeader` to carry the typed value; done as part of the `GetCodeHash` refresh rework rather than in this mechanical retype. - **`WriteSet` ownership by type** — `normalizeWriteSet`'s output shares cells with `blockIO`, and the "don't mutate a shared cell" rule is enforced only by comment. Making ownership a type-level property (owned/mutable vs shared/read-only view) is an API-design change deferred to the same follow-up; this PR keeps `main`'s sharing semantics unchanged. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Mark Holt <erigon@dev-bm-e3-ethmainnet-n4.erigon.io>
| target := int64(s.tableCapBytes / 10 * 9) | ||
| for s.tableSizeBytes.Load() > target { | ||
| k, v, err := c.Next() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if k == nil { | ||
| break | ||
| } | ||
| if err := c.DeleteCurrent(); err != nil { | ||
| return err | ||
| } | ||
| s.tableSizeBytes.Add(-int64(len(k) + len(v))) | ||
| } | ||
| return nil |
| scan := func(from, to []byte) { | ||
| for k, v, err := c.Seek(from); k != nil && err == nil; k, v, err = c.NextNoDup() { | ||
| if bytes.Compare(k, to) >= 0 { | ||
| return | ||
| } | ||
| if len(v) < 8 { | ||
| continue | ||
| } | ||
| m[string(common.Copy(k))] = common.Copy(v[8:]) | ||
| } | ||
| } |
| // Adaptive controller hook: decide promotions/demotions from this batch's | ||
| // miss pressure using the in-flight (pre-Commit) tx so commitment reads see | ||
| // the just-flushed bytes. Runs before Commit because the tx is finalized | ||
| // after; the coherence floor evicts pins from a rolled-back batch. |
| // Per-instance domain cache, held on the tester so Close releases its | ||
| // envelope reservation. The production default is safe here: the caches | ||
| // jump-grow from a small start into the shared envelope on demand, so a | ||
| // fixture with a small working set stays small regardless of the configured | ||
| // budget — no hand-tuned test size, same code path as production. |
| // Before any unwind every entry's txN is at/below the floor, so the epoch | ||
| // check never strands a valid entry. | ||
| bc.coh.Init() | ||
| log.Info("[branch-cache] init", "trunkEnabled", !bc.trunkDisabled, "tailCap", tailCapacity, "trunkDepth", maxDepth) | ||
| return bc |
yperbasis
left a comment
yperbasis
left a comment
There was a problem hiding this comment.
Reviewed at b5563e3583. Requesting changes: two must-fix bugs (one reproduced under -race) plus a pin-coherence gap. #22120 findings 1/2/3/8 are verified fixed; line refs below are against the PR branch.
Blockers
-
Data race in
tailLRU— reproduced with-race.branch_cache_tail.go:84readscurCapinAddunsynchronized;maybeGrow(:117) andresetwrite it underresizeMu. A concurrent-Put test crossing the 512-entry grow threshold fires the detector immediately (Add:84 vsmaybeGrow:117 viastore → tailForWrite().Add). It's the same race this PR fixes inGenericCache(atomiccurCap+ regression test citing the-raceeest shard), not applied to the tail copy — any-racerun that grows a tail goes red. Consider deduplicating the three grow-LRU implementations (GenericCacheinline,growLRU,tailLRU) while at it. -
CodeStore.GetByHashcaches mmap-backed memory in a process-lifetime cache.code_store.go:65-72stores thetx.GetOneresult into the otter tier (and returns it) without copying, violating the kv contract (db/kv/kv_interface.go:363: "must not be accessed after txn has terminated"). Once the tx ends and MDBX recycles pages, cached bytecode silently mutates → wrong execution → wrong state root.USE_CODE_STOREdefaults on, and the memdb same-tx unit test can't catch it. Fix: copy beforemem.Set/return. (PutByHashis fine — its input is the Commit callback's heap copy.)
Medium — correctness
-
Preload pins stamped
txN=0(parallel path alsostep=0) escape both coherence gates.preload.go:101,preload_parallel.go:122.txN=0is immune to the unwind floor thatdomain_shared.go:653-663explicitly claims covers "entries seeded by … the trunk preload"; pins are actually covered only by the changeset-gated eagerInvalidate, and changesets aren't generated below the reorg window — a deep unwind leaves stale pinned branches served afterwards (wrong root). The Commit-hook claim "the coherence floor evicts pins from a rolled-back batch" (domain_shared.go:1009) is wrong for the same reason.step=0separately defeats thecStep <= maxStepgate (domain_shared.go:1262-1274). The real(step, txN)are available at both sources (the provider strips the 8-byte inverted step it could decode; the read-fill path already usesMeteredGetLatestWithTxNfor exactly this). -
Pins have no owner across SD rotation.
AdaptivePinControlleris per-SD, pins live in the aggregator-scope cache, andBindis last-writer-wins on the single miss-callback slot (every ExecV3 run rebinds, incl. fork validation). A replaced SD's promoted contracts are never demoted → orphan pins (up to 64 MB × 8 per rotation) linger for cold contracts; meanwhile misses can attribute to a discarded fork-SD controller and stall promotion. Should at least be named in the planned controller-placement follow-up (or demote-all onBind).
Medium — performance
-
ShardedLRU.Len()locks every shard and is called on every insert in all three grow-LRUs — and the&&order evaluatesLen()before thecurCap < maxCapshort-circuit (generic_cache.go:346,grow_lru.go:92;branch_cache_tail.go:84has no short-circuit at all). The 256-shard tail pays 256 RLock/RUnlock per Put, forever, even fully grown. Reorder the conditions and/or track a cheap counter. -
storageRouteallocates before the nil-pinned check.branch_cache.go:402-427:CompactToHex+ a 32-byte packed key are computed on every ≥64-nibble lookup/store even when no contract was ever pinned. Hoist apinned.Load() == nil && !createearly return above the decode. -
The adaptive hook runs synchronously inside
Commiton the FCU critical path, and the dbBranches provider (domain_shared.go:1040-1058) scans the contract's entire MDBX-resident branch range per promoted/extended contract per Commit — pinning is budget-bounded, this scan is not. -
avgCodeEntryBytes = 4096under-sizes real bytecode. 512 MB budget → 131k entries per content layer while hot contracts skew 10-24 KB; the byte counters are now stats-only (nothing evicts on byte overrun, and the envelope accounts slots, not payloads) → multi-GB worst case across the two duplicated content layers.
Low — cleanup
-
Dead APIs (zero callers):
ProbeStateLayers/SiteIdentity(+probeSd/probeTx),TryClaimPreload,PinnedStats,PromotedContracts,WarmerBranchOutcomeStats,diskLoadStorage/diskLoadAccount(incremented, never read), andPublishMetrics— never called, so allmxPinned*/mxPreload*metrics are never emitted. The serialContractTrunkPreloadis unreachable in production (Commit always installs the parallel factory).ClearBranchCache's docstring claims a SetHead integration that doesn't exist.var _ = context.Backgroundinadaptive_pin.gois unnecessary (and the threadedctxparams are unused). -
Latent
PublishMetricswraparound:Clear()resetspinnedHits/Missesbut notlastPublished*→ uint64 wrap delta once it's wired. -
BranchCache.Closedoesn't release the tail's envelope reservation (asymmetric withCodeCache.Close/GenericCache.Close) — ~256 KB phantom reservation per closed aggregator with an allocated tail. -
Deleted tests:
TestBranchCache_StateKeyNeverCached(the invariant is still enforced but no longer pinned; updatable viatailLen()) andTestBranchCache_ShardedTailUnwindAcrossShards(the property still exists). A kept-and-extended tail concurrency test would likely have caught the blocker race. -
Misc: each deployed code is keccak'd and copied twice in
Commit(codeStore callback +PutCodeWithHashapply);code_store.gohas no license header,code_store_test.gosays 2024, andadaptive_pin.go/preload*.go/trunk_pin_metrics.gocarry truncated headers;state_object.gois a blank-line-only diff;DISABLE_ADAPTIVE_PINis read inline rather than declared inexperiments.golikeUSE_CODE_STORE;activeBranchCachesis ambient global state (>10 live caches silently shallow later ones — cross-test interference); the PR body referencesAB-METHODOLOGY.mdanddocs/plans/20260630-cache-adaptivity-consolidation.md, neither of which exists in the repo.
…Store mmap-retain Two review blockers: - tailLRU.curCap was a plain uint32 read in Add while maybeGrow/reset wrote it under resizeMu — the same race GenericCache already fixes with an atomic, not applied to the tail copy. Make curCap an atomic.Uint32; add a concurrent tail-grow regression test (run under -race). Also reorder the grow check to `curCap < maxCap && Len() >= curCap` in all three grow-LRUs so a fully-grown cache never pays Len()'s per-shard locks on every insert. - CodeStore.GetByHash cached and returned the mmap-backed tx.GetOne result into the process-lifetime otter tier without copying, violating the kv contract (bytes must not outlive the tx) — a recycled MDBX page could silently mutate cached bytecode and diverge the state root. Copy before caching/return. Also: Evict positioned its cursor with First() before Next(), and code_store.go gets the missing LGPL header.
…egator scope The pin controller held its recency accounting (promoted contracts, cold-block streaks) per-SharedDomains while the pins live in the aggregator-scope BranchCache. Because SDs bind continuously (every ExecV3 run + fork validation), that put the pin lifecycle under SD churn: a replaced SD's promotions were orphaned with no owner to age them out, and last-writer-wins on the miss callback let a fork SD's controller steal miss attribution. Move the controller to the same lifetime as the cache (the commitment Domain), fetched by SharedDomains via a duck-typed provider like the cache itself. Pin residency now ages purely by block-access recency across SD rotations. The tx-scoped reader/resolver-factory/dbBranches-provider are passed per OnBlockComplete call instead of stored via SetParallelMode, so the now-shared controller carries no per-tx mutable state (c.mu serializes callers); Bind becomes idempotent (one controller, one onCacheMiss). Also bounds the dbBranches provider scan by the per-contract pin budget and fixes its nil-upper-bound (all-0xff prefix) early-stop and swallowed cursor err.
…etrics, dead code) - CodeCache avgCodeEntryBytes 4KB -> 12KB: the entry-count cap is the only bound (byte counters don't evict), so size it to the resident-code skew (hot contracts 10-24KB) to keep RAM near the budget instead of several x over. - Wire BranchCache.PublishMetrics from OnBlockComplete so the pin/preload metrics are actually emitted, and reset the publish watermarks in Clear() so the next publish can't compute a wrapped delta. - BranchCache.Close now releases the tail's cachebudget reservation (was asymmetric with CodeCache/GenericCache.Close). - Remove dead APIs (PinnedStats, TryClaimPreload + its preloadClaimed field, PromotedContracts); lower the per-construction branch-cache init log to Debug. - Declare DISABLE_ADAPTIVE_PIN in dbg experiments (parity with USE_CODE_STORE) and trim two scope-narration comments per the comment policy.
…ders - Restore TestBranchCache_StateKeyNeverCached (state-key-never-cached invariant) and TestBranchCache_ShardedTailUnwindAcrossShards (lazy epoch+floor unwind across tail shards), updated to the current API. The latter is sized within the tail start capacity so it asserts the unwind floor deterministically rather than depending on budget-funded tail growth, and closes the cache to return its budget/active-count (avoids cross-test interference). - code_store_test.go copyright year 2024 -> 2026; complete the truncated LGPL headers on adaptive_pin.go / preload.go / preload_parallel.go / trunk_pin_metrics.go.
|
Thanks — worked through the review in order. Pushed 6266fac, f1d4cfe, 4e8c56a, 615c83e. Blockers
Medium — correctness
Medium — performance
Med 4/7 — the controller placement (the substantive change)Per the thread: the pin accounting was in the wrong place — per- Low — cleanup
Deferred (called out, not silently)
|
Perf A/B — head
|
| workload | base | PR | Δ |
|---|---|---|---|
test_ether_transfers_onchain_receivers (value transfers) |
31.6 MGas/s | 31.0 | −0.6 (noise) |
test_sstore_bloated (10 GB single-contract storage, existing_slots) |
61.9 MGas/s | 59.8 | −2.1 (noise) |
Both neutral — no measurable difference. On sstore_bloated the read counts are identical (rd=23.93k on both), i.e. the same commitment work; the pin changed nothing for that block.
Why (and why this is expected, not a knock): the adaptive trunk-pin acts at block boundaries (AdaptivePinController.OnBlockComplete → preload/pin a hot contract's trunk for the next block), driven by per-contract miss pressure it accumulates as it goes. So a single-block fork-validation run can't surface it: value transfers don't touch the storage-trunk path at all, and even single-block sstore_bloated only lets the controller learn from that block's misses and pin afterward — too late for the block being timed. The benefit is inherently cross-block: a live node processing consecutive blocks that repeatedly hit the same bloated contract, where block 1 warms the pin and blocks 2…N hit it.
Bottom line: neutral on the single-block benchmarks available here, with no regression — consistent with the pin being designed for a multi-block sstore-spam workload these fixtures don't reproduce. Validating the actual speedup needs a multi-block sequential import repeatedly touching the same bloated contract (timing blocks 2…N), which is a different harness than the single-block replay. Happy to run that if a fixture/setup exists.
Env note: reset copies of the 534 GB chaindata had to use direct-I/O (dd iflag=direct oflag=direct) — plain cp filled page cache to the RAM limit and got OOM-watchdog-killed.
yperbasis
left a comment
yperbasis
left a comment
There was a problem hiding this comment.
Re-reviewed at 615c83e053. Both blockers from the previous round are verified fixed (commitment + cache packages pass under -race locally, including the new TestBranchCache_ConcurrentTailGrow), pin stamps and the controller hoist check out. Approving.
Residuals — fine as follow-ups, not another round:
preload_parallel.go:134still stampsstep=0inPinEntry(the txN half is fixed). ThecStep <= maxStepgate (db/state/execctx/domain_shared.go:1280) therefore stays trivially satisfied for parallel-preloaded pins; per that gate's own comment the global floor is coarser than the per-key unwind signal, so a narrow stale window remains. If a merged-across-files branch genuinely has no single step, worth recording that rationale.code_store.go:144: the restructuredEvictloop checkserronly inside thek != nilbody, and cursors returnk=nilon error — a cursor failure now silently ends eviction withreturn nil, an error-handling regression vs the previous loop. Same pattern insumTableBytes(code_store.go:165).TestBranchCache_ConcurrentTailGrowgrows a tail and never closes it, leaking its reservation into the shared envelope for the rest of the package run; cheap todefer c.Close()now that Close releases it (same forTestBranchCache_StateKeyNeverCached).- Dead APIs remain:
ProbeStateLayers/SiteIdentity(plus theprobeSd/probeTxwiring feeding only them),WarmerBranchOutcomeStats, write-onlydiskLoadStorage/diskLoadAccount,ClearBranchCache(zero callers; docstring describes a SetHead integration that doesn't exist), andvar _ = context.Backgroundinadaptive_pin.go(the import is already justified by signatures). - Trivia:
state_object.gois a blank-line-only diff; each deployed code is keccak'd and copied twice per Commit (flush callback + apply).
…residuals - code_store: fix the Evict/sumTableBytes error swallow — a cursor error returns k=nil, which exited the k!=nil loop condition before the in-body err check; now checked after the loop. - branch_cache_test: defer Close on the ConcurrentTailGrow and StateKeyNeverCached tests so they return their tail reservation to the shared envelope. - preload_parallel: record why the parallel preload pins with step=0 (no single source step across merged files; pinTxNum already gives unwind coherence). - Remove dead code: ClearBranchCache, WarmerBranchOutcomeStats (+ its write-only counters), write-only diskLoadStorage/diskLoadAccount, ProbeStateLayers/ SiteIdentity (+ probeSd/probeTx wiring and SharedDomains.ProbeReadLayers), and the redundant `var _ = context.Background`. - state_object: drop a stray blank line (whitespace-only diff vs main).
|
Thanks for the approval. Cleared the residuals from the approval note in
Not done (the one remaining trivia item): the double keccak+copy of deployed code per Commit (flush callback + apply) — that's a genuine perf micro-opt best done as its own change; happy to file/track it.
|
## Problem
The `eest-spec-enginextests-benchmark-150m-{parallel,sequential}` jobs
intermittently kill their runner — the job log ends with `make: ***
Terminated` followed by `The runner has received a shutdown signal`, the
hosted-runner presentation of the VM exhausting memory. 5 of the last 54
`benchmark-150m` job instances died this way (e.g. [this run on
erigontech#22053](https://github.com/erigontech/erigon/actions/runs/29070572016/job/86291064304),
[an unrelated branch the same
morning](https://github.com/erigontech/erigon/actions/runs/29069679000),
[a sequential-variant
instance](https://github.com/erigontech/erigon/actions/runs/29067842538)),
always at 95–100% of the test phase, independent of the PR under test.
Profiling the 150m shard (locally, pinned to CI parallelism) shows peak
demand of **~16.8 GB against the runner's 16 GB**: a 9.9 GB evm process
footprint (live heap 4.9 GB, roughly doubled by default GOGC; dominated
by `opReturn` return-data buffers and `TemporalMemBatch` write-set
clones during the `test_unchunkified_bytecode` cases, which allocate
10.6–12.1 GB each) plus a 6.9 GB MDBX datadir sitting on the 8 GB
ramdisk — tmpfs bytes and process bytes compete for the same RAM. 1075
of the shard's 1077 tests share one `(fork, preAllocHash)` group, so a
single node's datadir grows for essentially the whole run and the peak
lands at the end. Whether a run survives comes down to GC timing and
randomized test order — hence the flakiness.
## Prior investigation
erigontech#22325 (closed in favour of this PR) profiled the same OOM and pinned
why the shard began flaking on Jul 7: the persistent code cache from
erigontech#22154 grew the benchmark datadir by ~2 GB (5.65 → 7.40 GB peak),
pushing the peak co-resident heap+datadir from ~12 GB to ~14+ GB — see
[the profiling
comment](erigontech#22325 (comment)).
The FCU-less CodeStore growth is being capped separately in erigontech#22335; with
the datadir off RAM, that growth no longer threatens the runner either
way — the two changes are complementary.
## Change
The ramdisk exists for shards that churn hundreds of short-lived
datadirs, where create/unlink journaling dominates. The benchmark shards
are the opposite shape (3 long-lived datadirs), so the ramdisk buys them
no wall time — and costs them the RAM that OOMs the runner.
- `tools/eest-spec-shards.yml`: new per-shard key `no-ramdisk: true`,
set on all 14 `enginextests-benchmark-*` shards. Opt-in-true on purpose:
a `false`-valued key would be invisible to both jq's `//` default and
GitHub expressions' loose `==` (`null == false` is true there).
- `.github/workflows/test-eest-spec.yml`: honors the key — skips
creating the tmpfs. Nothing else sets
`ERIGON_EXECUTION_TESTS_TMPDIR`/`TMPDIR` on Linux (the setup-erigon
TMPDIR override is Windows-gated), so these shards exercise the
env-var-unset path and their datadirs land in the runner's default temp
dir (`/tmp`, on the same root SSD as `$RUNNER_TEMP`).
- `tools/run-eest-spec-test.sh`: the same key also skips the local
(Darwin) auto-ramdisk for these shards — whose 2 GB default the 150m
datadir (~7 GB) could not fit anyway. Local runs likewise fall through
to the OS default temp dir.
Non-benchmark shards are unchanged.
## Wall-time evidence (A/B on identical runners)
[Dispatched run
29073818835](https://github.com/erigontech/erigon/actions/runs/29073818835)
— the 150m shards with datadirs on SSD, vs the five most recent green
ramdisk runs:
| | ramdisk (5 runs) | no ramdisk | delta |
|---|---|---|---|
| 150m-parallel, test phase | 16m40s – 17m06s | 17m09s | +1.8% vs mean |
| 150m-sequential, test phase | 11m14s – 13m06s | 13m22s | +8% vs mean,
within the baseline's own ±8% spread |
| 150m-parallel, job total | 20.3 – 22.5 min | 20.0 min | inside range |
| 150m-sequential, job total | 17.0 – 17.7 min | 17.3 min | inside range
|
Both A/B jobs passed, with ~7 GB more headroom at peak.
**Benchmark semantics note:** these are `--time` throughput shards, so
datadirs-on-disk puts SSD writeback inside the measured path — per-test
wall times (and any MGas/s derived from them) shift slightly at this
PR's boundary; the A/B above bounds it at ~+1.8% for the 150m-parallel
test phase. This is deliberate: production nodes run MDBX on SSD/NVMe
with the OS page cache, so the post-PR numbers are more representative
of real-world execution than tmpfs-backed ones. Treat pre-/post-PR
timings as different baselines when comparing historical job logs.
## Verification
- `actionlint`, `shellcheck`, `bash -n` clean; `make lint` clean.
- Matrix render (`yq -o=json` of the manifest) carries `no-ramdisk`
through to `matrix.*`; row parsing verified for benchmark, stable, and
race-regex shards.
- `make eest-spec-enginextests-benchmark-1m-sequential` run locally on
Darwin through the new path: no auto-ramdisk created, datadirs in the
default temp dir, all tests pass (1076/1076); a stable-shard control
still creates the ramdisk.
- On this PR's CI: every benchmark shard's `Create RAM disk` step is
skipped and its log prints the default-tmpdir routing, while stable
shards keep `tmpdir: /mnt/erigon-ramdisk`; the first CI Gate run had all
eest shards green including both 150m jobs.
- The no-ramdisk configuration was also validated end-to-end by the A/B
run above before this PR.
No Go code changes; this is CI/tooling configuration, so the TDD cycle
does not apply.
…tion (noMaterialize) + warm-read caching (erigontech#22409) ## What Makes the parallel execution path **cache-free** by unifying all reads/writes on the versionMap (the `noMaterialize` path): the per-tx `stateObject` is removed from parallel exec and `IntraBlockState` resolves state as `versionMap + journal`, with the Block Access List and OCC read-set derived from the same versioned reads. Removing the resident `stateObject` cost ~2.4x on warm reads, so this branch also adds a **read-side caching layer** that recovers it without reintroducing the stale-read bug: - **versionMap per-account locking** — `sync.Map` + per-`AddressEntry` `RWMutex` instead of one global `RWMutex` (removes reader-counter contention). - **Read-once fast-path** — return the value already recorded this tx (gated on a clean address) instead of re-probing the versionMap; conflicts still caught at commit by `ValidateVersion`. - **Committed-account caching** — `BlockStateCache.committedAccounts` → `sync.Map` (write-once-per-key, lock-free hits) + a per-tx memo of the committed fallback in `versionedAccountBase` (block-immutable, shared with read-only callers). ## Performance (benchmarkoor, 100M gas, parallel exec, 6 cores) Baseline = erigontech#22154 (`current` column from that PR). `serial` / `parallel` = this branch with serial- and parallel-commitment respectively. Both arms are **34/34 VALID**. Peers are one consistent pandaops snapshot; rank is erigon's position among the 6 clients. **serial → parallel commitment is a non-diff** (median p/s = 0.987), so the two arms give the identical rank picture. Headline: the `warm-*` / repeated-read family improves **1.5–6.7× over erigontech#22154**, with 7 cells climbing a rank and **no regressions**. warm-extcodehash now beats geth (2222 vs 1816). ### vs erigontech#22154 (baseline = erigontech#22154 `current`; serial + parallel commitment arms; improvement / rank / gap computed from the parallel arm) | cell | baseline | serial | parallel | reth | geth | besu | nethermind | ethrex | improvement | rank pre | rank now | rank Δ | gap to 1st | gap to 2nd | |---|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:| | sstore-bloated-slots-set | 365 | 382 | 392 | 1028 | 61 | 69 | 100 | 93 | 1.07x | 2/6 | 2/6 | 0 | 2.62x | 0.25x | | sload-bloated-slots-set | 392 | 379 | 339 | 1033 | 59 | 88 | 92 | 96 | 0.86x | 2/6 | 2/6 | 0 | 3.05x | 0.28x | | warm-callcode | 394 | 671 | 704 | 1520 | 534 | 92 | 330 | 781 | 1.79x | 4/6 | 3/6 | +1 | 2.16x | 1.11x | | warm-delegatecall | 361 | 714 | 690 | 1650 | 611 | 95 | 201 | 887 | 1.91x | 4/6 | 3/6 | +1 | 2.39x | 1.29x | | warm-staticcall | 217 | 625 | 658 | 1696 | 509 | 103 | 187 | 790 | 3.03x | 4/6 | 3/6 | +1 | 2.58x | 1.20x | | extcodecopy-contract | 1471 | 1449 | 1471 | 4055 | 602 | 147 | 276 | 1917 | 1.00x | 3/6 | 3/6 | 0 | 2.76x | 1.30x | | extcodecopy-missing | 2041 | 2564 | 2273 | 6339 | 585 | 295 | 403 | 4902 | 1.11x | 3/6 | 3/6 | 0 | 2.79x | 2.16x | | sstore-bloated-no-slots | 374 | 361 | 362 | 1015 | 60 | 111 | 503 | 297 | 0.97x | 3/6 | 3/6 | 0 | 2.80x | 1.39x | | warm-extcodehash | 334 | 2222 | 2128 | 6691 | 1816 | 296 | 271 | 4649 | 6.37x | 4/6 | 3/6 | +1 | 3.14x | 2.18x | | balance-eoa | 298 | 356 | 344 | 1169 | 135 | 88 | 465 | 139 | 1.15x | 3/6 | 3/6 | 0 | 3.40x | 1.35x | | warm-extcodesize | 1471 | 2632 | 2326 | 8199 | 2111 | 290 | 400 | 5073 | 1.58x | 4/6 | 3/6 | +1 | 3.53x | 2.18x | | sload-same-key-preset | 1786 | 2381 | 2273 | 8080 | 1610 | 422 | 449 | 2663 | 1.27x | 3/6 | 3/6 | 0 | 3.56x | 1.17x | | callcode-eoa | 280 | 324 | 328 | 1196 | 135 | 80 | 467 | 138 | 1.17x | 3/6 | 3/6 | 0 | 3.65x | 1.42x | | sload-same-key-no-preset | 1538 | 2273 | 2222 | 8122 | 1438 | 362 | 441 | 2560 | 1.44x | 3/6 | 3/6 | 0 | 3.65x | 1.15x | | call-eoa | 256 | 309 | 298 | 1148 | 133 | 78 | 442 | 138 | 1.17x | 3/6 | 3/6 | 0 | 3.85x | 1.48x | | staticcall-eoa | 277 | 316 | 311 | 1216 | 132 | 77 | 469 | 138 | 1.12x | 3/6 | 3/6 | 0 | 3.92x | 1.51x | | delegatecall-eoa | 281 | 314 | 279 | 1247 | 135 | 80 | 416 | 138 | 0.99x | 3/6 | 3/6 | 0 | 4.46x | 1.49x | | extcodesize-contract | 77 | 114 | 114 | 122 | 84 | 72 | 125 | 154 | 1.49x | 5/6 | 4/6 | +1 | 1.35x | 1.09x | | sload-bloated-no-slots | 369 | 345 | 341 | 978 | 58 | 176 | 497 | 343 | 0.92x | 3/6 | 4/6 | -1 | 2.87x | 1.46x | | warm-call | 228 | 540 | 532 | 1535 | 676 | 87 | 243 | 857 | 2.33x | 5/6 | 4/6 | +1 | 2.89x | 1.61x | | warm-balance | 1587 | 2222 | 2326 | 7468 | 2484 | 291 | 528 | 4171 | 1.47x | 4/6 | 4/6 | 0 | 3.21x | 1.79x | | extcodehash-missing | 376 | 380 | 366 | 1341 | 79 | 101 | 500 | 931 | 0.97x | 4/6 | 4/6 | 0 | 3.66x | 2.54x | | extcodesize-missing | 368 | 388 | 379 | 1387 | 79 | 96 | 513 | 987 | 1.03x | 4/6 | 4/6 | 0 | 3.66x | 2.61x | | staticcall-missing | 262 | 318 | 322 | 1222 | 77 | 87 | 492 | 948 | 1.23x | 4/6 | 4/6 | 0 | 3.80x | 2.95x | | balance-missing | 352 | 362 | 358 | 1390 | 80 | 100 | 547 | 968 | 1.02x | 4/6 | 4/6 | 0 | 3.88x | 2.70x | | callcode-missing | 332 | 337 | 328 | 1310 | 77 | 93 | 501 | 940 | 0.99x | 4/6 | 4/6 | 0 | 4.00x | 2.87x | | delegatecall-missing | 320 | 336 | 332 | 1368 | 78 | 94 | 457 | 952 | 1.04x | 4/6 | 4/6 | 0 | 4.12x | 2.87x | | call-missing | 318 | 328 | 306 | 1318 | 79 | 85 | 456 | 879 | 0.96x | 4/6 | 4/6 | 0 | 4.31x | 2.87x | | extcodehash-contract | 79 | 78 | 78 | 121 | 83 | 71 | 119 | 153 | 0.99x | 5/6 | 5/6 | 0 | 1.96x | 1.55x | | balance-contract | 76 | 79 | 78 | 121 | 83 | 74 | 129 | 154 | 1.03x | 5/6 | 5/6 | 0 | 1.97x | 1.65x | | delegatecall-contract | 76 | 75 | 75 | 111 | 85 | 66 | 109 | 155 | 0.99x | 5/6 | 5/6 | 0 | 2.07x | 1.48x | | callcode-contract | 78 | 74 | 75 | 123 | 84 | 66 | 104 | 155 | 0.96x | 5/6 | 5/6 | 0 | 2.07x | 1.65x | | call-contract | 76 | 74 | 74 | 121 | 83 | 63 | 117 | 154 | 0.97x | 5/6 | 5/6 | 0 | 2.10x | 1.65x | | staticcall-contract | 78 | 73 | 74 | 122 | 84 | 68 | 118 | 156 | 0.94x | 5/6 | 5/6 | 0 | 2.12x | 1.66x | ### Improving cells (summary: parallel vs erigontech#22154 + ranking movement) Cells that climbed a client rank or improved ≥1.30× over baseline. Seven cells climb a rank; the warm-* read family is where the read-side caching pays off. | cell | baseline | serial | parallel | improvement | rank pre → now | |---|--:|--:|--:|--:|:-:| | warm-extcodehash | 334 | 2222 | 2128 | 6.37x | 4/6 → 3/6 (+1) | | warm-staticcall | 217 | 625 | 658 | 3.03x | 4/6 → 3/6 (+1) | | warm-call | 228 | 540 | 532 | 2.33x | 5/6 → 4/6 (+1) | | warm-delegatecall | 361 | 714 | 690 | 1.91x | 4/6 → 3/6 (+1) | | warm-callcode | 394 | 671 | 704 | 1.79x | 4/6 → 3/6 (+1) | | warm-extcodesize | 1471 | 2632 | 2326 | 1.58x | 4/6 → 3/6 (+1) | | extcodesize-contract | 77 | 114 | 114 | 1.49x | 5/6 → 4/6 (+1) | | warm-balance | 1587 | 2222 | 2326 | 1.47x | 4/6 → 4/6 | | sload-same-key-no-preset | 1538 | 2273 | 2222 | 1.44x | 3/6 → 3/6 | ## Scope / follow-ups (not in this PR) - **Phase 2** — genesis / RPC commit via the write-set (default `noMaterialize`), dropping the remaining `stateObjects`/`nilAccounts`/`balanceInc` plumbing. - **Persistent account/slot cache** — the `-contract` / `*-bloated` / `*-missing` cells (bottom of the tables, ~1.0×) are `NO_CACHE` random cold reads over a bloated multi-GB state; they are cache-defeating by design and are the target of a separate persistent-cache PR, not the in-exec caching here. ## Testing - `execution/state` + `execution/vm` unit tests and `-race` green; `make lint` clean. - Consensus gated by CI `eest_stable` (max-failures=0). The `glamsterdam-devnet` shard is known-WIP upstream (CI `max-failures: 3473`). --- Follow-up work (single versionedio model, remove the stateObject) is tracked in erigontech#22458. --------- Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com>
execution/cache, execution/commitment, db/state: consolidate the cache stack
Consolidates the cross-block cache work onto one foundation: freelru everywhere,
the #21386 review fixes, and the persistent code cache.
What this does
accountTrunktiers (depth 1–4, per-slotatomic.Pointer) + per-contract pinned storagetrunks, an adaptive residency controller (promote/extend/demote hot contracts by miss
pressure), and wave-BFS preload/warmup, over the LRU tail. This is the trunk-pin base the
rest builds on.
hashToCode/codeHashToCode/codeSizeByCodeHash):maphash.Map→freelru.ShardedLRU, so a full layer LRU-evicts the coldest entryinstead of freezing and refusing newly-seen contracts (execution/cache: review findings for #21386 (StateCache LRU + (txNum,epoch) lazy unwind) #22120 finding 1).
maphash.ShardedLRU→freelru.ShardedLRU.entry clamp
1<<22→1<<24so the 1 GB budget is actually reachable, not capped at ~384 MB(finding 2); comment-policy trims in
domain_shared.go(finding 8).CodeStore: otter in-mem over a persistent MDBXTblCodeCachebacking (decompressed code keyed by keccak). Read-through instateObject.Code, write-through on the CodeDomain flush atCommit, pruned in theforkchoice prune cycle. Gated by
USE_CODE_STORE(default on). otter is the deliberateexception to freelru-everywhere for this tier.
Performance (100M, serial commitment)
Baseline note:
baselineis main before the cache effort began —a8eeb459(Jun 12, pre-#21380). It's chosen so this table captures the delta of all cache changes to
date as one package. In future PRs the baseline will be current main, so each PR shows
its own incremental delta rather than the cumulative one.
Method: amsterdam-bench-pd3, newPayloadV5 MGas/s, cores 0-5, N=3 median.
current= this branch, serial commitment, full cache stack on. Peers (incl. ethrex) from thetrunk-vs-others-100Msnapshot.rankis of 6 clients;gap to 1st/2nd= fastest/2nd-fastestpeer ÷ current.
What improved
EXTCODEHASH on existing contracts): ~1.5–1.87× (≈42→77 MGas/s), moving these 7 cells from
rank 6/6 → 5/6 — where the BranchCache trunk-pin + StateCache + CodeStore apply.
slightly (warm-callcode/delegatecall/balance, sstore-bloated-slots-set, 0.92–0.97×); these
are exec-path (MVCC over-refresh) / write-path bound, not cache-serviceable — follow-up below.
Consensus
eest-devnet(amsterdam BAL, parallel exec, serial commitment): 2572/0.--experimental.parallel-commitmentfails 39 of these tests, but that's the known,tracked parallel-commitment gap (Complete testing of parallel commitment and turn it on as default #21137), not this change — proven by isolation: caches
on vs off under parallel-commitment give the identical 39, and serial is 0. Deferred to Complete testing of parallel commitment and turn it on as default #21137.
#22120
Addressed findings 1, 2, 3, 8. Remaining (4/5/6/7 + enforcement, #22116) tracked as follow-up.
Follow-ups (ordered)
GetCodeHashreadsCodeHashPath2–3× through theGetCodeHash → versionedRead → getStateObjectnesting, plusa full-account
refreshVersionedAccountand a per-readSelfDestructPathprobe. Cut this to awatermark-gated, per-field refresh — the lever for the warm-extcodehash outlier and the 5/6
cells (contract first-touch + warm-call).
Note: the adaptive trunk-pin controller is currently wired via an in-flight-tx
OnBlockCompletehook; a follow-up either moves it to proper post-commit placement or shelves it.