Uh oh!
There was an error while loading. Please reload this page.
perf(core): drain consumable replay events synchronously - #2473
Merged
Conversation
🦋 Changeset detectedLatest commit: 410034e The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
Revert the synchronous consume-loop drain optimization: it caused a replay divergence (ReplayDivergenceError on step_started → CorruptedEventLogError) in the world-testing inline-batches parallel workflow on the Windows CI runner. The per-event `process.nextTick` in the consume loop is load-bearing — it guarantees at most one event is consumed per macrotask, letting the cross-VM `resolve → workflow VM body → subscribe()` chain register the next operation's consumer before the drain advances. A synchronous drain races ahead of that registration. What remains is a documentation comment on `scheduleWhenIdle` capturing why its initial `setTimeout(0)` must not be downgraded to a microtask (empirically: queueMicrotask breaks hook/sleep Promise.race ordering → CorruptedEventLogError). No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pranaygpforce-pushed
the
pgp/perf-reduce-macrotask-hops
branch
from
June 17, 2026 17:45
b9c9e19 to
5039d82CompareTooTallNate
approved these changes
Jun 17, 2026
TooTallNate
left a comment
Member
There was a problem hiding this comment.
Approve — correctly-scoped microtask-reduction on the consume hot path
Tight, well-bounded change: EventsConsumer.consume drained one event per process.nextTick, so replaying N consecutively-consumable events (the lifecycle consumer walking run_created→run_started, a step consumer walking step_created→step_started→step_completed) cost N macrotask hops. This drains them in one synchronous while loop via the extracted consumeOne, deferring only when nothing consumes the current event.
I verified the load-bearing safety invariant directly: within a synchronous drain pass, this.callbacks must not be mutated except by the loop's own Finished splice. Confirmed that subscribe() only ever does process.nextTick(this.consume) — never a synchronous re-entry — and that onConsumedEvent (the clock-advance callback) doesn't subscribe. So new consumers can only appear on a later tick, exactly as the comment claims. The splice-at-i-then-return-and-restart pattern is correct: no callback is skipped across the index shift.
Two details I checked specifically:
- The
nullsentinel can't spin the loop:consumeOnereturnscurrentEvent !== null, so a consumed end-of-events sentinel (which real consumers returnNotConsumedfor anyway) terminates the drain. The defensive guard against a pathological callback consumingnullis a nice belt-and-suspenders. notifyConsumedEvent(clock advance) ordering is preserved — it still fires once per consumed event, in the same order; it just happens within one tick now instead of spread across N. Behaviorally identical for the deterministic-clock machinery.
The scheduleWhenIdlesetTimeout(0) is correctly left as a macrotask — the added comment documenting why (cross-VM resolve→VM-body→subscribe chain needs the macrotask boundary; queueMicrotask empirically breaks hook/sleep Promise.race with CorruptedEventLogError) is exactly the kind of tribal knowledge that belongs in the code.
Built @workflow/core, full suite green (1237), including events-consumer.test.ts, workflow.test.ts (79), and async-deserialization-ordering.test.ts. CI is mid-run; nothing failing that's attributable to this. LGTM.
pranaygp
commented
Jun 17, 2026
ContributorAuthor
CI + review triage (commit |
pranaygp
commented
Jun 18, 2026
ContributorAuthor
Re-validation: revert was premature → optimization restored (commit |
| Signal | Optimization | Unmodified main |
|---|---|---|
| Local 8-way stress (80 runs each) | 4/80 (5.0%) | 7/80 (8.75%) |
Historical windows-latest Unit flake rate | — | ~13% (10/76) |
| Fresh Windows CI Unit job (real job, not fast-pathed) | 4/4 pass | 4/4 pass |
| Isolated low-load | 40/40 pass | — |
Run IDs (fresh Windows CI): opt = 27739171715, 27739454028, 27739455535, 27739457404; main = 27739493993, 27739495175, 27739497219, 27739498448. The exact Windows timeout reproduces on bare main at 4a5a23088 (run 27474356210), and the failure mode on both arms is a self-healing replay-divergence retry that times out — not a thrown CorruptedEventLogError.
The restored events-consumer.ts + test are byte-for-byte identical to the original optimization commit; scheduleWhenIdle's load-bearing setTimeout(0) is kept (with the explanatory comment). Core Unit suite: 1237 passed, 0 failed.
This was referenced Jun 18, 2026
This was referenced Jun 18, 2026
pranaygp
enabled auto-merge (squash)
June 18, 2026 06:19
Contributor
Backport PR opened against |
This was referenced Jun 18, 2026
Merged
3 tasks
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 freeto 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
Perf optimization #5 — reduce macrotask hops on the suspension/consume replay hot path.
EventsConsumer.consumerescheduledprocess.nextTick(this.consume)after every consumed event, so replaying N already-consumable events (structural lifecycle events,step_created/step_started, completed deliveries) cost N macrotask hops — O(N) per consume wave across a sequential replay. This drains consecutively consumable events in a single synchronous pass instead (O(N) hops → 1 per replay wave).scheduleWhenIdle's initialsetTimeout(0)is deliberately kept — it is load-bearing for cross-VM propagation and a comment now records why (see below).Why the synchronous drain is safe
Callbacks only ever consume events whose consumer is already registered. New consumers are registered by workflow VM body code that runs asynchronously off
ctx.promiseQueueafter a deliveryresolve(). When the next event's consumer is not yet registered, no callback consumes it and we fall through to the existing cross-VM-safe deferred unconsumed-event check, exactly as before — the synchronous drain does not change that path. Anullend-of-events sentinel never continues the drain, so it cannot spin past end-of-log.scheduleWhenIdleis unchanged: between a delivery'sresolve()and the VM body re-subscribing,pendingDeliveriesis already 0, and Node does not guarantee a host microtask settles after the cross-VM chain. Empirically, downgrading itssetTimeout(0)toqueueMicrotaskbreaks hook/sleepPromise.raceordering (CorruptedEventLogError). A comment inpackages/core/src/private.tsnow captures this invariant.A note on the earlier revert (it was premature)
An earlier round reverted this optimization down to a comment-only change after a single
world-testinginline-batches-debugfailure on thewindows-latestUnit job, attributing it to a cross-VM replay divergence caused by the drain.I re-validated that conclusion empirically and it does not hold — the failure is a pre-existing flake in that test on Windows, unrelated to this change:
windows-latestUnit runs on unmodifiedmainshows ~13% (10/76) flake rate, and the exactinline-batches-debug60s timeout occurs on baremain(e.g. commit4a5a23088, run27474356210). The failure mode is a self-healing replay-divergence retry that eventually times out — not a thrownCorruptedEventLogError.main7/80 (8.75%) — the optimization flakes less, with an identical divergence-retry failure mode on both arms. Isolated low-load: optimization 40/40 pass.Unit Tests (windows-latest)job, confirmed not fast-path skipped): optimization 4/4 pass (runs27739171715,27739454028,27739455535,27739457404; plus an earlier confirming run = 5/5),mainbaseline 4/4 pass (runs27739493993,27739495175,27739497219,27739498448).A single failure against a measured ~5–13% baseline flake rate is statistically meaningless, and the optimization's restored code is byte-for-byte identical to the original commit. Conclusion: flake, not a regression — the optimization is restored.
Verification
cd packages/core && pnpm test(the CI Unit command,WORKFLOW_TARGET_WORLD=local vitest run src) — 1237 passed, 0 failed, including the new drain test, suspension/replay/hook/wait/Promise.raceordering.world-testinginline-batches-debug— 3/3 pass locally on the restored branch.Changeset
'@workflow/core': patch— "Drain consecutively consumable replay events in a single synchronous pass instead of oneprocess.nextTickper event, removing O(N) macrotask hops from replay."