State Cache Consolidation (PR #1 of the perf stack) - #21380
Merged
Merged
Conversation
Pure refactor — behavior preserved. Splits the encoded-branch→cells decode logic out of HexPatriciaHashed.unfoldBranchNode into a free function DecodeBranchInto so the same code is consumed by: - unfoldBranchNode (existing trie unfold path) - future cache populators (decoded-payload BranchCache) - future parallel pre-unfold orchestrator (Stage E) Today these would each have to re-derive the encoded-branch parsing logic. Centralising it ensures one decoder, one set of edge cases, one place to fix any bug in the on-disk format handling. DecodeBranchInto is intentionally PURE — it does not call deriveHashedKeys. Trie callers (which need hashed keys for the fold state machine) follow the decode with their own keccak loop. Cache callers can skip the derive step entirely until the cell is consumed by the trie. Tests: - TestDecodeBranchInto_RoundTrip: BranchEncoder.EncodeBranch produces bytes that DecodeBranchInto recovers cell-for-cell. Property test that keeps the canonical decoder consistent with the canonical encoder. - TestDecodeBranchInto_DeletedFlag: touchMap/afterMap convention with the deleted parameter. - TestDecodeBranchInto_TruncatedInput: clean errors on truncated input (no panic). Plus the existing commitment test suite (incl. trie-mismatch tests in TestBranchData_*) all pass without modification, confirming the refactor preserves unfoldBranchNode's behavior. This is the foundation for the next refactors in the representation-reduction track (see agentspecs/trie-data-pipeline-complexity-tax.md): subsequent PRs will introduce a decoded-payload cache that reads through this same decoder, and will lift unfoldKeyPath as a per-key traversal primitive that the warmer + future Stage E both consume. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces a new BranchCache type, distinct from WarmupCache and
designed for the longer-lived caching the cross-block persistence
work needs (step 7 of the representation-reduction sequence).
Distinguishing characteristics vs WarmupCache:
- Bounded LRU tail with configurable capacity (vs WarmupCache's
unbounded map). Suitable for caches that outlive a single Process
without unbounded memory growth.
- Single pinned slot for the root branch (compact prefix [0x00]).
Root never evicts. Atomic-pointer load/store on the hot read
path, no lock involved.
- dirty-flag + PutIfClean invariants — same semantics as the
invariants added to WarmupCache in the previous commit. Lets
cross-block writers race safely with fold updates.
- Lazy GetDecoded — same lazy-decode pattern as WarmupCache's
GetBranchDecoded; cells populated on first decoded-read and
cached for subsequent reads.
NOT yet wired into the trie's read or write paths. This commit just
adds the type, with tests. The trie integration (where this cache
plugs into branchFromCacheOrDB and the encoder's PutBranch) is the
discussion point at the step 6 boundary — see the conversation
captured at this point in the representation-reduction sequence.
Today the cache is intended to be ephemeral (per-Process,
constructed alongside the trie, dies with it). Step 7 lifts the
lifetime to the aggTx level for cross-block persistence; the cache
shape (bounded LRU + pinned root + dirty-flag) is in place ahead
of that.
Tests:
- TestBranchCache_RootPinning: root branch lands in pinned slot,
deep branches land in LRU tail; per-tier hit counters update
independently.
- TestBranchCache_RootSurvivesEvictionPressure: root persists when
tail is overfilled past capacity.
- TestBranchCache_DirtyFlag: PutIfClean refuses dirty entry,
unconditional Put replaces and clears dirty.
- TestBranchCache_GetDecoded: lazy-decode round-trip with
BranchEncoder; cells pointer reused across reads.
- TestBranchCache_Invalidate: removes from both tiers.
- TestBranchCache_Clear: empties both tiers, resets stats.
- TestBranchCache_Stats: deterministic format with per-tier counts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Doc-only addition to BranchCache's package-level comment, capturing
the caller invariants the cache assumes and the conditions under
which the existing concurrent trie satisfies them.
Three caller invariants:
1. Single writer per prefix at any moment.
2. Mark-dirty-then-Put discipline for racing writers.
3. Decoded cells from GetDecoded are read-only (alias entry storage).
The current ConcurrentPatriciaHashed satisfies all three by
construction:
- Mounts partition by first nibble (disjoint prefix spaces, no
cross-mount writes to the same key).
- Root branch written by single sequential fold post-Wait.
- Mount→root grid roll-up is rootMu-protected (in-memory grid
only, separate from cache writes).
Doc explicitly flags that any future parallel fold redesign (Stage F
in agentspecs/stage-e-pre-unfold-design.md) MUST preserve these
invariants — particularly the single-writer-per-prefix one, which
breaks if parent branches are written incrementally as children
complete in parallel. The required coordination layer goes at the
orchestrator (per-parent atomic counter; only the last-decrementer
writes the parent), NOT inside the cache. The cache's existing
primitives (atomic dirty flag, thread-safe LRU, atomic root pointer)
are sufficient for that orchestrator to build on.
Motivation: Stage F is likely deferred because the bench data shows
fold isn't the bottleneck for the canonical SSTORE-bloat workload.
But adding the constraint to the cache later (after caching is in
production) is much harder than documenting it now — correctness
regressions from a missed coordination layer can hide for many
blocks. Documenting the contract on the cache itself ensures any
engineer touching parallel fold sees it.
No code change. Doc-only. All tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 7a of the representation-reduction sequence: per-Process
integration of the BranchCache type added in the previous commit.
Plumbing:
- Trie interface gains SetBranchCache(*BranchCache).
HexPatriciaHashed implementation propagates to its branchEncoder.
ConcurrentPatriciaHashed implementation propagates the SAME
instance to root + all 16 mounts (sharing one cache is correct
under the concurrency contract — mounts partition prefix space
by first nibble, so cross-mount writes target distinct keys).
- InitializeTrieAndUpdates constructs a new BranchCache(default)
per trie instance and attaches it. Lifetime today = trie
lifetime = per-Process. Future cross-block persistence work
(step 7b) lifts this to aggTx scope by constructing the cache
one layer up and passing it in.
Read path (HPH.branchFromCacheOrDB):
L1 WarmupCache (existing) → L2 BranchCache (new) → L3 ctx.Branch.
L3 hits with non-empty result populate L2 so subsequent reads hit
L2 within the cache's lifetime. L1 stays first because warmup
workers may have pre-fetched with prefix-walk-derived freshness.
Write path (BranchEncoder.CollectUpdate):
- MarkDirty(prefix) BEFORE encode work — protects against
concurrent warmup-style writers racing into PutIfClean during
the encode (race documented in the cache's Concurrency Contract).
- Put(prefixCopy, updateCopy) AFTER ctx.PutBranch succeeds —
replaces the dirty entry with fresh canonical bytes. Single
writer per prefix per fold step (current sequential fold +
first-nibble mount partitioning) means no race on this Put.
Lifecycle:
HPH.Reset clears the BranchCache when called from the root trie
(gated by !hph.mounted). Mounted subtries share the root's cache,
so a mount calling Clear would dump entries the root still wants.
Carries the invariant from PR #19954 commit 1612d56.
Today's expected performance impact: minimal. Per-Process lifetime
means cache is empty at Process start, so first reads always miss.
The cache helps only branches that are read multiple times within
ONE Process — uncommon in current code paths. Step 7b is where
the real perf swing comes from (cross-block persistence so block
N reads hit branches written by block N-1).
This commit is the safe stepping stone: it validates the wire-up
end-to-end (read path + write path + concurrency contract +
lifecycle) without changing perf characteristics. Bench should
match Run I baseline (7.16 mgas/s on canonical SSTORE-bloat block).
All commitment tests pass, lint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the BranchCache was constructed inside InitializeTrieAndUpdates, giving it per-SharedDomains lifetime. SharedDomains is reconstructed for every batch / many tx boundaries — verified with logs in the prototype (921 fresh BranchCache constructions per bench run) — so the cache started cold every batch and never delivered the cross-block hits the design is about. Per-Process scope kept cache-cleanup correctness simple but defeated the whole point of caching. Place the cache on the commitment Domain struct (one cache per aggregator, matching the pattern on add_execution_context_with_caches where each domain owns its valueCache). ConfigureDomains attaches the cache once after domains are initialised — idempotent, lifetime = aggregator lifetime. AggregatorRoTx exposes BranchCache() returning the commitment domain's cache, so SharedDomains' construction path can fetch it without forcing db/state/execctx to import db/state (db/state already imports execctx via squeeze.go, so the reverse import would create a cycle). The placeholder commitment.BranchCacheProvider interface lets the SD construction path use a duck-typed type assertion on tx.AggTx() any. Plumb the cache through NewSharedDomainsCommitmentContext into InitializeTrieAndUpdates as an explicit parameter; nil falls back to a fresh per-init cache so test helpers without an aggregator still get a valid cache. Reset behaviour: HexPatriciaHashed.Reset no longer calls Clear on the cache. Aggregator-scope persistence requires the cache to survive Reset between commitment calculations. Callers that genuinely need to invalidate the cache (unwind, fork validation) now call ClearBranchCache explicitly. The bench is forward-only so this is a safe change for measurement; an explicit unwind clear path can land in a follow-up commit when needed.
Adds two debug-gated diagnostics for cross-block-cache-lifetime
investigations. Both off by default — meant to be flipped on when a
correctness regression surfaces and you need to localise *where in the
fold path* the cache started lying, instead of waiting for a downstream
trie-root-mismatch many blocks later.
1. BRANCH_CACHE_VERIFY: branchFromCacheOrDB cross-checks every L2
(BranchCache) hit against ctx.Branch and increments a divergence
counter when bytes disagree. Logs the prefix and both byte forms so
the first divergent read shows up directly in the erigon log.
BranchCache.VerifyDivergences() exposes the count for assertion in
tests.
2. BRANCH_CACHE_FINGERPRINT: SharedDomainsCommitmentContext.ComputeCommitment
emits a "[cache-fp]" log at end of every compute with (block, root,
cache fingerprint, divergence count). Two builds running the same
workload can be diffed offline ("first block at which their fp's
differ") to nail down the first block where lifecycle invariants
diverged. Fingerprint is an order-independent FNV-1a fold over
(key-hash, data-hash) pairs across both root and tail tiers.
Adds maphash.LRU.Range so the Fingerprint can iterate the LRU tail
without touching recency (Peek under the hood). The wrapper otherwise
discards original byte-keys on insert; mixing by hash instead of key is
correctness-equivalent up to hash collision, which is acceptable at the
working-set sizes here.
Motivation: today we just chased a wrong-root regression to commit
d673052 (aggregator-scope cache lifetime). Took two full bench cycles
(~12 min) to localise. With the divergence detector running, the first
divergent read would surface in the erigon log within seconds. With
fingerprint logging in two builds (one good, one regressed), the
diverging block boundary would be a single grep across two log files.
The deferred-encoding path (CollectDeferredUpdate + ApplyDeferredUpdates)
parallelises EncodeBranch + merge work at apply time. The cache
correctness consequence: between collect and apply, sd.mem holds the
old state while the cache is also unchanged. When apply finally fires
(end of Process or duplicate-prefix flush mid-Process), sd.mem
advances but the cache isn't touched — CollectDeferredUpdate doesn't
have a cache hook (and wiring one breaks
TestSharedDomain_RepeatedUnwindAcrossStepBoundary +
TestCustomTraceReceiptDomain because it violates an implicit
trie-during-Process cache stability invariant).
The result on the FV bench: cache stays at the very-first-Process's
read-side L3 fallback bytes for prefixes the trie writes, while
ctx.Branch advances to each block's actual state. Divergence on every
hot prefix (root, root-zone children) starting from block 2.
Use CollectUpdate (inline) instead. CollectUpdate writes
sd.mem + WarmupCache + BranchCache atomically at fold time via
PutBranch — cache mirrors sd.mem at every write, the trie sees its own
writes consistently, and the cross-Process cache state matches what
post-FCU MDBX commit produced. Loses the encoder's parallel-encoding
optimisation, but bench profile is I/O-bound, not encode-CPU-bound, so
the trade is favourable.
History (ETL) writes are still inline via DomainPut. Splitting that
("sd.mem inline, history queued for flush at FCU") is the proper
architectural answer to defer the slow disk work without touching the
sd.mem invariant — tracked as a follow-up.
Bisection helper: force branchFromCacheOrDB to skip the L2 BranchCache read path entirely so every read goes via ctx.Branch (sd.mem → MDBX). Cache writes still fire so verify-mode can keep comparing cache vs canonical. Flipping the env at runtime distinguishes "cache holds bad data" (bench passes further with cache reads off) from "deeper compute bug" (same failure regardless). Used in the 2026-05-06 investigation to confirm the cache was actively corrupting block 13's compute on the canonical SSTORE-bloat bench: with cache reads on, wrong-trie-root at block 13. With reads off, blocks 13-16 produce the correct roots and the bench advances to a separate failure at block 17 (unrelated pre-existing bug). Default off; gate via env DISABLE_BRANCH_CACHE_READS=true.
When verifyBranchCache=true and a cache hit disagrees with ctx.Branch, sample sd.mem, sd.parent.mem, and tx-direct (MDBX) for the same prefix and dump all layers in the divergence log line. Comparing those four byte sequences against the cached and canonical bytes pinpoints which state layer holds the bytes the cache disagrees with — the rewriter we need to identify before fixing the canonical-store-divergence bugs the cache currently exposes. Decision matrix (read off the log line): - cache != tx, sd.mem == cache → in-memory writer is fresh, MDBX is stale (commit-timing issue). - cache != tx, tx == ctx.Branch, sd.mem != cache, parent.mem != cache → MDBX has been rewritten by something outside the CollectUpdate write path (collation, file build, squeeze). - cache != ctx.Branch, parent.mem matches ctx.Branch but sd.mem doesn't → parent merge is the source. - cache != ctx.Branch, all of sd.mem / parent.mem / tx == ctx.Branch → cache itself was populated incorrectly (write-side bug). Three changes: 1. SharedDomains.ProbeReadLayers (db/state/execctx/domain_shared.go): public method that samples sd.mem, sd.parent.mem (private field accessed from the same package), and tx.GetLatest. Read-only; copies bytes so callers can hold them past tx lifetime. 2. TrieContext (execution/commitment/commitmentdb/commitment_context.go): add probeSd + probeTx fields populated at trieContext() construction; expose ProbeStateLayers method that delegates to sd.ProbeReadLayers. The local `sd` interface gets the ProbeReadLayers method too so the duck-typed reference can call it without an import cycle to execctx. 3. branchFromCacheOrDB log site (execution/commitment/hex_patricia_hashed.go): on divergence with verifyBranchCache, type-assert ctx for the probe interface and append sd_mem / parent_mem / mdbx fields to the log line. Existing field shape preserved for log parsers; new fields are additive. Pre-existing test failures (TestSharedDomain_RepeatedUnwindAcrossStepBoundary, TestValidateChainAndUpdateForkChoiceWithSideForksThatGoBackAndForwardInHeight) are unchanged — they were failing on the stack before this probe landed and are part of what the divergence work is meant to localise.
Per-write provenance for divergence-detection diagnostics. When a
divergence fires (cache hit disagrees with ctx.Branch), we now log
which write site produced the cached bytes and when, so we can
correlate the bad write against the FCU / build / step timeline.
Tag fields added to branchCacheEntry:
- origin short label of the write site (e.g. "CollectUpdate",
"L3-fallback-read")
- writeSeq monotonic counter per BranchCache instance
- writeTimeNanos unix nanos at write time
Put / PutIfClean signatures take an origin string. Two writers
updated:
- BranchEncoder.CollectUpdate → "CollectUpdate"
- branchFromCacheOrDB L3-fallback Put → "L3-fallback-read"
GetWithOrigin returns bytes plus the metadata; uses a non-counting
peek so it can be called alongside Get without double-counting hits.
The divergence-detection log site at branchFromCacheOrDB now appends
cache_origin / cache_seq / cache_t_ns fields. Combine with the
existing sd_mem / parent_mem / mdbx fields to localise both who wrote
the stale bytes and which layer the canonical value lives in.
Pre-existing test failures
(TestValidateChainAndUpdateForkChoiceWithSideForksThatGoBackAndForwardInHeight)
are unchanged from previous commits.
BranchCache previously sat in front of the sd.mem -> parent.mem -> MDBX
read chain (consulted in branchFromCacheOrDB before ctx.Branch). The
shared aggregator-scope cache was written from CollectUpdate by every
SD running commitment compute, including fork-validator SDs whose
writes never reach MDBX. Origin-tagged probe (run-step7b-probe-sdid)
showed five distinct SD pointers writing the same prefix to a single
cache entry, so any reader whose lineage didn't match the most-recent
writer saw bytes that disagreed with MDBX -> wrong trie root from
block 13 onward in the canonical SSTORE-bloat fork bench.
Layering after this change:
Read: sd.mem -> sd.parent.mem -> branchCache -> aggTx (MDBX)
Write: sd.mem only (DomainPut path)
Flush: sd.mem -> MDBX, then branchCache.Clear()
The cache now mirrors MDBX-flushed bytes only. Writers' in-flight bytes
live in sd.mem above; cache hits below sd.mem are always equivalent to
reading MDBX, so cross-SD pollution is impossible by construction.
Cache fills lazily on the MDBX-read path inside sd.GetLatest, and
clear-on-flush prevents pre-flush bytes from coexisting with new MDBX
state. Per-key invalidation is a follow-up (PR2).
BranchCache entries gain a step field so Get returns (data, step, ok)
matching the aggTx contract. Without this, sd.GetLatest's cache hit
returned step=0 and CheckDataAvailable rejected the boot SeekCommitment
with "commitment state out of date".
Removed:
- cache.Put from CollectUpdate (commitment.go)
- cache.Put + divergence detection from branchFromCacheOrDB
(hex_patricia_hashed.go); now just calls ctx.Branch
- L3-fallback Put (cache fills via sd.GetLatest now)
Validated on canonical cold bench (run-step8b): first FCU VALID, all
payloads through end VALID, 0 cache divergences, 0 wrong-root errors.
Prior probe bench had 23 divergences and INVALID payloads from the
fail block onward.
Probe scaffolding (SiteIdentity, ProbeStateLayers, divergence counters)
left in place for now; can be stripped in a cleanup follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ache state Update the BranchCache type comment to reflect the architectural state after the WarmupCache consolidation (steps 2a-2c, 3, 4): - BranchCache is the single branch cache (WarmupCache deleted) - Aggregator-scope lifetime, plumbed via BranchCacheProvider - Passive store: cache itself never reaches into underlying state - Branch warmer is branch-scoped; no leaf-data prefetch - Block-processing trie walker takes Updates from executor + memoization for siblings — no prefetch needed - Witness / proof generation walker drives its own state reads; if that path turns out to be cold-bound it indicates a need for separate account/storage caches (treat as separate concern with different scope/lifetime/invalidation; do not regrow the branch warmer to cover it) - disk_sto / disk_acc counters on cache-fp log surface any unexpected fall-through to ctx.Account / ctx.Storage as a signal of memoization gap or missing walker-side prefetch Doc-only; no behaviour change.
Adds a third cache tier between the root-pin slot and the LRU tail: a per-prefix pinned map fed by PinEntry. Pinned entries: - Never evict (no LRU pressure on this tier). - Are checked in lookup before the LRU tail, after the root slot. - Survive Put/SD.Flush updates: a Put for a pinned prefix updates the pinned entry in place rather than displacing it. Cross-block correctness via the existing dirty-flag invalidation discipline is preserved — the new bytes land in the same pinned slot. - Carry the same metadata as Put-tier entries (step, origin, writeSeq, writeTimeNanos) so divergence-detection and Stats treat them uniformly. Sized by the preload policy. Intended consumer is the storage trunk preload for big contracts (the 'storage root trunk cache for big accounts' direction): per-contract trunk branches at depth 65-70 get pinned at SD/cache creation, persist for the cache's lifetime. PinEntry is the public API; pinnedHits/pinnedMisses atomic counters are the new stats. PinnedCount() exposes current size for observability. Step 2 of the storage-trunk pin prototype.
Adds a function to pre-pin commitment branches for a given contract's storage subtree. Walks the trie depth-by-depth from depth 64 (storage subtree root) down to maxDepth: reads each branch via the supplied CommitmentReader, pins it via BranchCache.PinEntry, decodes the child bitmap, and recurses only into children that actually exist (no blind 16-way probing). contractHash is keccak256(address); the trunk lives at the prefix corresponding to the first 64 nibbles of the storage path. For a dense storage subtree (the SSTORE-bloat workload's bloat contract), expected pin count is ~16 + 256 + 4096 ≈ 4.4 K branches at depth 65/66/67, plus the root at depth 64. Sparse subtrees produce fewer. Loading strategy: per-prefix lookups via the reader. Simplest correct implementation. A bulk seg.Getter range-scan over sorted .kv would amortize disk seeks (per the parity-cluster observation in the consolidation memo) but requires building a prefix-range API on top of recsplit; defer until per-prefix lookup is shown to bottleneck. Step 3 of the storage-trunk pin prototype.
Adds a SharedDomains constructor hook that, when PIN_CONTRACT_TRUNKS is set, fires a one-shot background goroutine to preload the storage-subtree-trunk of each listed contract into BranchCache's pinned tier. Format: comma-separated list of 64-hex-char contract hashes (each is keccak256(addr)). Mechanism: - BranchCache.TryClaimPreload (atomic CAS) ensures the goroutine fires exactly once per cache lifetime, even though many SDs may be constructed (per-tx instances etc.). - Goroutine wraps sd.GetLatest as a CommitmentReader and calls commitment.PreloadContractTrunk for each contract hash, depth 64-70. - Logs progress per contract on completion. Closure-over-(sd, tx) is the prototype shape — works for the bench (both live for the whole process). Production deployment needs to revisit the lifetime — sd's tx may not outlive the goroutine. Step 4 of the storage-trunk pin prototype. Bench measurement is the next step (commit 5).
Previous async-goroutine shape (d204c1b) shared the SD's MDBX tx with the calling thread. Concurrent cursor use under the same tx tripped Go's cgo-pointer-pinning runtime check: panic: runtime error: cgo argument has Go pointer to unpinned Go pointer surfacing in an unrelated PruneBlocks goroutine during boot. Make the preload synchronous in the SD constructor for now: same TryClaimPreload guard (fires once per cache lifetime), but no goroutine. Boot pays the per-contract preload time as a one-off. Background-with-own-tx is the proper shape and remains a follow-up; owning the SD's tx exclusively for the preload duration is the safe shape until that lands.
… cap The previous bench (run-pin-trunk-instrumented-cold-cgroup-191347) hung at SD construction with no [trunk-preload] log lines for 5+ minutes. Erigon never reached "engine RPC ready" so all blocks came in as SYNCING. Two changes to localise + bound: 1. **Localisation**: add INFO logs at triggerTrunkPreload entry, per-contract starting/done with took, and a 500-prefix progress log inside PreloadContractTrunk. Whatever it does (or hangs on) is now observable. 2. **Bound**: cap PreloadContractTrunk at 10000 branches (vs ~4.4K expected for a saturated 4-level subtree at maxDepth=67). Drops maxDepth from 70 → 67 in the trigger (depth 64-67 = 16+256+4096 max branches) so we don't recurse into the per-slot tail where pinning has no value. Preload fails-fast on pathological subtrees rather than blocking SD construction indefinitely.
The previous shape (d204c1b) ran triggerTrunkPreload BEFORE sd.SeekCommitment in NewSharedDomains. Bench result: when the preload fired (PIN_CONTRACT_TRUNKS set), every subsequent block came back SYNCING — engine kept attempting backward-download which fails on this peerless setup, no block ever validated, no cache-fp ever fired. Without the preload firing, the same binary works fine (verify-bench PASS at 3.26s). Hypothesis (untested but matches the symptom): preload's sd.GetLatest reads ran before SeekCommitment had resolved the SD's view of the chain head. Pinned values were therefore inconsistent with the committed state, and the trie compute on the first block got wrong root → SYNCING → backward-download → no peers → death spiral with no Flush ever updating the (stale) pinned entries. Fix is mechanical: move the preload call to after SeekCommitment. The TryClaimPreload guard still ensures fire-once-per-cache lifetime. If subsequent bench shows pin_count > 0 + pin_hit > 0 + blocks validating normally, the hypothesis is confirmed; if SYNCING repeats, the bug is something else and we need to revert and debug differently.
…ParaTrieDB Previous prototype iterations both broke block validation: 1. Async sharing the SD's MDBX tx (d204c1b) → cgo "unpinned Go pointer" panic from concurrent cursor use. 2. Synchronous from NewSharedDomains (5a81976 / 4c9ead456d) → blocked the engine HTTP handler for ~3-4s during the preload window, causing the bench's first NewPayload to be dropped. Confirmed: the bench's height=24358001 is ABSENT from the erigon log; the next received block (24358002) then fails backward-download (no peers) → SYNCING forever. Restructure: - Move trigger from NewSharedDomains to EnableParaTrieDB. The latter is called from the staged-sync exec-stage init, NOT from request handlers, AND has access to a kv.TemporalRoDB. - triggerTrunkPreload now takes the DB (not a tx) and spawns a goroutine that opens its OWN tx via db.BeginTemporalRo. No shared cursors with the main pipeline; no blocking the engine. - Reader uses tx.GetLatest directly (not sd.GetLatest) — the SD layering would re-introduce shared-state risk and isn't needed (pinned bytes don't depend on sd.mem state). Same TryClaimPreload guard ensures the preload fires once per BranchCache lifetime regardless of how many SDs construct. If this works the bench should: - Show [trunk-preload] log lines firing once - Pin ~4369 branches - TEST block cache-fp shows pin_hit > 0 and files_comm < 1K - All blocks validate normally (no SYNCING failure)
Make the trunk-pin maxDepth configurable via env (default 67) so we can sweep depths to find the memory/perf sweet spot without rebuilding. Bump the per-contract maxBranches cap from 10K to 200K so deeper saturated subtrees don't get truncated mid-walk.
The previous code disabled the Warmuper for the parallel commitment path out of concern that it would interact with the calculator's SetUpdates call. In practice the Warmuper's reads are independent of the calculator's update buffer — they pre-fetch branch data while EVM execution runs, and the calculator's SetUpdates only affects ComputeCommitment's input set, not the warmup paths. Re-enabling produces a measured 8× throughput improvement on the perf-devnet-3 SSTORE-bloated benchmark (block 24358306, the canonical fixture for #20920), restoring the win first observed in Run H/I of the trie-perf investigation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds atomic counters (branch hit/miss/evict + bytes-served, account hit/miss, storage hit/miss) to WarmupCache, plus a Stats() string formatter and ResetStats() for per-Process accumulation reset. Counters are updated on every Get/Evict path (existing Put paths were already counted via cache size). Useful for: - Confirming warmup effectiveness in production logs - Per-block diagnostics when investigating commitment perf - Future per-pool dashboards once a coordinator/observability layer lands (tracked separately) No behavior change beyond the counter updates themselves. Stats() format is one line, suitable for embedding in the existing LogCommitments output. ResetStats() zeros counters without touching cached data — useful for per-Process windowed measurement. Clear() also resets counters along with the data, since data and counters were accumulated together. Test: TestWarmupCache_Stats covers hit/miss/evict accounting across branch/account/storage paths and verifies Stats() format + ResetStats() preserves data. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pure refactor — behavior preserved. Lifts the unfold-loop from
HexPatriciaHashed.followAndUpdate into its own method, parameterized
over (hashedKey, plainKey) and intended as the per-key traversal
primitive that future orchestrators consume.
Today only followAndUpdate calls it, replacing the inline loop with a
one-line call. The extracted method preserves the existing metric
attribution (StartUnfolding) and trace-print behavior verbatim.
Why now: this is the second step in the representation-reduction
sequence (see agentspecs/trie-data-pipeline-complexity-tax.md). Future
PRs will introduce orchestrators that drive unfold-only walks of
touched-key paths to fill cell state without going through the full
fold/update cycle:
- Cache populator (decoded-payload BranchCache) needs to walk a
touched-key path and capture the cells encountered, without
triggering fold or modifying the trie's update buffer.
- Stage E parallel pre-unfold orchestrator drives unfoldKeyPath
across multiple HexPatriciaHashed instances concurrently to
pre-warm trie state before commit.
Both consume the same primitive. Centralising it now means each future
orchestrator is a thin wrapper rather than a duplicate of the
unfold-loop logic.
Tests: full commitment test suite passes without modification (all 8+
test files in execution/commitment/), confirming the refactor preserves
followAndUpdate's behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the carry-as-is correctness invariants from the PR #19954 investigation as scaffolding on the existing WarmupCache: - branchEntry.dirty atomic.Bool — signals "stale until cleared" - PutBranchIfClean(prefix, data) bool — skips write if entry dirty - MarkBranchDirty(prefix) — mark for later refusal of stale puts These are scaffolding additions; no callsites use them yet. Existing PutBranch unconditionally overwrites and clears any prior dirty flag (creates a fresh entry), preserving today's semantic exactly for existing callers. Why now: the prototype investigation (see agentspecs/commitment-cache-prototype-dev-context.md) found that inline-invalidate-on-write is incompatible with deferred encoding — update-in-place breaks correctness because there's a window between fold (computes hash, holds new state) and encoder (writes encoded bytes) where readers see stale cached bytes. The reth-research (agentspecs/reth-1ggas-research.md §4) calls the dirty-flag pattern out as the design that resolves this without forcing synchronous encoding: the encoder marks the entry dirty BEFORE its own write completes, so any racing read knows to bypass the cache for that key. Today's WarmupCache lifecycle (per-Process, warmup completes before fold begins) does NOT exhibit this race — these invariants are infrastructure for the future cross-block persistence work where warmup-style writers can outlive their parent Process. Tests: - TestWarmupCache_DirtyFlag: PutBranchIfClean refuses dirty entry, unconditional PutBranch clears dirty. - TestWarmupCache_DirtyFlag_MarkAbsentKey: marking absent key is no-op (no panic, no entry created). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an additive read method that returns cached branches in already- decoded form, lazy-decoding on first decoded-read per entry and caching the parsed cells for subsequent reads. No existing callsite changes; existing GetBranch / GetAndEvictBranch / PutBranch callers continue to work with encoded bytes unchanged. Why now: this is step 5 of the representation-reduction sequence (see agentspecs/trie-data-pipeline-complexity-tax.md). The trie's read path currently does GetBranch (encoded) + DecodeBranchInto on every cache hit — paying decode CPU on every read. Switching that callsite to GetBranchDecoded (in a separate later commit) eliminates the redundant decode. The ENCODED form remains the source of truth — the encoder needs it for the merge-with-prev step, and it's what gets written to disk via PutBranch. The decoded form is derived lazily and cached alongside the entry. When PutBranch overwrites an entry, the new entry starts fresh and the next decoded read re-derives from the new bytes. API design notes: - Returns (bitmap, *[16]cell, ok). Caller derives touchMap/afterMap from bitmap based on its own deleted-vs-present-after context — the cache stores cells independent of that context so the same entry serves both readers. - The returned *[16]cell aliases entry-owned storage. Read-only consumption is safe across concurrent calls (decode runs at most once per entry via sync.Once); MUST NOT be modified in place. - Decode error → ok=false (don't count as hit OR miss; caller falls through to canonical re-read). Tests: - TestWarmupCache_GetBranchDecoded: round-trip equality with direct DecodeBranchInto, plus same-pointer reuse on repeat reads. - TestWarmupCache_GetBranchDecoded_Miss: behaves like GetBranch on absent keys. - TestWarmupCache_GetBranchDecoded_TruncatedData: graceful failure on corrupt entry (no panic). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ng payloads Closes a timing hole that surfaced once we wired the aggregator-scope BranchCache: between an FCU completing and the next newPayload, MDBX hasn't been committed yet (RunLoop's CommitCycle only fires under memory pressure), but currentContext.mem holds the latest writes from MergeExtendingFork. The fresh doms created in ValidateChain has no parent and a fresh roTx, so its ctx.Branch reads stale-MDBX while the aggregator-scope BranchCache (populated by the prior FV's CollectUpdate writes) holds the fresh state. That's the cross-newPayload divergence pattern observed in the bench (8-61 divergences and wrong-trie-root errors at block 3-14 across runs). Set doms.SetParent(currentContext) when the new payload extends the current canonical head (header.ParentHash == ReadHeadBlockHash). For fork payloads that don't extend head, leave parent unset: unwindToCommonCanonical below reverts doms's view to the common ancestor, and exposing currentContext.mem (post-divergence canonical writes) via the parent chain would shadow the unwound base and break fork validation. Verified by TestReorgsWithInsertChain — the "head-only" predicate is what the current single-canonical-chain SD topology supports. A proper per-branch SD lineage (each fork's validation chains to the last validated SD on its own branch, not always currentContext) is the follow-up needed for concurrent multi-fork validation. The current design supports a single canonical chain only; that's enough to close the divergence we have today, with the lineage extension tracked separately.
Foundation for the "Snapshot vs MDBX read-cost equivalence"
investigation (memory: snapshot-vs-mdbx-performance-equivalence.md).
This file produces the headline ratio that quantifies the gap the
investigation aims to close: warm-cache reads from snapshot .kv files
should cost the same as warm-cache reads from MDBX (same disk, same
page cache). H0 measures how far apart they are today.
Five sub-benches:
- MDBX_path full chain, key in MDBX
- File_path full chain, key in file
- Forced_file_path file-only debug path, file-resident keys
- Forced_db_path DB-only debug path, MDBX-resident keys
- Bloom_miss_path file-only debug path, MDBX-resident keys
(file misses in xorfilter for every probe)
Two operating modes; only synthetic is wired in this commit:
- Synthetic (testDbAndAggregatorBench fixture): writes 64 full
16-tx steps, BuildFiles + repeated PruneSmallBatches drains all
but the tip step into files. Phase 2 keys at txNums past the
built-step boundary stay in MDBX. Partition by *actual* residency
after setup so bench inputs match where keys really live.
- Real-datadir (--snapdatadir flag): TODO. Opens an existing
chaindata+snapshots datadir read-only and picks keys via cursor
iteration / .kv decompressor walk. Required for production-
relevant numbers since synthetic has tiny files and small values.
Initial synthetic results on AMD EPYC 4244P (Accounts domain):
MDBX_path 173 ns/op
File_path 226 ns/op (1.31x MDBX)
Forced_file_path 30 ns/op
Forced_db_path 158 ns/op
Bloom_miss_path 30 ns/op
Synthetic dataset is too small to surface the production gap that
pprof shows (xorfilter at 35% CPU on real bloat workload). H1-H4
benches and the real-datadir mode are the next steps.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the --snapdatadir flag path to the H0 bench. Opens an existing
chaindata + snapshots datadir read-only via the same recipe as
cmd/integration (mdbx Accede + state.New + temporal.New) and picks
keys by cursor-walking the per-domain values table.
Pragmatic adjustments:
- On heavily-pruned production datadirs (perf-devnet-3-run was
100% pruned), every MDBX values-table row is step-shadowed by a
file, so getLatestFromDb returns ok=false. The MDBX-side
sub-benches skip in this case; File_path numbers stand on their
own and the synthetic MDBX_path baseline serves as the cross-
mode comparator.
- skipIfEmpty short-circuits per sub-bench rather than failing the
whole run, so we can still get the file-path numbers.
First production numbers (AMD EPYC 4244P, AccountsDomain, 2012
file-resident keys from perf-devnet-3-run, fully pruned):
File_path 211 ns/op (synthetic was 226; essentially same)
Forced_file_path 30 ns/op (synthetic was 30; identical)
Surprising finding: real .kv file reads cost the same as synthetic.
This means production bloat-workload bottleneck is NOT in
getLatestFromFiles — it must be in HistorySeek (.ef history files
walked by HistoryStateReader.GetAsOf). The GetAsOf shortcut work
flagged in getasof-regression-suspect.md is the right lead.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related additions to the snapshot-vs-MDBX perf-equivalence
investigation (memory: snapshot-vs-mdbx-performance-equivalence.md):
1. H_GetAsOf bench (db/state/snapshot_vs_mdbx_bench_test.go).
New runHGetAsOf with four sub-benches for HistorySeek-via-GetAsOf
on file-resident keys: GetLatest_baseline, GetAsOf_recent (asOf
near endTxNum), GetAsOf_mid (asOf at endTxNum/2), GetAsOf_floor
(asOf=1). Tests the path the calculator's HistoryStateReader.Read
uses, which is distinct from getLatestFromFiles measured in H0.
Real-datadir results on perf-devnet-3 (AccountsDomain, endTxNum=2.9B):
GetLatest_baseline 202 ns/op 0 allocs
GetAsOf_recent 570 ns/op 0 allocs <- 2.8x baseline, no result
GetAsOf_mid 235 ns/op 5 allocs
GetAsOf_floor 196 ns/op 4 allocs
GetAsOf_recent (the calculator's pattern after PR #21010) scans
the .ef looking for a record at-or-after endTxNum-1, finds none
(most keys haven't changed in the last txNum), falls through to
GetLatest. The 370ns/op overhead vs GetLatest is wasted scan.
Confirms the GetAsOf shortcut described in
getasof-regression-suspect.md as a real lever, though small in
absolute terms (~2ms/block on the bloat workload).
2. Surface "took" + "keys" on the existing [commitment][cache-fp]
Info log line (commitmentdb). Was already computed in the
debug-level "[commitment] processed" log, but the bench runs with
--log.dir.disable so debug logs aren't captured.
This made it possible to attribute the 4.3s gap inside
newPayload(TEST block) directly: the calculator's ComputeCommitment
takes 4220ms for the 5910-key bloat block — 91% of the entire
block wall time. Per-key cost is ~700us, consistent across blocks
of all sizes. The actual perf lever for the bloat workload is
making per-branch ComputeCommitment cheaper, not file/state reads.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds three groups of fields to the existing
[commitment][cache-fp] log line so the calculator's per-block
behaviour is observable at Info level (the bench runs with
--log.dir.disable, so debug logs aren't captured):
- took, keys: ComputeCommitment wall + key-count for the block.
Pre-existing internally, now surfaced.
- load, skipped, reset: process-cumulative counts of
computeCellHash decisions:
* load = had no memoized stateHash, fetched value from DB
* skipped = had memoized stateHash, reused without fetch
* reset = had stateHash but had to invalidate it
Surfaced via new commitment.SkipLoadResetCounters().
- files_acc / files_sto / files_code / files_comm: per-domain
file-read counts pulled from sd.Metrics().Domains[domain].
Decomposes the aggregate `files=N` from the [domain reads]
log line into its actual sources (e.g. on the SSTORE-bloated
block the 32k file reads break down as 5.9k Storage value
loads + 26.6k Commitment branch reads + a handful of others).
All counters are cumulative; per-block deltas are obtained by
subtracting consecutive cache-fp lines.
Pure observability — no behaviour change. Used as the measurement
framework for the snapshot-vs-MDBX perf-equivalence investigation
and the follow-on commits that target specific levers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A committed SharedDomains is spent: callers open a fresh one (Commit + new SD, or Close) rather than reset and reuse. Both are now Deprecated to steer new code to Commit/Close; the only remaining callers are the exec --no-commit dry-run, which flushes a tx it never commits.
- cap the read-amplification dedup set (seenFileReads) at maxSeenFileReads so it can't grow unbounded under a long dbg.KVReadLevelledMetrics run - remove dead maphash funcs (LRU.Peek, Map.Range, Map.DeleteByHash) left by the removed fingerprint diagnostic - note that read-through BranchCache Put is correct only on the latest snapshot - de-duplicate the 'Commit advances cache only on success' comment (state it at the Commit docstring, drop the copies in executor/forkchoice/set_head) - trim editorial/scenario narration (HasStorage fall-through, UniqueLenBuckets depth table, seenFileReads trade-off essay) - stages.go chainTipMode: stop creating a throwaway SD + tx after the last block
Sweep the PR's net-added comments per CLAUDE.md/.claude/rules/comments.md: collapse multi-paragraph design/concurrency essays to one sentence, drop forensic detail (PR numbers, file:line refs, incident narration) and scope narration, and state each shared rationale once (import-cycle note at BranchCacheProvider; spent-SD lifecycle at Commit; read-chain ordering at the branchCache field) with terse pointers elsewhere. Comments only — no logic change.
Contributor
Author
|
Thanks for the thorough pass — all points addressed (latest commit Risks
Cleanups
Nit: removed the redundant trailing empty The SD-lifecycle change (point 3) is broader than a comment fix — it makes |
mh0lt
added a commit
that referenced
this pull request
Jun 17, 2026
awskii
added a commit
that referenced
this pull request
Jun 18, 2026
- adopt #21380 aggregator-scope BranchCache; drop per-trie WarmupCache plumbing (warmup_cache.go + obsolete tests removed; Warmuper page-cache prefetch kept) - adopt main's GenerateWitness(produceExclusionProofs) signature + witness tests - drop removed VariantBinPatriciaTrie; keep parallel/streaming trie variants
This was referenced Jun 18, 2026
awskii
added a commit
that referenced
this pull request
Jun 19, 2026
The aggregator-scope BranchCache (#21380) is shared across the parallel commitment workers' patriciaContexts and caches values aliasing freed .kv mmap; the mem-batch flush (SharedDomains.Commit memmove) then faults reading an unmapped region. Gate its creation off when parallel/streaming commitment is active. The fold-scoped file-view pin handles the fold-read path separately.
pull Bot
pushed a commit
to Dustin4444/erigon
that referenced
this pull request
Jun 21, 2026
…ming-commitment) (erigontech#21709) **Draft / checkpoint** — streaming + parallel commitment trie, with `main` merged in and the parallel-path stability bugs fixed and validated on mainnet. ## What Adds `StreamingCommitter`, a commitment engine that overlaps trie folding with block execution (touched keys folded into per-top-nibble splits during execution; root stitched at block end). Selected via `--experimental.streaming-commitment` (precedence: streaming > parallel > concurrent). The branch stacks concurrent → parallel → streaming; streaming reuses the parallel engine's prefix-trie / split machinery and delegates `Process` to the committer. ## Stability fixes (parallel/streaming) Live mainnet validation surfaced a deterministic wrong root and two `.kv` **mmap use-after-munmap** SIGSEGVs, all now fixed: - **Wrong root** (deep storage-fold path) — the mount-only fold is the correct parallel trie; the unsound deep storage split is off by default. - **Mem-batch alias — root cause of *both* crashes.** `TemporalMemBatch.putLatest` stored values without copying; under parallel commitment they alias a `.kv` mmap of the foreground exec tx's file generation, which a background merge munmaps mid-fold. `sd.mem` is read first by every worker's `TrieContext` (shared `SharedDomains`), so a concurrent worker (`TrieContext.Branch`) or the commit flush (`SharedDomains.Commit`) reads the freed pointer → fault. Fix: **copy-on-put** so `sd.mem` owns heap bytes (`8196bac851`). (Copy-on-*get* under `latestStateLock` can't fix this — the source is already munmapped before the copy runs.) - Supporting: a **fold-scoped file-view pin** across the parallel fold (`6fadc5d9aa`), and **gating main's erigontech#21380 BranchCache off under parallel/streaming** (`1fc7e2a605`) — it gives ~nothing under parallel (which warms itself) and complicates shared-context lifetimes. ## Status - **Mainnet (live node):** parallel & streaming cross the historically-failing blocks — **25320897** (prior deterministic wrong root) and **25346499** (prior mmap fault) — with **no state-root divergence and no crash**; validation ongoing toward tip. - **Parity:** streaming root + stored branches match `ModeDirect` / `ModeParallel` across multi-depth, incremental storage collapse (partial + full delete), and whale corpora; `-race -count` clean. - `make lint` clean; `make erigon integration` builds. ## Perf (1M-whale benchmark, 18 cores) | engine | time/op | vs sequential | |--------|---------|---------------| | sequential (ModeDirect) | 1.465 s | 1.0× | | parallel | 0.410 s | 3.6× | | streaming | 0.420 s | 3.5× | ## Known follow-ups - Correct **deep storage-interior split** (depth > 64); current flat 16-way per-nibble fold already recovers the win. - Fold is **sync-bound** (goroutine coordination), not compute-bound — optimization opportunity. - Re-evaluate whether the fold-scoped pin is still needed now that copy-on-put owns the mem-batch bytes; and whether erigontech#21380's BranchCache can be made parallel-safe rather than gated. - copy-on-put adds one alloc per `DomainPut` on the exec hot path (bounded; same order as what the state/branch caches already pay). --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
pull Bot
pushed a commit
to Dustin4444/erigon
that referenced
this pull request
Jul 1, 2026
…rigontech#21386) This PR ships the `execution/cache` **LRU + Mode** rework plus the `(txNum, epoch)` lazy-unwind coherence model for the state, code and commitment-branch caches, as the follow-on to [PR erigontech#21380 (State Cache Consolidation)](erigontech#21380). It was always meant to ship separately so the cache **policy** change can be reviewed independently of erigontech#21380's consolidation. > [!NOTE] > **erigontech#21380 is merged — this PR is now based on `main`** (no longer stacked on `mh/perf-caches-pr`). The branch has been merged up to `main` and is conflict-free. ## Scope **Cache structure & policy (`execution/cache`)** - Replace the `GenericCache` map (nuke-the-whole-map-on-low-hit-rate) with a **sharded LRU + `Mode`**, so eviction is per-entry and the working set warms up instead of being periodically dropped. - `STATE_CACHE_MODE` env override at `NewStateCache` time; production default caps are Account 1 GB / Storage 150 MB / Code 512 MB / Addr 16 MB. Test/CLI harnesses pass a small cache per instance (`ethconfig.Config.StateCacheBudget` for the `eth.New` path, an explicit cache via `ExecModuleTester`) so the full corpus doesn't allocate the production cache per fixture. - `CodeCache`: addr→maphash→bytes dedup (multiple addresses sharing one bytecode cost one copy), an addr→codeHash LRU, and a size-only (`EXTCODESIZE`/`EXTCODEHASH`) layer. The three content-addressed layers share one `putAccounted` insert path (skip-live / accounted-stale-drop / cap / back-out-on-overshoot) instead of hand-rolling it per layer. **`(txNum, epoch)` lazy O(1) unwind coherence (erigontech#21752)** - Every cached entry (account / storage / code / commitment-branch) carries `(txNum, epoch)`. An unwind bumps the epoch and lowers a floor — **O(1), scan-free**; stale entries drop lazily on their next `Get`. This replaces the eager `UnwindTo` walk for the BranchCache and gives all caches one uniform invalidation model. The `(epoch, floor)` primitive is factored into a leaf package `execution/cache/coherence` so the state/code/branch caches share one implementation. - Code **existence** (not just value) is honored: code deployed on a rolled-back fork stops being discoverable even by codeHash. **Cache ownership & coherence** - The state cache is an **app-level** concern that the **SharedDomains owns and manages**, not a storage-layer object. Coherence is enforced **structurally by the architecture**: app components reach state only through the SD, and the SD owns cache population (on flush) and invalidation (`sd.Unwind → stateCache.Unwind`). It is deliberately *not* also type-enforced — that would require the storage layer to depend on an app-level cache type, crossing the app/storage boundary — so the cache is injected via `SetStateCache` and managed by the SD thereafter. - The one component that **bypasses the SD** is the read-ahead warmup: each worker reads committed state from its own `RoTx` + `ReaderV3` (so it cannot see SD overlay / in-flight writes) and writes through to the shared cache directly. It therefore owns its own coherence: in-flight warmups are drained before an unwind bumps the cache epoch, so a fire-and-forget warm `Put` can't launder a dead-fork value as live. The drain makes the unwind wait; a drain-free alternative (capture the cache epoch in the getter and stamp/skip through it) is tracked as a follow-up in erigontech#22116. - **Architectural direction (out of scope here):** the "managed by convention" framing above disappears entirely if app-level domains become **first-class typed objects** that own their caches, so ownership and coherence are enforced by the type system rather than by routing discipline. A proof-of-concept is on the [`add_execution_context_with_caches`](https://github.com/erigontech/erigon/tree/add_execution_context_with_caches) branch, which can be integrated once the current performance changes are complete — too large to fold into this PR. **Commit-gating** - The state cache is populated only at flush and invalidated only on unwind (`Unwind(txNum)`), with `epoch` disambiguating a txNum reused across forks — no schedule-time poisoning. **Lock-free per-worker metrics (`db/state/kvmetrics`)** - Per-worker `DomainMetrics` combined via `Merge`, carried through a context-passed collector (no process-wide write lock on the read path). Metric types are **relocated to their rational home** `db/state/kvmetrics` (they had been parked in `db/state/changeset`). The process-level collector self-manages its goroutine lifecycle rather than registering on `Aggregator.wg`. ### Removed during the merge: the unsound CodeDomain codeHash bypass The original handoff list included a "SD-transparent codeHash bypass for `CodeDomain`" (`cb4443bf51`). It was **found to be incorrect and has been removed**. It answered `GetLatest(CodeDomain, addr)` from the account's codeHash via the content-addressed cache, but the account record can be *ahead* of the per-address `CodeDomain` value (the account commits with codeHash `H` before the code write for that address lands in the queried layer). The phantom value was then taken as the `prevVal` of the deploy's code write; since it equalled the identical bytecode being written, the `DomainPut` diff elided the write and the code was never persisted — surfacing later as `ErrNoCode` (regression in `TestCustomTraceReceiptDomain` with the cache on). Code dedup-by-hash still happens soundly in the addr-keyed cache; only the unsound shortcut is gone. ## Validation - `make lint` clean; full `erigon` + `integration` build clean. - Green (cache on, default): `execution/cache` (incl. `-race`), `execution/state`, `db/state/...`, `execution/commitment/...`, full `execution/stagedsync`, and `execution/tests` in **both serial and parallel** (`EXEC3_PARALLEL=true`). --------- Co-authored-by: Mark Holt <erigon@dev-bm-e3-ethmainnet-n4.erigon.io> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pull Bot
pushed a commit
to Dustin4444/erigon
that referenced
this pull request
Jul 6, 2026
…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>
pull Bot
pushed a commit
to Dustin4444/erigon
that referenced
this pull request
Jul 7, 2026
…ack (freelru + persistent code cache) (erigontech#22154) # execution/cache, execution/commitment, db/state: consolidate the cache stack Consolidates the cross-block cache work onto one foundation: freelru everywhere, the erigontech#21386 review fixes, and the persistent code cache. ## What this does - **BranchCache resident trunk-pin** — a resident, lock-free upper-trie cache: fixed-array `accountTrunk` tiers (depth 1–4, per-slot `atomic.Pointer`) + per-contract pinned storage trunks, 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. - **freelru everywhere** — the in-memory caches share one eviction structure: - CodeCache content layers (`hashToCode` / `codeHashToCode` / `codeSizeByCodeHash`): `maphash.Map` → `freelru.ShardedLRU`, so a full layer **LRU-evicts** the coldest entry instead of freezing and refusing newly-seen contracts (erigontech#22120 finding 1). - BranchCache tail: `maphash.ShardedLRU` → `freelru.ShardedLRU`. - **erigontech#21386 review fixes (erigontech#22120)** — accounted over-cap back-out (finding 3); account-cache entry clamp `1<<22`→`1<<24` so the 1 GB budget is actually reachable, not capped at ~384 MB (finding 2); comment-policy trims in `domain_shared.go` (finding 8). - **Persistent code cache** — a two-tier `CodeStore`: otter in-mem over a persistent MDBX `TblCodeCache` backing (decompressed code keyed by keccak). Read-through in `stateObject.Code`, 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 freelru-everywhere for this tier. ## Performance (100M, serial commitment) **Baseline note:** `baseline` is **main *before* the cache effort began** — `a8eeb459` (Jun 12, pre-erigontech#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 the `trunk-vs-others-100M` snapshot. `rank` is of 6 clients; `gap to 1st/2nd` = fastest/2nd-fastest peer ÷ current. | cell | baseline | current | reth | geth | besu | nethermind | ethrex | improvement | rank pre | rank now | rank Δ | gap to 1st | gap to 2nd | |---|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:| | sload-bloated-slots-set | 374 | 392 | 1033 | 59 | 88 | 92 | 96 | 1.05x | 2/6 | 2/6 | 0 | 2.63x | 0.24x | | sstore-bloated-slots-set | 395 | 365 | 1028 | 61 | 69 | 100 | 93 | 0.92x | 2/6 | 2/6 | 0 | 2.82x | 0.27x | | sload-bloated-no-slots | 338 | 369 | 978 | 58 | 176 | 497 | 343 | 1.09x | 4/6 | 3/6 | +1 | 2.65x | 1.35x | | sstore-bloated-no-slots | 350 | 374 | 1015 | 60 | 111 | 503 | 297 | 1.07x | 3/6 | 3/6 | 0 | 2.71x | 1.34x | | extcodecopy-contract | 1515 | 1471 | 4055 | 602 | 147 | 276 | 1917 | 0.97x | 3/6 | 3/6 | 0 | 2.76x | 1.30x | | extcodecopy-missing | 2083 | 2041 | 6339 | 585 | 295 | 403 | 4902 | 0.98x | 3/6 | 3/6 | 0 | 3.11x | 2.40x | | balance-eoa | 281 | 298 | 1169 | 135 | 88 | 465 | 139 | 1.06x | 3/6 | 3/6 | 0 | 3.93x | 1.56x | | callcode-eoa | 259 | 280 | 1196 | 135 | 80 | 467 | 138 | 1.08x | 3/6 | 3/6 | 0 | 4.27x | 1.67x | | staticcall-eoa | 265 | 277 | 1216 | 132 | 77 | 469 | 138 | 1.04x | 3/6 | 3/6 | 0 | 4.39x | 1.69x | | delegatecall-eoa | 269 | 281 | 1247 | 135 | 80 | 416 | 138 | 1.05x | 3/6 | 3/6 | 0 | 4.44x | 1.48x | | call-eoa | 243 | 256 | 1148 | 133 | 78 | 442 | 138 | 1.05x | 3/6 | 3/6 | 0 | 4.49x | 1.73x | | sload-same-key-preset | 1266 | 1786 | 8080 | 1610 | 422 | 449 | 2663 | 1.41x | 4/6 | 3/6 | +1 | 4.52x | 1.49x | | sload-same-key-no-preset | 1429 | 1538 | 8122 | 1438 | 362 | 441 | 2560 | 1.08x | 4/6 | 3/6 | +1 | 5.28x | 1.66x | | extcodehash-missing | 351 | 376 | 1341 | 79 | 101 | 500 | 931 | 1.07x | 4/6 | 4/6 | 0 | 3.57x | 2.48x | | extcodesize-missing | 338 | 368 | 1387 | 79 | 96 | 513 | 987 | 1.09x | 4/6 | 4/6 | 0 | 3.77x | 2.68x | | warm-callcode | 410 | 394 | 1520 | 534 | 92 | 330 | 781 | 0.96x | 4/6 | 4/6 | 0 | 3.86x | 1.98x | | callcode-missing | 304 | 332 | 1310 | 77 | 93 | 501 | 940 | 1.09x | 4/6 | 4/6 | 0 | 3.94x | 2.83x | | balance-missing | 334 | 352 | 1390 | 80 | 100 | 547 | 968 | 1.05x | 4/6 | 4/6 | 0 | 3.95x | 2.75x | | call-missing | 289 | 318 | 1318 | 79 | 85 | 456 | 879 | 1.10x | 4/6 | 4/6 | 0 | 4.15x | 2.77x | | delegatecall-missing | 304 | 320 | 1368 | 78 | 94 | 457 | 952 | 1.05x | 4/6 | 4/6 | 0 | 4.27x | 2.97x | | warm-delegatecall | 392 | 361 | 1650 | 611 | 95 | 201 | 887 | 0.92x | 4/6 | 4/6 | 0 | 4.57x | 2.46x | | staticcall-missing | 265 | 262 | 1222 | 77 | 87 | 492 | 948 | 0.99x | 4/6 | 4/6 | 0 | 4.67x | 3.62x | | warm-balance | 1639 | 1587 | 7468 | 2484 | 291 | 528 | 4171 | 0.97x | 4/6 | 4/6 | 0 | 4.70x | 2.63x | | warm-extcodesize | 1515 | 1471 | 8199 | 2111 | 290 | 400 | 5073 | 0.97x | 4/6 | 4/6 | 0 | 5.58x | 3.45x | | warm-staticcall | 216 | 217 | 1696 | 509 | 103 | 187 | 790 | 1.01x | 4/6 | 4/6 | 0 | 7.80x | 3.63x | | warm-extcodehash | 333 | 334 | 6691 | 1816 | 296 | 271 | 4649 | 1.00x | 4/6 | 4/6 | 0 | 20.01x | 13.90x | | extcodehash-contract | 52 | 79 | 121 | 83 | 71 | 119 | 153 | 1.52x | 6/6 | 5/6 | +1 | 1.94x | 1.54x | | callcode-contract | 42 | 78 | 123 | 84 | 66 | 104 | 155 | 1.87x | 6/6 | 5/6 | +1 | 1.98x | 1.57x | | staticcall-contract | 42 | 78 | 122 | 84 | 68 | 118 | 156 | 1.86x | 6/6 | 5/6 | +1 | 2.01x | 1.57x | | extcodesize-contract | 42 | 77 | 122 | 84 | 72 | 125 | 154 | 1.85x | 6/6 | 5/6 | +1 | 2.01x | 1.63x | | balance-contract | 51 | 76 | 121 | 83 | 74 | 129 | 154 | 1.51x | 6/6 | 5/6 | +1 | 2.02x | 1.69x | | call-contract | 42 | 76 | 121 | 83 | 63 | 117 | 154 | 1.82x | 6/6 | 5/6 | +1 | 2.02x | 1.59x | | delegatecall-contract | 42 | 76 | 111 | 85 | 66 | 109 | 155 | 1.83x | 6/6 | 5/6 | +1 | 2.03x | 1.45x | | warm-call | 226 | 228 | 1535 | 676 | 87 | 243 | 857 | 1.01x | 5/6 | 5/6 | 0 | 6.74x | 3.76x | ### What improved - **Contract account/code access** (CALL/CALLCODE/DELEGATECALL/STATICCALL/EXTCODESIZE/ 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. - **sload-same-key** (preset 1.41×, no-preset 1.08×) and **sload-bloated-no-slots** moved up a rank. - **geomean 1.156×** across all 34 cells vs the pre-perf baseline. - Honest notes: the already-warm repeated-access cells and one storage-write cell regressed 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 - hive `eest-devnet` (amsterdam BAL, parallel exec, serial commitment): **2572/0**. - `--experimental.parallel-commitment` fails 39 of these tests, but that's **the known, tracked parallel-commitment gap (erigontech#21137)**, not this change — proven by isolation: caches on vs off under parallel-commitment give the identical 39, and serial is 0. Deferred to erigontech#21137. ## erigontech#22120 Addressed findings 1, 2, 3, 8. Remaining (4/5/6/7 + enforcement, erigontech#22116) tracked as follow-up. ## Follow-ups (ordered) 1. Land **erigontech#21536** (typed-vio refactor) — the dependency for the exec-path fix below. 2. **Remove the GetCodeHash over-refresh** (comes after erigontech#21536): `GetCodeHash` reads `CodeHashPath` 2–3× through the `GetCodeHash → versionedRead → getStateObject` nesting, plus a full-account `refreshVersionedAccount` and a per-read `SelfDestructPath` probe. Cut this to a watermark-gated, per-field refresh — the lever for the **warm-extcodehash** outlier and the 5/6 cells (contract first-touch + warm-call). 3. Re-run *with* parallel-commitment once **erigontech#21137 / erigontech#22113** land consensus-clean. Note: the adaptive trunk-pin controller is currently wired via an in-flight-tx `OnBlockComplete` hook; a follow-up either moves it to proper post-commit placement or shelves it.
Sahil-4555
pushed a commit
to Sahil-4555/erigon
that referenced
this pull request
Aug 12, 2026
…rigontech#23185) Three unreferenced things in the commitment package, found while auditing the parallel path. ## Changes - `cell.GetAccountAddr` / `cell.GetStorageAddr` — no reference anywhere, including tests. - `CompactKey` — no reference anywhere, including tests. - `collectDeleteUpdate`'s `evictCache` parameter — erigontech#21380 removed the `hph.cache.EvictBranch` call it gated when the trie stopped owning a cache, leaving the parameter unread and its docstring describing eviction that no longer happens. All three call sites passed `true`. `EvictBranch` no longer exists in the tree, so the invalidation moved rather than being lost. --------- Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com>
This was referenced Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is PR #1 of a 3-PR perf stack. It consolidates state caching across all modes — sync, tip-tracking, integration and testing — so the state cache is either on and completely trustworthy, or off to measure — never off because it unexpectedly breaks things.
The caches (the account/storage
StateCacheand the commitmentBranchCache) are an internal implementation detail ofSharedDomains. No external entity accesses or mutates them directly: callers drive state throughFlush/Commit/GetLatest/DomainPut, and the full cache lifecycle (population, invalidation, commit-gating) is owned insideSharedDomains.What this PR contains
BranchCache— single aggregator-scope commitment cachesd.memchain so unwinds and fork-validations see consistent state.txNum;sd.Unwindevicts everything above the unwind watermark (BranchCache.UnwindTo).One switch for all caches
The
BranchCacheis a type of state cache, so it rides the existingUSE_STATE_CACHEtoggle rather than getting its own env. One operator switch turns all caching off — the relevant operation when bisecting a state-root mismatch, where an operator shouldn't have to reason about the interaction of several independent caches. (This is a deliberate deviation from the review's "add a separateBranchCachekill-switch" suggestion — flagged for confirmation.)The BranchCache reflects only committed state — by construction
The BranchCache can never hold a value a failed commit rolled back:
SharedDomains.Commitflushes the in-memory batch into the tx, commits, and only then applies the flushed commitment branches to the cache (the flush is implicit in committing; a failed commit applies nothing). PlainFlush— callers that own their own commit, e.g. offline tools — never touches the cache; read-through populates from committed files. The caches are an internal detail ofSharedDomains; nothing else writes to them.BUG #21138 — parallel-exec from-0 wrong trie root
ResetExecwipes the commitment DB table; the aggregator's in-memoryBranchCachecould still reference the just-deleted trie nodes, so a from-0 re-exec served stale entries when computing block 0's commitment → wrong root, dropping genesis-allocated balances no later block touched (mainnet block 46147,0xA1E4380A3B1f749673E270229993eE55F35663b4). Fix:ResetExecclears the aggregator'sBranchCache.TestFromZero_GenesisAllocPreservedAfterResetReExecpasses on currentmain; the test's value here is keeping this PR's cache safe across reset, not fixing a livemainbug.Follow-ups (the rest of the stack)
txNum/epochmodel.mh/branch-cache-trunk-pin, to be re-benchmarked before merge.GetLatestvariants into a single metered,txNum-returningGetLatest.Testing
Behaves identically across parallel and serial exec — confirmed in CI across both exec modes. Unit coverage for the BranchCache (tiers, tx-precise
UnwindTo, commit-gated population) plus the engine/exec-module FCU commit paths.Given today's changes (the commit-gating of both caches), we will do another A/B performance run before merging.