Repository navigation
perf(evmonly): let parse workers claim their own ranges - #4234
Conversation
parseBlockTxs fed indices to its workers through an unbuffered channel, so a
block's 1,848 transactions cost 1,848 rendezvous on a stage the execute loop
waits for. A block carries a sender for nearly every transaction, so the work
behind each handoff is a decode rather than a signature recovery, and the
handoff cost more than the parse: the parallel path ran slower than the serial
one it was meant to beat.
Workers now claim ranges off an atomic counter. Measured on a 1,848-tx block
with a sender for every transaction, which is the shape the node path builds:
workers=1 768 us -> 808 us (serial path, unchanged)
workers=8 1,610 us -> 355 us 4.5x
workers=32 1,516 us -> 332 us 4.6x
Validators run ParseWorkers at GOMAXPROCS, so the 32-worker figure is the live
one: 1.25 ms off a 28.83 ms loop, or 679 ns per transaction.
Output is written by index, so order never depended on dispatch order.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR SummaryLow Risk Overview The serial path ( Reviewed by Cursor Bugbot for commit ba31dbb. 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 @@
## giga-1 #4234 +/- ##
========================================
Coverage 65.55% 65.55%
========================================
Files 2081 2078 -3
Lines 157460 157237 -223
========================================
- Hits 103222 103078 -144
+ Misses 54097 54018 -79
Partials 141 141
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.
Replacing the per-transaction job channel with atomic range claiming is correct — output stays index-addressed, cancellation is still checked before each claim is parsed, and the counter is bounded because each worker exits on its first out-of-range claim. The one gap is test coverage: at parseClaim = 16 the existing 8-transaction parallel test now runs entirely inside a single worker's first claim, so multi-claim and partial-tail behavior is unverified.
Findings: 0 blocking | 1 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 1 suggestion(s)/nit(s) flagged inline on specific lines.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…sei-protocol#4345) Three small hot-loop optimisations from the evmonly serial-budget survey landed on `giga-1` as sei-protocol#4234, sei-protocol#4236 and sei-protocol#4237 but were never brought to `main` (PLT-1302, PLT-1304, PLT-1305 under PLT-1297). All three cherry-pick cleanly; this PR carries them as-is, one commit each. `parseBlockTxs` fed indices to its workers over an unbuffered channel, so with a known sender on nearly every transaction the rendezvous cost more than the decode and the parallel path ran slower than the serial one; workers now claim ranges off an atomic counter (about 4.5x faster at 8-32 workers). `hashEVMOnlyResult` wrote the app-hash stream to sha256 a few bytes at a time with a fresh length prefix per write; it now goes through a 32 KiB buffer with a scratch prefix, cutting roughly 7,000 allocations per block. The FlatKV changeset encoder allocated a `KVPair`, a key and a value per emitted pair; it now cuts all three from one slab per kind, the way `EncodeMemoryStoreChangeSet` already does, cutting roughly 10,500 allocations per block. None of this changes bytes on the wire or in state: `TestHashEVMOnlyResultGoldenVector` and `TestEncodeFlatKVChangeSetGolden*` pin the hashed stream and the emitted pairs to values captured from the previous implementations, and `parsed[i]` was always written by index so parse order never mattered. Validated with `go test -race ./giga/evmonly/ ./sei-tendermint/internal/evmonlyapp/` on this branch. --------- Co-authored-by: FromTheRain <bdchatham@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
One of four independent PRs from the serial-budget survey (#4234, #4235, #4236, #4237); each targets
giga-1on its own.parseBlockTxsfed indices to its workers through an unbuffered channel, so a block's 1,848 transactions cost 1,848 rendezvous on a stage the execute loop waits for. A block carries a sender for nearly every transaction, so the work behind each handoff is a decode rather than a signature recovery — and the handoff cost more than the parse. The parallel path ran slower than the serial path it was meant to beat.Workers now claim ranges off an atomic counter.
Measured
Measured locally with a throwaway
BenchmarkParseBlockTxs(not included in this PR). 1,848 transactions each with a known sender, which is the shape the node path builds:Validators set
ParseWorkers: runtime.GOMAXPROCS(0)(sei-tendermint/internal/evmonlyapp/app.go:119), so on a 32-vCPU validator the 32-worker row is the live one: 1.25 ms off a 28.83 ms loop, 679 ns per transaction, ~4.3% of loop wall clock.This is the largest win in the stack.
Correctness
parsed[i]is written by index, so output order never depended on dispatch order. Cancellation is still checked per claim.TestParseBlockTxsMixesKnownAndRecoveredSenderscovers the mixed known/recovered path;-raceclean over repeated runs.Link to Devin session: https://app.devin.ai/sessions/0952787e9df84a7994fda23fea5db400
Open in Devin Desktop: https://app.devin.ai/desktop/session/0952787e9df84a7994fda23fea5db400?variant=devin
Requested by: @bdchatham