execution/stagedsync: reuse commitment BAL-fold buffer across blocks - #22801
Open
sudeepdino008 wants to merge 4 commits into
Open
execution/stagedsync: reuse commitment BAL-fold buffer across blocks#22801sudeepdino008 wants to merge 4 commits into
sudeepdino008 wants to merge 4 commits into
Conversation
commitmentCalculator.computeRootFromBAL allocated a fresh Updates via cc.updates.NewEmpty() every block (and per mid-block step checkpoint). For a ModeParallel calculator that allocates a fresh parallelUpdate -> prefixTrie -> prefixArena -> new(prefixSlab) — a [16384]prefixNode backing array (~1-1.5MB) — then discards it. Profiling a from-genesis glamsterdam-devnet-6 sync showed commitment.newPrefixArena as the single largest execution-time allocator (~20%, 2.54GB/30s), driven entirely by this per-block NewEmpty. Keep one reusable cc.balUpdates and Reset() it each fold instead. Reset clears the buffer while retaining the arena's first slab (the reuse path already used on the non-BAL compute path). The buffer is fully consumed within each fold on the single committer goroutine, so reuse is state-equivalent to NewEmpty. New TestUpdatesParallelReuseAvoidsFreshAlloc: NewEmpty=7 allocs/op -> Reset=0, buffer left clean (empty keys, root-only trie). Existing committer root-assertion tests and the full execution/commitment suite pass through the reused path.
sudeepdino008
marked this pull request as ready for review
July 28, 2026 15:00
sudeepdino008
requested review from
awskii,
mh0lt,
taratorio and
yperbasis
as code owners
July 28, 2026 15:00
taratorio
reviewed
Jul 28, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the execution staged-sync commitment calculator by reusing a per-block BAL fold commitment.Updates buffer across blocks, avoiding repeated prefix-trie arena slab allocations during from-genesis sync (especially impactful in ModeParallel).
Changes:
- Add
balUpdatestocommitmentCalculatorand reuse it viaReset()forcomputeRootFromBALinstead of allocating a freshUpdateseach block. - Close the reused
balUpdatesbuffer during calculator shutdown.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The shared commitment context still references it (post-exec ComputeCommitment runs after the deferred Stop), so closing it was a use-after-close. GC reclaims the arena at teardown.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
execution/stagedsync/committer.go:270
- The Stop() comment is misleading: as long as the SharedDomainsCommitmentContext still references the Updates buffer via SetUpdates, the buffer cannot be GC’d even after the calculator is dropped. It would be clearer to state that we intentionally avoid Close() because the buffer may still be referenced, and it will only become collectible after the commitment context is pointed at a different Updates instance.
// Not closed here: the shared commitment context may still reference it
// (post-exec ComputeCommitment); GC reclaims it when the calculator drops.
sudeepdino008
force-pushed
the
sudeepdino008/reuse-commitment-fold-buffer
branch
from
July 30, 2026 04:42
2c696cc to
8e34251
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The commitment calculator's BAL fold (
computeRootFromBAL) allocated a freshUpdatesbuffer viacc.updates.NewEmpty()on every block. InModeParallelthat allocates a new prefix-trie arena each time — a fixed 16,384-node slab (~1–1.5 MB) — then throws it away.This reuses one buffer and
Reset()s it per fold instead.Why
Profiling a from-genesis sync,
commitment.newPrefixArenawas the single largest execution-time allocator (~20%) — and almost all of it wasted: a block touches a few hundred of the slab's 16k nodes before the whole slab is discarded.Correctness
The fold buffer is created, filled, consumed into a root, and never retained past the fold — all on the single committer goroutine — so
Reset()-reuse is equivalent to a fresh allocation.Reset()already exists and is used on the non-BAL compute path. Existing committer root-assertion tests and the fullexecution/commitmentsuite pass through the reused path.