Repository navigation
perf(evmonly): parallelize OCC validation and merge behind the serial acceptance barrier - #4457
Conversation
… acceptance barrier Port of #4261 and the #4273 follow-up from giga-1 onto main. Acceptance stays in block order on the calling goroutine. The executor splits the write index and the accepted prefix into 64 address shards. The OCC pool indexes every speculative result up front. A parallel pass accepts the longest run that the serial frontier would accept. The frontier then handles only the first result that needs a rerun or fails. The merge builds each shard's changeset on the pool and joins the shards in address order. A per-shard base-row cache in the merge replaces prefetchBaseAccounts. The port keeps main's renames (baseAccount, baseAccountReader, phase constants). It needs no change from #4258 or #4260. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tial executor Seeded dense and sparse blocks mix hot recipients, read-modify-write slots, hot-balance reads, nonce chains, mid-block funding, contract creation, a create-and-destroy child, reverts, logs, and rejected nonces. OCC results at 3 and 8 workers must equal the sequential executor's results, including the encoded FlatKV pairs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@seidroid review |
PR SummaryHigh Risk Overview Validation alternates parallel prefix passes ( Merge drops Docs add expected speedups/regressions by workload shape. Tests/benchmarks add shard and backoff coverage, a large mixed OCC-vs-sequential test, seeded OCC equivalence against sequential execution (including FlatKV encoding), validate/merge benchmarks, and an ERC20 load-test scenario. Reviewed by Cursor Bugbot for commit 043d815. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4457 +/- ##
==========================================
+ Coverage 56.77% 56.80% +0.03%
==========================================
Files 2126 2127 +1
Lines 166876 167018 +142
==========================================
+ Hits 94737 94871 +134
- Misses 72134 72142 +8
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Ports the parallel OCC validation and merge to main: a sharded write index filled up front, a parallel pass that accepts runs ahead of the serial frontier, sharded prefix apply, and a per-shard parallel changeset merge. Approving: the full index only ever adds conflicts (the accepted incarnation's writes are always indexed), stmFrontierAccepts matches needsSTMRerun, touchedShards covers every address applyOwned and indexResults touch, and shard concatenation keeps canonical address order. Reviewed against the PR merge tree but not built (no Go toolchain in the sandbox); the codex scout returned no reading.
1 nit, not posted on the code
giga/evmonly/occ_shards.go:272— The fragments go back to occFragmentPool still holding the last block's *big.Int balances and cloned code slices. resetForReuse only runs on the next Get, so that data stays reachable until the next parallel merge. Resetting each fragment before Put would release it right away.
seidroid review · decision approve · session 40d2fa1050864bf9b6ac911728d0533d · turn resp_claude_64ad0acff67424e7ec6a8ae3f513d8c7 · item bdcfe70d0c6f5028a0d050546a331766
Findings: 0 blocking | 0 non-blocking | 0 posted inline
…pool The pool now holds only cleared fragments, so the last block's balances and code slices stop being reachable as soon as the merge ends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@seidroid review |
There was a problem hiding this comment.
Since the last review, releaseOCCFragments clears each fragment before it goes back to occFragmentPool, which fixes my earlier nit about the pool keeping balances and code reachable; nothing else changed, so the earlier correctness analysis still holds and this is approved. I kept codex's finding about the gas prefix being rebuilt on every validation pass, downgraded to a suggestion because the cost is small next to EVM execution at current block sizes; this review is based on the current merge tree, not a build.
Non-blocking
1 finding on the changed lines, as inline comments.
seidroid review · decision approve · session 40d2fa1050864bf9b6ac911728d0533d · turn resp_claude_3543caff4c3abc1f1e350032a5b069cf · item b846d0d9a9915c06bd7749cf795454df
Findings: 0 blocking | 1 non-blocking | 1 posted inline
A parallel pass computed the cumulative gas of every result to the end of the block and allocated a new buffer for it. When conflicts are just over 64 results apart, each pass still accepts enough to reset the serial backoff. This serial work then grows with the square of the block size. A pass now looks ahead at most twice as far as the previous pass accepted, and at least occMinPassLookahead (2048) results. The cumulative-gas buffer is one per block. The accepted runs and every frontier decision stay the same. BenchmarkOCCValidateAndMerge adds a 20,000-transaction block with a conflict every 65 results. The equivalence test adds two long profiles that cross the look-ahead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@seidroid review |
wen-coding
left a comment
There was a problem hiding this comment.
Approving since this is a backport, some questions inside.
| // occStateShards is the number of address shards the accepted prefix and the write index are split | ||
| // into. Shards are contiguous address ranges, so concatenating per-shard output in shard order keeps | ||
| // it in canonical address order. | ||
| const occStateShards = 64 |
There was a problem hiding this comment.
how is this constant decided? Same for constants below, how were they decided, maybe put those in comments?
There was a problem hiding this comment.
256 and 64 copy the package's existing pool thresholds (minPrefetchedAccounts, occParallelReceiptThreshold), 64 shards fits the uint64 shard bitmask, and 2048 sizes one pass to a testnet block. Agreed the comments should say so.
There was a problem hiding this comment.
Done in 043d815: each constant now says where its value comes from (design choice, shared pool threshold, or sized for testnet blocks) and that none of them is measured for this path.
|
|
||
| // occShardOf returns the shard that holds addr. | ||
| func occShardOf(addr common.Address) int { | ||
| return int(addr[0]) * occStateShards / 256 |
There was a problem hiding this comment.
How do we guarantee the loads are evenly distributed among shards?
There was a problem hiding this comment.
We don't guarantee it: addr[0] is uniform for hash-derived addresses, but a hot contract or leading-zero vanity addresses land in one shard, which costs only speed (it degrades toward the old serial merge); sub-sharding storage by slot range, flagged back on #4261, is the fix.
There was a problem hiding this comment.
Done in 043d815: occShardOf now names the skew (hot contract, leading-zero addresses) and that it costs parallelism, not correctness. Balancing it, by slot-range sub-sharding or per-block ranges, is tracked in PLT-1379.
| between that incarnation's source prefix and its own index. | ||
|
|
||
| Acceptance is the serial barrier, but the work behind it is spread across the | ||
| pool: every incarnation's writes are indexed by transaction index up front, a |
There was a problem hiding this comment.
I guess this will work very well on the transfer benchmark, not sure it works well for all workloads. Should we add pros and cons here?
There was a problem hiding this comment.
Agreed: the gain is largest on transfer-like blocks (validate+merge −20% conflict-free, −11% sparse) and flat to slightly worse under contention (+3.8% dense, +1.7% hot recipient), so a pros/cons note with those numbers belongs here.
There was a problem hiding this comment.
Done in 043d815: the Block-STM section now says when parallel acceptance helps and when it costs a few percent, with the local benchmark range.
… sharding helps Each OCC shard constant now says whether it is a design choice, a threshold shared with the package's other pool fan-outs, or sized for testnet blocks. occShardOf names the skew a hot contract or leading-zero addresses cause, and the README gives the workloads where parallel acceptance helps or costs a few percent, with the local benchmark range. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@seidroid review |
There was a problem hiding this comment.
Since the last review, each parallel validation pass now looks ahead at most max(2048, 2 × the previous pass's accepted count) and reuses one cumulative-gas buffer per block, so a block's passes do linear work; that addresses the suggestion I raised from codex, already marked resolved, and accept/reject decisions are unchanged. The fragment-pool fix still holds and nothing in the update breaks correctness, so this is approved; codex found nothing this round and only a nit remains, in the new benchmark.
1 nit, not posted on the code
giga/evmonly/occ_bench_test.go:71— With GOMAXPROCS=1, OCCWorkers is 1, so NewExecutor never creates occPool. runRanges then calls Run on a nil pool and the benchmark panics. Clamping workers to at least 2, or skipping when the pool is nil, avoids that.
seidroid review · decision approve · session 40d2fa1050864bf9b6ac911728d0533d · turn resp_claude_5bd475cc8fd4b55b6d28452cf0ff83f3 · item e504d5e785a25288be4a182c5ee7bfd0
Findings: 0 blocking | 0 non-blocking | 0 posted inline
|
@seidroid review |
There was a problem hiding this comment.
Nothing changed since the last review: same merge commit (145a203), diff, description and review standards, and the look-ahead cap and reused gas buffer behind the resolved occ_shards.go:159 thread are still in place. Approved as before, with only the GOMAXPROCS=1 benchmark nit open; codex found nothing this round, so there was nothing from it to keep or drop.
1 nit, not posted on the code
giga/evmonly/occ_bench_test.go:71— With GOMAXPROCS=1, OCCWorkers is 1, so NewExecutor never creates occPool, and runRanges calls Run on a nil pool, so the benchmark panics. Clamping workers to at least 2, or skipping when the pool is nil, avoids that.
seidroid review · decision approve · session 40d2fa1050864bf9b6ac911728d0533d · turn resp_claude_9e5bffc3f586defc744ac94a9c272279 · item 87705234d5a853ef9c2cfc24e6ab9ded
Findings: 0 blocking | 0 non-blocking | 0 posted inline
Backport: this ports #4273, the reviewable version of #4261, to main from the giga-1 PR stack #4270–#4273. The stack targeted stacked giga-1 branches and never reached main; only its first PR landed on main, as #4366. @shemnon approved #4273 at 1143167. Since that commit, the changes are main's renames (
baseAccount, thephaseOCC*constants,takeAccessSets) and two review fixes: merge fragments are cleared before they return to the pool, and each parallel pass's look-ahead is bounded.On giga-testnet-2,
occ_validateandocc_mergetook about a third of the executor main loop, all of it on the calling goroutine. Most of that time went to transactions that the frontier accepts as they stand. A block of about 1,800 transactions holds less than one conflict. Ongiga-1, #4261 moved this work onto the OCC pool. It cut validate and merge from about 8.5 ms to 4.1 ms per block. That gave about 20% more blocks per second during the 95k tx/s window. #4269 reverted it for the reviewable stack, and that stack (#4273) never reached main. This PR ports #4261 and the #4273 follow-up to main (PLT-1377).Acceptance stays a serial barrier in block order.
stateAccessIndexandblockSTMStatesplit into 64 address shards.indexResultsrecords the writes of every speculative result up front, andconflictsWithin(key, sourcePrefix, txIndex)replacesconflictsWithAfter.acceptValidatedPrefixfinds the first result that the serial frontier would not accept, andapplyRangefolds the run before it into the prefix shard by shard.validateBlockSTMFrontierthen handles only that one result, andserialBackoffkeeps dependency chains on the calling goroutine. Each pass looks ahead at most twice as far as the previous pass accepted, and at least 2,048 results, so the work of a block's passes stays linear in its size.changeSetIntoParallelbuilds the changeset of each shard on the pool and joins the shards in address order. Its per-shard base-row cache replacesprefetchBaseAccounts. The port needs nothing from #4258 or #4260; the only conflicts came from thebaseAccountand phase-constant renames on main.Non-app-hash-breaking: changesets, receipts, and tx results stay byte-identical to main. Shards are contiguous ranges of the first address byte, so shard order is canonical address order. Each address lives in one shard, so the apply order in a shard is block order. The new index can only add conflicts, from stale writes of an earlier incarnation. An extra rerun executes against the exact accepted prefix, so it gives the sequential result, and the extra reruns show only in OCC metrics. Review
stmFrontierAcceptsmost closely, because it must mirrorneedsSTMRerun, andtouchedShards, because it must cover every address thatapplyOwnedandindexResultstouch.TestOCCRandomizedConflictingBlocksMatchSequentialchecks seeded dense and sparse blocks at 3 and 8 workers against the sequential executor, down to the encoded FlatKV pairs. A seeded digest run gave identical output on main and this branch for 864 blocks, including 3,000- and 6,000-transaction blocks that cross a pass's look-ahead.