Skip to content

evmonly/evmonlyapp: size the next block's decode from its cost and the time it has; time PrepareBlock - #4262

Merged
bdchatham merged 4 commits into
giga-1from
devin/1789850370-parse-workers-cap
Sep 20, 2026
Merged

bdchatham merged 4 commits into
giga-1from
devin/1789850370-parse-workers-cap

Conversation

@bdchatham

@bdchatham bdchatham commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Describe your changes and provide context

Since #4260, PrepareBlock decodes block n+1 (RLP decode + ecrecover for senders CheckTx did not see) on GOMAXPROCS workers while block n's OCC speculation holds a worker per processor. The occ_speculate share rose 0.19 → 0.36 s/s on that roll, so the prepare pool is competing with the block on the critical path for work that only has to finish before block n does.

Rather than a fixed share of the processors, the decode is sized per block from the work and the time it has:

// giga/evmonly
func (e *Executor) PrepareBlockWithin(ctx, req BlockRequest, budget time.Duration) (PreparedBlock, error)
//   workers = clamp(ceil(len(txs) × perTxCost / budget), 1, min(ParseWorkers, len(txs)))
//   budget == 0, or no estimate yet  → every parse worker (unchanged behaviour); budget == 0 does not feed the estimate
//   perTxCost: elapsed × workers / txs over earlier budgeted decodes on this executor —
//              a decode above the estimate replaces it at once, one below decays it by 1/8
func (e *Executor) PrepareBlock(ctx, req) = PrepareBlockWithin(ctx, req, 0)

// evmonlyapp
budget = executeEstimate / 2   // executeEstimate: EWMA (1/8) of ExecutePreparedBlock wall time over prepared blocks

So on today's testnet-2 blocks (~1,850 txs, ~50 µs/tx ecrecover, ~20 ms execution) it decodes on ~9–13 workers of 32 (the measured cost includes contention, so the fixed point sits a little above the nominal count — the safe side); on a small block it uses 1–2; on a 5k-tx block it grows; and on a different box or tx mix the per-tx estimate re-fits itself. The asymmetry is deliberate: an undersized decode delays the block that needs it (the fetcher hands the prepared block over synchronously), an oversized one only spends processors, so the estimate jumps up on the first poorly-cached block and comes down slowly. The budget is smoothed for the same reason — a single short block dents it rather than zeroing it, which would hand the next decode every worker. The unprepared FinalizeBlock path (executeBlockPipelined) keeps PrepareBlock with no budget, so a decode that is on the critical path still uses every worker. Config.ParseWorkers remains the ceiling (still GOMAXPROCS in production wiring).

Also adds an evmonly_prepare phase timer (parse) around the decode so prepare wall time vs block time is visible after the roll; evmonly_prepare_phase_duration_seconds_total{phase="parse"}.

Measurement after roll: occ_speculate s/s and executed tx/s vs the #4261 baseline (~94k/validator), and evmonly_prepare parse wall staying well under evmonly_finalize execute wall. If parse wall approaches execute wall, a mid-decode re-check of the remaining budget is the next step.

Testing performed to validate your change

  • giga/evmonly: parse_sizer_test.go — every worker until the first estimate, fits the estimated cost in the budget (5 → 3 → 1 workers as the budget grows; capped at the ceiling / tx count), no budget uses every worker, estimate rises at once and decays gradually, ignores empty observations. go test -race ./giga/evmonly/ green.
  • sei-tendermint/internal/evmonlyapp: TestPrepareBudgetIsAShareOfTheTypicalExecution, TestExecuteEstimateSmoothsOverBlocks; go test -race ./internal/evmonlyapp/ green (prepared-hit and unprepared-fallback paths both covered by the existing tests).
  • golangci-lint run on both packages, make fmtcheck clean.

Link to Devin session: https://app.devin.ai/sessions/ff612badcded4aa5914ea408dbb41888
Open in Devin Desktop: https://app.devin.ai/desktop/session/ff612badcded4aa5914ea408dbb41888?variant=devin
Requested by: @bdchatham

… PrepareBlock

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@cursor

cursor Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes block-pipeline scheduling and parallelism; mis-sized budgets could delay prepared blocks or increase OCC contention, but zero-budget and unprepared paths preserve prior behavior.

Overview
Ahead-of-time block decoding no longer always uses every parse worker (GOMAXPROCS). The executor gains PrepareBlockWithin with a time budget and a parseSizer that picks worker count from tx count, budget, and an EWMA of per-tx decode cost (fast upward, slow downward). PrepareBlock is unchanged for callers that need full parallelism (budget 0).

The EVM-only app passes half the smoothed ExecutePreparedBlock wall time as the prepare budget, updates that estimate only on the prepared finalize path, and wraps prepare decode in evmonly_prepare phase metrics. Unprepared finalize still calls PrepareBlock with no budget so on-critical-path decode stays maxed out.

Reviewed by Cursor Bugbot for commit 3ae1d46. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedSep 20, 2026, 2:17 AM

@codecov

