[experiment] execution/stagedsync: let Normalize read through the tx's own read set - #23050
[experiment] execution/stagedsync: let Normalize read through the tx's own read set#23050AskAlexSharov wants to merge 19 commits into
Conversation
Status: experiment, not for mergeGoal is a pure Where it standsThe substitution is correct.
That covers the self-destruct / EIP-161 / CREATE2 family, which is where this could plausibly go wrong. But the current lookup barely firesMeasured read-set hit rate for
Structural, and the code says why:
The fill loop wants a whole account record and asks Remaining DB reads to eliminate
Notes for anyone reading the historyThree failures on this branch were the instrument, not the hypothesis:
|
Dig 2: per-path resolution — correct, and worth exactly nothingFollowed the obvious lead from the last comment: the read set stores reads per path, while the fill loop asked It is correct. With It removes no DB reads at all:
17,795 fields served from memory, and not one domain read avoided. WhyThe domain read is hoisted per address (
All-four-present is zero in 35,516 samples. Partial coverage cannot beat a per-address hoist, so this direction is capped at zero by construction. What that leavesA pure fill loop needs the whole account record, not more fields. The worker already loads it —
So the next question is not "how do we squeeze more out of the read set" but "can an AddressPath read be recorded for accounts the worker fully loaded, without breaking the The per-path resolver is kept on the branch: it is correct, it is free when it hits, and it becomes load-bearing the moment the per-address hoist is reconsidered. But on its own it is a null result, recorded so nobody re-runs it. |
…x/normalize_readset_assert_37 # Conflicts: # cmd/prometheus/dashboards/erigon_internals.json # execution/stagedsync/exec3_metrics.go
… as absent The fill loop's account fallback went to the domain even when the tx's own read set already held an AddressPath entry saying the address has no account. readAccountInternal records that entry header-only (Val=nil) before the load, and accountRead overwrites it with the account as soon as a load finds one, so a header-only entry surviving to Normalize means the load came back empty. Measured over rpc/jsonrpc and execution/tests: the implication "read set says absent -> the read returns nil" held 10618/10618 times, and cutting the read takes apply-loop domain account reads through the versioned reader from 18961 to 8926 in rpc/jsonrpc (-52.9%). normalize_probe.go is the instrument that produced those numbers, off by default behind NORMALIZE_PROBE; NORMALIZE_SKIP_ABSENT restores the old behaviour for A/B. Both are experiment-branch scaffolding, like checked_reader.go.
Dig 3: the read set already knows, and the answer is "nothing there"The last comment asked whether an AddressPath read could be recorded for accounts the worker fully loaded. Wrong question — it already is. So the low hit rate had to come from somewhere else. Classifying every fill-loop account fallback by what the tx's read set held for that address:
The dominant bucket is the header-only entry, and it is not a gap in the recording — it is an answer. The implication holds on every sample. Counting what the read returned for that bucket:
10618 for 10618, including the self-destruct / EIP-161 / CREATE2 family in ChangeOne condition in ResultApply-loop domain account reads through the versioned reader,
−52.9%. Unlike dig 2, this one is not capped at zero by construction: the per-address hoist that made per-path resolution worthless is exactly what makes this work, because one skipped read covers all four missing fields. Green with What is not claimed
|
…reads Extends the probe to the storage no-op filter and to the call site behind each domain account read. Storage is already served from the read set: 6795 of 6795 versioned no-op-filter fallbacks find the exact slot recorded, and only 48 of 7789 reach the domain. The 8925 residual account reads come from 3 sites and touch 22 distinct addresses: calcFees 6264/4, the Normalize fill loop 1420/3, finalizeSystemTx 1242/15. Repetition, not coldness.
Dig 4: the remaining reads are not cold, they are the same 22 addressesTwo questions settled, and the second one moves the target. Storage was already solvedItem 2 on the remaining-reads list ( Measured on the no-op filter's fallback arm,
The worker's pre- Items 1 and 3 are also closed. Where the remaining reads actually areAttributing every domain account read through the versioned reader to its call site,
22 addresses, 8925 reads. The apply loop is not reading cold state — it is re-reading the coinbase, the burnt-fee contract and a handful of system contracts once per transaction. That closes the read-set line of attack for these. I wired
What this changes"Give the apply loop the worker's reads" is finished as a direction. Dig 3 took the fill loop's account reads from 18961 to 8926; the rest is a different problem with a different fix — the base account for these 22 addresses is a pre-block value, and The open question is invalidation: whether the domain answer for those addresses can change during a block's apply loop, given that applied writes reach
|
…mitted account CachedReaderV3 with readCurrent=true asked GetCurrentAccount for a blob. On a miss in the write buffer that helper falls back to committedAccounts, which holds decoded accounts, and re-encodes one so the reader can decode it back. Split the write-buffer lookup out of GetCurrentAccount and take the committed account decoded, as the readCurrent=false branch already does. BenchmarkCachedReaderAccountRead/committed: 65.5 -> 24.3 ns/op, 144 -> 96 B/op, 2 -> 1 allocs. The path is taken 7933 times over the rpc/jsonrpc suite.
Dig 5: chasing the last reads found the cost was never the readDig 4 ended on a question — can the base account for those 22 addresses be read once per block instead of once per transaction? The answer is no, twice over:
So the per-transaction read is structural. What is not structural is what a hit costs.
Measured over
The re-encode is the common one, 4.5x over the path the blob format exists for. The apply loop mostly reads addresses no transaction has written yet this block.
Split out as #23075, off main, since it owes nothing to this branch's wiring. One thing worth recording for anyone touching that function: the lookup is open-coded rather than extracted into a helper because a helper does not inline — the Where the read-set thesis standsFinished, and successfully. Dig 3 took the fill loop's account reads from 18961 to 8926. Dig 4 showed storage was already covered (6795/6795) and that
|
…count (erigontech#23074) When the read set holds no account for an address, `versionedStateReader.ReadAccountData` went to the domain — including when the read set held an AddressPath entry that says there is no account. That entry is an answer, not a gap: `readAccountInternal` records it header-only before the load, and `accountRead` overwrites it with the account as soon as a load finds one, so a header-only entry means the load came back empty. Implication "read set records the address -> the read returns nil", over `rpc/jsonrpc` + `execution/tests`: **10618/10618**, including the self-destruct / EIP-161 / CREATE2 family. **Worth nothing on main today**, measured: apply-loop domain account reads through the versioned reader are 7506 with and without, whole `rpc/jsonrpc` suite. `Normalize` still gets the plain domain reader, so only the finalize IBS uses this path and it never asks for a recorded-absent address. Once `Normalize` takes the read-set reader (erigontech#23050), the same condition takes those reads 18961 -> 8926 (**-52.9%**). Divergence not seen in 10618 samples: an account self-destructed by an earlier tx and revived by a balance-only credit writes no AddressPath cell, so the old path returned the pre-SD account where this one falls through to BAL synthesis. Green: `execution/state`, `execution/stagedsync`, `execution/tests`, `rpc/jsonrpc`.
Experiment on top of #23027. The question:
Normalizeruns on the apply loop and reads state the worker already read. Can it take the tx's own read set instead of going back to the domain?Why it should be possible
The worker cannot credit an account without loading it, and
stateObject.SetStateperforms a versioned read of the previous value before everySSTORE(EIP-2200 gas depends on it). Those reads are recorded per-tx asVersionedRead{ReadHeader, Val}. The write set deliberately does not carry the base —VersionedWriteisWriteHeader + Val— so the base survives only in the read set, whichNormalizewas never given.versionedStateReaderalready layers read set -> versionMap -> domain and is handed to the finalize IBS two functions away. This branch hands it toNormalizetoo. That is the whole change:Result
Green locally:
execution/state,execution/stagedsync,execution/tests, andgo test -count=2 ./rpc/jsonrpc.Getting there took one wrong turn worth recording, because it is a trap for anyone repeating this. The branch originally wired both
Normalizecall sites through aCheckedStateReaderthat answered from the read set and read the domain on every call, panicking on disagreement. That failed widely — 6 rpc/jsonrpc tests, EEST spec tests in CI — while never firing the assert. Three configurations separate the cause:go test -count=2 ./rpc/jsonrpcThe probe was the problem. The domain reader here is
NewCurrentCachedReaderV3(..., blockStateCache), and reading through it fills the block state cache; the extra fills perturb it. #22444 bound StateCache fills to transaction views and made stale fills rejectable, so the cache is deliberately sensitive to when fills happen. Unproven as a mechanism, but it is the only hypothesis consistent with all three rows, and it explains why no assert ever fired: there was no read disagreement to find.checked_reader.gois kept in the branch as documentation of the probe, deliberately not wired in. Anything that double-reads through the cached reader cannot be trusted as a measurement here.What is not claimed
Normalize's domain reads were 19% (ReadAccountData) and 13% (ReadAccountCode) of its time in the n5 profile, but this branch has not been benchmarked.TestSelfDestructReceive:ReadAccountStoragereturning{0, found:true}from the read set where the domain says{0, found:false}. The no-op filter is provably insensitive to it — its two arms agree whenever the value is zero — but it is the same presence-vs-value class that execution/state: serve warm storage reads from the read set #22993 found load-bearing one layer down, and it is the thing to watch in wider testing.domainStorageKeys->IteratePrefixis bound toapplyTxandSharedDomains.mem, cannot come from a worker, and is what produced a 513msNormalizeon a 26-self-destruct tx.