From 3f3d194dec86d1af8c1ce13ea5f638b10883a6f2 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Tue, 9 Jun 2026 17:49:54 +0200 Subject: [PATCH 1/2] fix: stabilize fastConfirmation perf bench MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-iteration `everyoneVotes(...)` (100K–1M validator loop) and `updateHead()` in `beforeEach` were generating GC pressure right inside the measurement window, which on CI inflated the µs-scale fn timings enough to trip the regression threshold. The "flip votes" logic also never actually flipped — same-epoch votes are rejected by `addLatestMessage`, so the second `everyoneVotes` call was a no-op. Replace `beforeEach` with a pass-through. `runFastConfirmationRules` doesn't mutate `store`, so each iteration replays the same work and no per-iteration reset is required. Bench is now stable across runs: ~80K–160K samples per config (was 25–424), wall time per config ~0.3–5s (was 2–17s), and the vc/eq variations no longer move the number — which is the correct behavior, since neither parameter enters the executed code path; their old apparent effect was the GC pressure from the removed beforeEach work. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../perf/forkChoice/fastConfirmation.test.ts | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/fork-choice/test/perf/forkChoice/fastConfirmation.test.ts b/packages/fork-choice/test/perf/forkChoice/fastConfirmation.test.ts index 4b89131f4718..ebaf6183480c 100644 --- a/packages/fork-choice/test/perf/forkChoice/fastConfirmation.test.ts +++ b/packages/fork-choice/test/perf/forkChoice/fastConfirmation.test.ts @@ -41,10 +41,8 @@ function runFCRRulesBenchmark(opts: Opts): void { const forkChoice = initializeForkChoice({...opts, fastConfirmation: true}); const head = forkChoice.updateHead(); - const prevBlock = forkChoice.getBlockHexDefaultStatus(head.parentRoot); - if (!prevBlock) throw Error("no prevBlock"); - // Vote everyone for head + // Vote everyone for head so the FCR vote-map paths see a populated voteNextIndices. everyoneVotes(head, forkChoice); // Advance to second slot of epoch 2 (not epoch boundary) @@ -60,25 +58,17 @@ function runFCRRulesBenchmark(opts: Opts): void { const store = forkChoice["fcStore"] as unknown as IFastConfirmationStore; if (!ctx) throw Error("fastConfirmationContext not initialized"); - return {forkChoice, ctx, store, head, prevBlock, currentVoteIs1: true, currentSlot}; - }, - beforeEach: (data) => { - // Flip votes to force fresh vote map computation each iteration - const target = data.currentVoteIs1 ? data.prevBlock : data.head; - everyoneVotes(target, data.forkChoice); - data.currentVoteIs1 = !data.currentVoteIs1; - data.currentSlot = (data.currentSlot + 1) as Slot; - - // Advance the slot (this calls updateHead + processAttestationQueue but NOT the FCR) - // We need to advance the slot so getCurrentSlot() returns the right value - data.forkChoice["fcStore"].currentSlot = data.currentSlot; - // Apply votes to proto array so getHead/isDescendant work correctly - data.forkChoice.updateHead(); - - return data; + return {ctx, store}; }, + // Pass-through: the bench harness invokes fn(beforeEach(inputAll)), so without a + // beforeEach fn would receive undefined. store is not mutated by the rule runner so + // no per-iteration reset is required. + beforeEach: (data) => data, fn: ({ctx, store}) => { - // Measure ONLY the FCR rules — build cache + snapshot + run rules + // Measure ONLY the FCR rules — build cache + snapshot + run rules. + // store is not mutated by runFastConfirmationRules (only the rule runner's caller + // would assign store.confirmedRoot), so each iteration replays the same work and + // no beforeEach reset is needed. const cache = createFastConfirmationCache(); const snapshot = buildFastConfirmationSnapshot(ctx, store, cache); runFastConfirmationRules(snapshot, ctx, store, cache); From 3c5c63e59c7ad57668edecd3beb0e40267e067f3 Mon Sep 17 00:00:00 2001 From: Lodekeeper <258435968+lodekeeper@users.noreply.github.com> Date: Wed, 10 Jun 2026 13:41:00 +0100 Subject: [PATCH 2/2] fix: pass vote.slot (not nextEpoch) to addLatestMessage in everyoneVotes (#9496) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Applies the `everyoneVotes` fix Gemini flagged at `discussion_r3382054138` (confirmed in `discussion_r3382950784`): `addLatestMessage`'s second arg is `nextSlot: Slot`, not an epoch. The helper passed `computeEpochAtSlot(vote.slot)` (= `3` for the `bc:96` snapshot), so `voteNextSlots[i] = 3` got stored. On the next `updateTime(97)` tick, `computeEpochAtSlot(3) = 0` then made every vote look 3 epochs stale, and `runFastConfirmationRules` measured the empty-vote path instead of the intended fully-voted path. Pass `vote.slot` directly so `computeEpochAtSlot(nextSlot)` inside `addLatestMessage` lands on the right epoch (= 3 for the snapshot). `computeEpochAtSlot` was the only reason for the `@lodestar/state-transition` import in this file; drop it too. ## Why it matters The original PR (`#9494`) already fixed the GC-pressure flakiness, but the benchmark numbers weren't measuring what the bench id (`runFastConfirmationRules vc:600000 bc:96 ...`) claims they're measuring. With this fix, the bench actually exercises the FCR vote-map paths with a populated `voteNextIndices`. ## Test plan - [ ] CI: `Benchmarks` job runs cleanly on the merged commit, with `runFastConfirmationRules` cases reflecting populated-vote-map cost. - [ ] CI: `Type Checks` passes (the import drop doesn't break anything else). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: lodekeeper --- .../test/perf/forkChoice/fastConfirmation.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/fork-choice/test/perf/forkChoice/fastConfirmation.test.ts b/packages/fork-choice/test/perf/forkChoice/fastConfirmation.test.ts index ebaf6183480c..fe6405bd9eff 100644 --- a/packages/fork-choice/test/perf/forkChoice/fastConfirmation.test.ts +++ b/packages/fork-choice/test/perf/forkChoice/fastConfirmation.test.ts @@ -1,6 +1,5 @@ import {bench, describe} from "@chainsafe/benchmark"; import {SLOTS_PER_EPOCH} from "@lodestar/params"; -import {computeEpochAtSlot} from "@lodestar/state-transition"; import {Slot} from "@lodestar/types"; import { buildFastConfirmationSnapshot, @@ -77,9 +76,11 @@ function runFCRRulesBenchmark(opts: Opts): void { } function everyoneVotes(vote: ProtoBlock, forkChoice: ForkChoice): void { - const nextEpoch = computeEpochAtSlot(vote.slot); const nextRoot = vote.blockRoot; for (let i = 0; i < forkChoice["balances"].length; i++) { - forkChoice["addLatestMessage"](i, nextEpoch, nextRoot, PayloadStatus.FULL); + // addLatestMessage's second arg is `nextSlot: Slot`, not an epoch — pass vote.slot directly so + // the internal computeEpochAtSlot(nextSlot) lands on the correct vote epoch instead of + // computeEpochAtSlot(epoch)=0 making every vote look 3 epochs stale. + forkChoice["addLatestMessage"](i, vote.slot, nextRoot, PayloadStatus.FULL); } }