codecov Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.86%. Comparing base (7e3ca24) to head (3ae1d46).
⚠️ Report is 42 commits behind head on giga-1.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           giga-1    #4262       +/-   ##
===========================================
+ Coverage   65.55%   90.86%   +25.31%     
===========================================
  Files        2081       22     -2059     
  Lines      157460     3504   -153956     
===========================================
- Hits       103222     3184   -100038     
+ Misses      54097      320    -53777     
+ Partials      141        0      -141     
Flag Coverage Δ
sei-chain ?
sei-chain-pr 90.86% <100.00%> (?)
sei-db ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
giga/evmonly/executor.go 91.43% <100.00%> (+0.22%) ⬆️
giga/evmonly/parse_sizer.go 100.00% <100.00%> (ø)
sei-tendermint/internal/evmonlyapp/app.go 87.18% <100.00%> (+5.41%) ⬆️

... and 2068 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caps PrepareBlock's parse pool at max(2, GOMAXPROCS/4) and adds an evmonly_prepare{phase="parse"} timer so the tradeoff against OCC speculation can be measured. The change is correct and the single-timer-per-app assumption holds (PrepareBlock has one caller, the block fetcher goroutine); the only note is that the same knob also throttles the synchronous fallback decode inside FinalizeBlock.

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.

Comment thread sei-tendermint/internal/evmonlyapp/app.go Outdated
… pool

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…as, not a fixed share of the processors

PrepareBlockWithin picks the fewest parse workers the block's decode is
expected to fit in a budget on, from a moving estimate of the per-tx cost
measured on earlier decodes; evmonlyapp budgets half the previous block's
execution time. No budget (the unprepared FinalizeBlock path, or before the
first block) decodes on every worker.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration devin-ai-integration Bot changed the title evmonlyapp: prepare the next block on a quarter of the processors; time PrepareBlock evmonly/evmonlyapp: size the next block's decode from its cost and the time it has; time PrepareBlock Sep 20, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a5fad63. Configure here.

Comment thread sei-tendermint/internal/evmonlyapp/app.go Outdated
@bdchatham

Copy link
Copy Markdown
Contributor Author

@seidroid

@seidroid

seidroid Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Claude finished @bdchatham's task in 3m 31s —— View job


Review

  • Read the diff against origin/giga-1
  • Review parse_sizer.go (EWMA + worker sizing)
  • Review executor.go (PrepareBlockWithin)
  • Review evmonlyapp budget wiring + evmonly_prepare timer
  • Post findings

No blockers. The arithmetic is right and the wiring is sound. Four notes on the control loop, all non-blocking for an A/B, and two nits.


What I verified

  • giga/evmonly/parse_sizer.go:38 — ceil division is correct (budget > 0 is guaranteed by the guard above), and int64(txs)*perTx has ~6 orders of magnitude of headroom at realistic block sizes.
  • giga/evmonly/executor.go:202-208 — observe is skipped when parseBlockTxs errors, so a cancelled or undecodable block can't poison the estimate. Good.
  • NewExecutor sets e.cfg = cfg.WithDefaults() before newParseSizer(e.cfg.ParseWorkers) (executor.go:104-107), so maxWorkers ≥ 1 without relying on the max(...,1) in the constructor.
  • parseBlockTxs takes the in-caller serial path at workers <= 1 (parser.go:32), so a 1-worker decision costs no goroutines or errgroup.
  • PhaseTimer is documented not concurrency-safe (sei-db/common/metrics/phase_timer.go:97), but PrepareBlock is driven only by the single fetcher goroutine (sei-tendermint/internal/p2p/giga_router_common.go:501), so preparePhases has one owner. Holds.
  • The CAS loop and the test arithmetic agree; EWMA truncation ((measured-current)/8 → 0 when the gap is < 8ns) is irrelevant at the µs scale these live at.

I could not run go test or gofmt here — both were refused by the sandbox — so I'm relying on the PR's own report that go test -race on both packages is green.


1. The budget comes from block n−1 but is spent against block n

prepareBudget reads a.lastExecute, which is stored at the end of ExecutePreparedBlock (app.go:764). When the fetcher prepares block n+1 it is overlapping block n's execution, which hasn't finished — so the budget is half of block n−1's execute time, used as a forecast of block n's. The /2 is the entire margin, and it only covers the next block being up to 2× shorter.

The consequence is sharper than "we might miss a prepare", because prepare is synchronously on the critical path: the fetcher does PrepareBlock(n+1) and then sends on an unbuffered channel (giga_router_common.go:508-512), so the main loop cannot start block n+1 until prepare returns. There is no degrade-to-unprepared here — prepared=false only fires on a height/hash mismatch. So a decode sized to 20 ms (from a 40 ms block n−1) that lands against a 5 ms block n adds ~15 ms directly to block n+1's latency, which is strictly worse than the contention it was avoiding.

Worth considering: clamp the budget by something that reflects the block actually in flight, or re-check remaining time mid-decode rather than committing the worker count up front.

2. The EWMA measures a quantity that depends on the knob it sets

measured := elapsed * workers / txs is called "estimated processor time to decode one transaction" (parse_sizer.go:18-19), but under contention it isn't processor time — it's wall time scaled by a worker count that was itself competing. With P processors, OCC holding P runnable goroutines and w parse workers, the parse pool gets roughly P·w/(P+w) processors, so elapsed ≈ txs·c·(P+w)/(P·w) and measured ≈ c·(1 + w/P). The estimate therefore rises with w, which raises w next block — positive feedback.

It converges rather than diverges (fixed point w* = A·P/(P−A) for A = txs·c/budget), but it converges above the intended value: with the PR's own numbers (1850 txs, 50 µs/tx, 10 ms budget, P=32) A ≈ 9.25 but w* ≈ 13. And as A → P it pins at maxWorkers. Not a defect — just worth knowing that the observed steady state will sit above the 9–10 the description predicts, and that the perTx doc comment overstates what the number is. Fix this →

3. A short block hands the next prepare every worker

(Same as Bugbot's, and it's real.) A block that executes in 1 ms gives budget = 500 µs; a following 1,850-tx block needs needed = 185, clamped to maxWorkers. So every dip in block size restores exactly the full parse/OCC contention this PR exists to remove. It's bounded at the pre-PR behaviour rather than worse than it, which is why I'd not block — but a floor on the budget (or deriving it from the block interval rather than only from the last execute) would stop it recurring. Fix this →

4. Sender-cache hit rate moves per-tx cost by ~50×, and the loop recovers slowly

PrepareBlock passes a.peekSenders(req.Txs), so a block whose senders CheckTx already saw skips ecrecover entirely — ~1 µs/tx instead of ~50 µs/tx. The EWMA blends both regimes into one number and moves only 1/8 per block, so a run of well-cached blocks leaves the estimate low, and the first poorly-cached block after it gets sized for ~1 µs/tx and overruns by a large factor. Combined with finding 1, the penalty for that lands as pipeline stall, not as a cheaper unprepared path. Overestimates cost only some CPU; underestimates cost wall time on the critical path. An asymmetric adjustment (fast up, slow down) would match that.

Also note observe folds in decodes from executeBlockPipelined's unprepared call, which runs in a different contention regime, into the same average.


Nits

  • app.go:673-674 — defer a.preparePhases.Reset() is registered before the call, so phase="parse" also covers a.prepared.Lock(), which contends with takePrepared in FinalizeBlock. That inflates the exact metric the PR wants to compare against evmonly_finalize{phase="execute"}. Ending the parse phase right after PrepareBlockWithin returns (or naming the store a second phase) keeps the number clean.
  • app.go:111-113 — the lastExecute comment says "how long the most recent FinalizeBlock spent executing", but only the prepared branch stores it (app.go:764); the executeBlockPipelined branch leaves it stale. Say "the most recent prepared FinalizeBlock" so the field describes what it holds.
    · branch devin/1789850370-parse-workers-cap

…imate at once and lower it gradually

Review follow-ups on the adaptive sizing: the budget is half an EWMA of the
prepared blocks' execution time rather than half the last one, so a single
short block does not hand the next decode every processor; the per-tx cost
jumps up to any decode that overran its estimate and decays down, since an
undersized decode delays the block that needs it; unbudgeted decodes (the
unprepared FinalizeBlock path) no longer feed the estimate; the parse phase
timer ends before the prepared slot is taken.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

Re seidroid's review — addressed in 3ae1d46, one item at a time:

1 & 3 (budget from n−1; short block hands out every worker): the budget is now half an EWMA (1/8) of the prepared blocks' execution time (executeEstimate / nextExecuteEstimate) rather than half the last one. A single 1 ms block in a run of 20 ms blocks moves the estimate to 17.6 ms (budget 8.8 ms) instead of 0.5 ms; a sustained change in block size still tracks. I didn't add a hard floor: a small enough floor is another constant, and with the estimate smoothed the case it would cover is a sustained run of short blocks, where the next block is also likely small so needed stays low. The /2 plus the asymmetric cost estimate (below) is the margin for block n being shorter than typical; if the evmonly_prepare timer shows parse wall exceeding execute wall on testnet-2, a mid-decode re-check is the next step.

2 (the estimate depends on the knob): agreed, that's inherent to measuring wall time under contention; the fixed point sits above the nominal count, which is the safe side. Reworded the perTx comment to say what it measures (worker time including the processor share the workers were given).

4 (sender-cache hit rate; slow recovery): observe is now asymmetric — a decode that measured above the estimate replaces it outright, one below decays it by 1/8 — so the first poorly-cached block corrects the estimate immediately and only cheap blocks are trusted slowly. Unbudgeted decodes (the unprepared FinalizeBlock path, budget 0) no longer feed the estimate.

Nits: the parse phase now ends when PrepareBlockWithin returns, before a.prepared.Lock(); lastExecute is replaced by executeEstimate with a comment stating that only prepared blocks contribute.

@bdchatham
bdchatham enabled auto-merge September 20, 2026 02:22
@bdchatham
bdchatham added this pull request to the merge queue Sep 20, 2026
Merged via the queue into giga-1 with commit 9a731d5 Sep 20, 2026
63 checks passed
@bdchatham
bdchatham deleted the devin/1789850370-parse-workers-cap branch September 20, 2026 02:37
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.

1 participant