Repository navigation
Backport evmonly parse, app-hash and changeset perf fixes from giga-1 - #4345
Conversation
One of four independent PRs from the serial-budget survey (#4234, #4235, #4236, #4237); each targets `giga-1` on its own. `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 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: | workers | before | after | | |---|---|---|---| | 1 | 768 µs | 808 µs | serial path, untouched | | 8 | 1,610 µs | 355 µs | 4.5× | | 32 | 1,516 µs | 332 µs | 4.6× | 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. `TestParseBlockTxsMixesKnownAndRecoveredSenders` covers the mixed known/recovered path; `-race` clean 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 --------- 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> (cherry picked from commit 85706b8)
One of four independent PRs from the serial-budget survey (#4234, #4235, #4236, #4237); each targets `giga-1` on its own. `hashEVMOnlyResult` wrote the stream straight at the hash, a few bytes per call — about 17,700 writes for a full block, each length prefix allocated fresh by `binary.BigEndian.AppendUint64(nil, ...)`. The stream now goes through a 32 KiB buffer with the prefix built in a scratch array on the writer. ## Measured Measured locally with a throwaway `BenchmarkHashEVMOnlyResult` (not included in this PR). 3,503 pairs, the shape a full block emits: | | time | allocs | |---|---|---| | before | 222 µs | 7,017 | | after | 179 µs | 14 | **42 µs off a 28.83 ms loop, 0.15%.** That is below the 0.4–0.6% the survey estimated, and the reason is worth recording: the stage is bound by sha256 over roughly 350 KiB of stream, not by the call overhead buffering removes. The survey's 1.2–2.0% "absolute ceiling" for this stage is not reachable without changing the hash shape, which is a consensus change and out of scope here. What the change does buy outright is **7,000 fewer allocations per block, ~244,000/s at 34.8 blocks**. ## Correctness Not one byte of the stream moves. `TestHashEVMOnlyResultGoldenVector` pins three fixtures (0, 1 and 64 accounts, covering balances, nonces, written code, deleted code, storage writes and storage deletions) to hashes **captured from the previous implementation before it was touched**: ``` pairs=0 0x98c012bb0ffae50b5f9969e668577c9449f0aa53d728cfba607162470a07f7cb pairs=1 0xcecd86d6a97aef46e100d52fde74c47115ebd66c8097535cbfa6c06a45665793 pairs=64 0x1828dc77edbd0c72a3d8c91aedb3657056ff906a70e88b9056e070a42b2e1257 ``` The app hash is what validators vote on, so a failure there means the stream changed, not that the vector is stale. The one new failure mode is a missing `flush` before `h.Sum`. The golden vector catches it, and `flush`'s error is returned rather than dropped. 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 --------- 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> (cherry picked from commit b434ef6)
…4237) One of four independent PRs from the serial-budget survey (#4234, #4235, #4236, #4237); each targets `giga-1` on its own. The encoder allocated a `KVPair`, a key and a value for every pair it emitted — 10,502 allocations for the 3,501 pairs a full block produces, where `EncodeMemoryStoreChangeSet` beside it does the same work in five. It now cuts pairs, keys and values from one slab each, the way that sibling does. ## Measured Measured locally with a throwaway `BenchmarkEncodeFlatKVChangeSet` (not included in this PR). 3,501 pairs: | | time | allocs | |---|---|---| | before | 185.7 µs | 10,502 | | after | 162.4 µs | 7 | **23 µs off a 28.83 ms loop, 0.08%** — an order of magnitude under the 0.5–1.0% the survey estimated. Its "3 heap objects per KV pair" was exactly right; the wall clock those allocations cost was not. What it removes outright is **10,495 allocations per block, ~365,000/s at 34.8 blocks**, which is GC the OCC workers were paying for. ## Correctness `TestEncodeFlatKVChangeSetGolden` pins the emitted pairs — key, delete flag, value, and order — to a readable `key|delete|value` listing **captured from the previous encoder before it was touched**, over a fixture covering all ten branches: written, zero and nil balances; a zero and a max nonce; written, deleted and empty code; written, deleted and zero-valued slots. These pairs are committed to FlatKV and enter its LtHash, so a change here changes the state root. `TestEncodeFlatKVChangeSetGoldenStorageClear` pins the storage-clear path the same way (clears interleaved with regular writes), also captured from the previous encoder. `TestEncodeFlatKVChangeSetRejectsNegativeBalance` keeps the out-of-range error and its exact message, which also passed against the old encoder. Two notes for review: - Keys and values are subslices of the slabs. That still satisfies `NamedChangeSetEncoder`'s contract (`giga_store.go:26-38`) that the returned slices outlive the changeset, because the slabs are allocated per call and never reused. - Storage clears are discovered by iterating the store, so their count is unknown at sizing time and they keep the per-pair path. A block that clears nothing — every block on a chain that never selfdestructs — stays entirely on the slabs. `keys.PutEVMKey` is added as `BuildEVMKey` for a caller that owns the storage. It returns false rather than writing a short key, and the builder panics on that, matching `flatKVStoragePrefixByte`. Keys and values are handed out with their capacity capped at their length, so a downstream `append` reallocates instead of writing into the neighbouring slab entry. ## Testing performed to validate your change `scripts/ramtest.sh ./giga/evmonly/ -run FlatKVChangeSet`, `go test ./sei-db/common/keys/`, `make fmtcheck`. Both goldens re-run against the pre-refactor encoder at the base commit (Go 1.27.1) and pass. 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 --------- 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> (cherry picked from commit f2eb22c)
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
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 #4345 +/- ##
==========================================
- Coverage 67.60% 66.53% -1.08%
==========================================
Files 2192 2071 -121
Lines 168752 156767 -11985
==========================================
- Hits 114087 104298 -9789
+ Misses 54655 52459 -2196
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PR SummaryMedium Risk Overview
FlatKV changeset encoding builds KVPairs from pre-sized slabs ( Adds Reviewed by Cursor Bugbot for commit da51c02. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
This backports three hot-loop optimisations: atomic range claiming in parseBlockTxs, a buffered app-hash writer, and slab-allocated FlatKV changeset pairs. Golden-vector tests pin the hash stream and the emitted pairs to their earlier values, and I found no correctness or safety issues in the diff.
Findings: 0 blocking | 0 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
* main: (21 commits) Backport evmonly parse, app-hash and changeset perf fixes from giga-1 (#4345) Remove the oracle module behind a v6.8 upgrade (#4319) fix(seidb): report only the current migration boundary on the snapshot gauge (#4327) fix(flatkv): keep 10 old checkpoints instead of mirroring memIAVL's count (#4322) Backport Autobahn execute-loop and produced-tx metrics from giga-1 (#4330) Add giga.storage.receipts to toggle the Autobahn receipt store (#4333) optimize gather phase (#4326) Regenerate the Unreleased changelog as a plain PR list (#4336) Bump sei-protocol/go-ethereum to v1.15.7-sei-21 (#4332) Fail dynamic-gas precompile out-of-gas as an EVM out-of-gas call (#4318) Add dashboard and topology option for Autobahn e2e (#4167) Add Giga fetch/serve and BlockDB prune metrics (#4329) Add eth_getLogs to the EVM-only Giga RPC (#4308) Generate v6.8 precompiles (#4320) Add [giga] app.toml section and honor it on the Autobahn node (#4323) feat(evmonly): add eth_estimateGas via existing libraries (#4325) Use Pebble batch directly in SS (#4300) Fix pruning issue in SS causing huge disk spike (#4321) Make Autobahn always run the EVM-only executor, disable/remove some integration tests (#4316) reduce seal lock contention (#4314) ...
Three small hot-loop optimisations from the evmonly serial-budget survey landed on
giga-1as #4234, #4236 and #4237 but were never brought tomain(PLT-1302, PLT-1304, PLT-1305 under PLT-1297). All three cherry-pick cleanly; this PR carries them as-is, one commit each.parseBlockTxsfed 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).hashEVMOnlyResultwrote 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 aKVPair, a key and a value per emitted pair; it now cuts all three from one slab per kind, the wayEncodeMemoryStoreChangeSetalready does, cutting roughly 10,500 allocations per block.None of this changes bytes on the wire or in state:
TestHashEVMOnlyResultGoldenVectorandTestEncodeFlatKVChangeSetGolden*pin the hashed stream and the emitted pairs to values captured from the previous implementations, andparsed[i]was always written by index so parse order never mattered. Validated withgo test -race ./giga/evmonly/ ./sei-tendermint/internal/evmonlyapp/on this branch.