Skip to content

execution/state: cut the header walk and the per-slot probes in WriteSet.Normalize - #23027

Closed
AskAlexSharov wants to merge 8 commits into
mainfrom
alex/normalize_walk_37
Closed

AskAlexSharov wants to merge 8 commits into
mainfrom
alex/normalize_walk_37

Conversation

@AskAlexSharov

@AskAlexSharov AskAlexSharov commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Normalize runs once per tx result inside blockExecutor.nextResult — the single-threaded apply loop that is parallel execution's serialization point — so its cost is never spread across workers. On a mainnet profile it is 32% of nextResult.

Two optimizations:

1. Walk the per-path maps instead of AllHeaders. AllHeaders() yields a 56-byte WriteHeader per write and erases the typed value, so every arm of the switch h.Path looked the write back up by address (GetStorage, GetBalance, …) — a second probe of the entry just visited. Ranging the maps hands each branch its write directly. WriteSet.Apply already does this, with the reasoning at rw_v3.go:120.

2. Resolve the storage no-op filter's self-destruct probes once per address. ReadSelfDestruct and AnyDoneSelfDestructEquals key on the address alone but ran per slot; ranging storage per address resolves them once per slot group, lazily.

The self-destruct drop rule moves into dropForSelfDestruct(path, eip8246) — one exhaustive switch, so a new path must state its answer, instead of the predicate being repeated across loops. Measured free (+0.07%, p>=0.21).

Numbers

BenchmarkWriteSetNormalize vs main, 6 interleaved rounds of -count=3, n=18:

                         │     main    │              this PR                 │
                         │    sec/op   │   sec/op     vs base                 │
addrs=3/slots=1-16         3.251µ ± 3%   2.889µ ± 7%  -11.11% (p=0.000 n=18)
addrs=4/slots=2-16         4.317µ ± 2%   3.936µ ± 8%   -8.83% (p=0.002 n=18)
addrs=16/slots=8-16        23.20µ ± 3%   19.41µ ± 3%  -16.33% (p=0.000 n=18)
addrs=30/slots=10-16       53.08µ ± 2%   44.60µ ± 5%  -15.98% (p=0.000 n=18)
geomean                    11.47µ        9.961µ       -13.12%

An earlier EPYC run put the same comparison at -15.52% geomean. B/op and allocs/op unchanged — a copy-and-dispatch win, not an allocation one.

A storage-only variant (move just the StoragePath arm, leave the rest of the switch untouched) was built and measured as the smaller-diff alternative: it regresses +3.3% / +3.6% at the two narrow shapes, because storage then gets walked twice — once through AllHeaders into an empty arm, once in the grouped loop. Geomean -4.01%. Rejected.

The two narrow benchmark shapes are added here; the pre-existing ones were both wider than mainnet, where a tx writes ~3 addresses with ~1 storage slot (debug_traceBlockByNumber + prestateTracer, 2019-era replay and today's tip agree).

Review

Each old switch arm becomes one loop over the matching map, with its filter unchanged. Three things worth checking:

  • The sdSet matrix is preserved, now stated once in dropForSelfDestruct — including the paths that deliberately do not drop (CreateContract, SelfDestruct, CodeSize, Address), which previously said so only by being absent from the switch.
  • Reordering is safe. Every arm writes to a distinct per-path map keyed by address. The one cross-path emission — the self-destruct storage cascade — fires exactly for addresses in sdSet, whose raw storage writes are dropped, so the two can never collide.
  • Header fields, not map keys. eachWriteHeaderOf yields vw.WriteHeader, so the old code addressed writes by h.Address/h.Key; the new loops do the same.

Author comments are preserved verbatim, including the AddressPath and CodeSizePath markers for the two paths that produce no output. Filter logic, fill loop, EIP-7702 recovery, EIP-161 removal and all error paths are unchanged.

Green: execution/state, execution/stagedsync, execution/tests, including TestSelfDestructReceive, TestEIP161AccountRemoval, TestCVE2020_26265, TestDeleteRecreateAccount, TestDeleteRecreateSlots, TestDeleteRecreateSlotsAcrossManyBlocks.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Optimizes the hot WriteSet.Normalize path in execution/state by avoiding AllHeaders() iteration overhead and eliminating an extra pass for collecting field-level dirty addresses, reducing per-tx normalization CPU time in the parallel-exec apply loop.

Changes:

  • Reworks WriteSet.Normalize to range the per-path typed maps directly (dropping per-write WriteHeader copies and redundant Get* lookups) while collecting allAddresses during the same walk.
  • Preserves existing filtering semantics (self-destruct handling, incarnation filtering, and storage no-op filtering) while changing iteration strategy.
  • Extends BenchmarkWriteSetNormalize with a {3,1} shape to better reflect common mainnet write-set geometry.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
execution/state/writeset_normalize.go Replaces AllHeaders() dispatch with direct per-path map loops; collects allAddresses during the walk to avoid a second pass.
execution/state/writeset_normalize_bench_test.go Adds {3,1} benchmark case to represent typical mainnet write-set shape.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@AskAlexSharov
AskAlexSharov force-pushed the alex/normalize_walk_37 branch from 8ea8154 to 96b887c Compare August 5, 2026 14:20
@AskAlexSharov AskAlexSharov changed the title execution/state: walk the per-path maps directly in WriteSet.Normalize execution/state: resolve the storage no-op filter's self-destruct probes once per address Aug 5, 2026
@AskAlexSharov
AskAlexSharov changed the base branch from main to alex/normalize_map_walk_37 August 5, 2026 14:20
@AskAlexSharov
AskAlexSharov force-pushed the alex/normalize_map_walk_37 branch from dbaee57 to 392abc2 Compare August 5, 2026 14:31
@AskAlexSharov
AskAlexSharov force-pushed the alex/normalize_walk_37 branch from 96b887c to 5073683 Compare August 5, 2026 14:32
@AskAlexSharov
AskAlexSharov marked this pull request as ready for review August 5, 2026 15:04
@AskAlexSharov
AskAlexSharov force-pushed the alex/normalize_walk_37 branch from 5073683 to 3d6da4c Compare August 5, 2026 15:14
@AskAlexSharov
AskAlexSharov force-pushed the alex/normalize_map_walk_37 branch from 392abc2 to fbe4122 Compare August 5, 2026 15:14
@AskAlexSharov AskAlexSharov changed the title execution/state: resolve the storage no-op filter's self-destruct probes once per address execution/state: cut the header walk and the per-slot probes in WriteSet.Normalize Aug 5, 2026
@AskAlexSharov
AskAlexSharov changed the base branch from alex/normalize_map_walk_37 to main August 5, 2026 15:19
@AskAlexSharov
AskAlexSharov marked this pull request as draft August 5, 2026 15:21
@AskAlexSharov

Copy link
Copy Markdown
Collaborator Author

closing in favor of future Marks PR on the same field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants