Uh oh!
There was an error while loading. Please reload this page.
[ci] Run benchmarks in-deployment to avoid proxy overhead - #2967
Conversation
The CI benchmark drives load from a GitHub Actions runner through the public api.vercel.com ingress, so TTFS/WO were dominated by that external path rather than the runtime. Datadog traces of a real run show server-side "queue pickup → step body" is only ~170ms, yet the client-anchored TTFS was ~1192ms; ~800ms of that is VQS dispatch on an idle deployment plus the create request's inbound leg through api.vercel.com. Anchor TTFS/WO on the Vercel-assigned run_created timestamp (server-stamped createdAt, read via world.events.list with a world.runs.get fallback) and end on the deployment's step-body clock, making both metrics independent of the CI runner's clock and its network path to api.vercel.com. Add a flat +80ms (BENCH_RTT_OVERHEAD_MS) as an estimate of the client→ingress request overhead the server anchor doesn't capture, and note it in the PR comment. A diagnostic log prints server-anchored vs client wall-clock TTFS so the 80ms can be tuned. Move WO off the stream scenarios (where a single step makes it algebraically identical to TTFS) to a server-anchored whole-run aggregate on the sequential scenario. STSO is unchanged (already measured between step bodies on the deployment); SL stays client-observed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
📊 Workflow Benchmarkscommit Backend:
📜 Previous results (3)db0bb85Fri, 17 Jul 2026 20:47:55 GMT · run logs
63ddeb4Fri, 17 Jul 2026 20:00:22 GMT · run logs
9b712d5Fri, 17 Jul 2026 17:11:24 GMT · run logs
Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) 🟢/🔴 mark percentiles within/above target. Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · STSO (1-20) 20/30/60 · STSO (101-120) 30/45/90 · STSO (1001-1020) 40/60/120 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
Cold starts on the low-traffic workbench /flow invocation (~65% of runs on
preview) are the reason TTFS p75+ is ~1.3s despite warm dispatch being
~300ms. Keep them in the numbers (they are real bursty-workload latency) and
make the warm floor visible instead of hiding it.
- Add a p10 column to every metric (computeStats + render table). p10 shows
the warm-start floor next to the cold-start-inflated upper percentiles.
- Add benchStepWorkflow: one trivial no-op step, no stream, turbo mode — the
cleanest TTFS scenario ("step"), alongside the existing streaming-step
turbo scenario.
- Document in the file header and PR comment footnote that cold starts are
intentionally included and p10 is the warm reference.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
karthikscale3
left a comment
There was a problem hiding this comment.
Four inline findings from the benchmark methodology review.
| @@ -483,22 +574,11 @@ describe('workflow benchmarks', () => { | |||
| results.map((r) => r.ttfsMs), | |||
There was a problem hiding this comment.
[P1] Version the new methodology before comparing baselines. TTFS keeps the existing ttfs/stream and ttfs/hook + stream keys while changing from the CI/proxy-inclusive clock window to an in-deployment window, and the result schema is still version: 1. annotateWithBaseline() matches only backend/app/metric/scenario, so the PR comment's TTFS deltas compare incompatible measurements; historical entries also inherit the new footer. Please add a methodology/schema version to baseline and history matching, or suppress those comparisons until main has produced a compatible baseline.
There was a problem hiding this comment.
Fixed in 6c7ede8. Results now carry methodologyVersion: 2 and annotateWithBaseline() keys deltas on methodologyVersion/backend/app/metric/scenario, so old proxy-inclusive main baselines no longer match the in-deployment runs — the delta column stays blank until main has produced a v2 baseline, then v2-vs-v2 comparisons resume. Added a suppresses deltas when the baseline methodology version differs test. (The footer legend is a single global block describing the current methodology; per-entry footers for collapsed history are out of scope here.)
| namespace: SL_STREAM_NAMESPACE, | ||
| }); | ||
| const writer = writable.getWriter(); | ||
| await new Promise((resolve) => setTimeout(resolve, SL_WRITE_DELAY_MS)); |
There was a problem hiding this comment.
[P1] Replace the fixed delay with an explicit reader-ready barrier. A 750 ms sleep cannot guarantee the parallel reader step has started and is blocked: under scheduler delay or load, the writer can publish first and the reader will consume a retained chunk. That measures scheduling/catch-up rather than live pub/sub propagation. Please initiate reader.read(), signal readiness through a separate hook/stream, and let the writer stamp/write only after observing that signal.
There was a problem hiding this comment.
Fixed in 6c7ede8. Replaced the 750ms sleep with an explicit reader-ready barrier over a second stream: the reader initiates its SL read() (establishing the stream GET) then writes a marker to bench-sl-ready; the writer blocks reading that marker before it writes+stamps the SL chunk. So the write always lands on an already-attached reader (live propagation), not a retained chunk consumed by a late reader. The ready handshake tolerates either attach order (a retained marker is fine — we don't measure its latency), so no deadlock.
| ttfs: { | ||
| name: 'TTFS', | ||
| description: | ||
| 'time to first step body (in-deployment start() → first step body, deployment clocks)', |
There was a problem hiding this comment.
[P2] Update the renderer test with the new output shape..github/scripts/render-benchmark-comment.test.js still expects the old table header without P10 (line 106) and the old TTFS wording (lines 111-114), so node --test .github/scripts/render-benchmark-comment.test.js fails. Please update both assertions and add coverage for the P10 cell.
There was a problem hiding this comment.
Fixed in 6c7ede8. Updated render-benchmark-comment.test.js: the header assertion now includes P10 (ms), the TTFS wording matcher is updated, and there's a new P10-cell assertion (| 412 | 357 | 398 🔴 |). node --test passes (11/11).
| * Vercel only. | ||
| * @workflow/world-postgres, local filesystem otherwise. Because SL is now | ||
| * measured inside the workflow (not by a reader in this process), it no longer | ||
| * depends on `run.getReadable()` working across processes; CI still runs this |
There was a problem hiding this comment.
[P3] Keep the benchmark workflow documentation aligned..github/workflows/benchmarks.yml:98-100 still says SL requires run.getReadable() from the test process and therefore cannot use the local world's in-process streamer. This PR moves the reader inside the workflow, so that comment now contradicts the implementation.
There was a problem hiding this comment.
Fixed in 6c7ede8. Updated the benchmarks.yml comment — SL is now measured inside the workflow (benchSlWorkflow's parallel reader/writer steps), so it no longer needs run.getReadable() from the test process; the runner only polls returnValue.
…st/doc sync - [P1] Add methodologyVersion (=2) to results; renderer keys baseline deltas on it so the in-deployment numbers aren't diffed against old proxy-inclusive main baselines (deltas stay blank until main has a v2 baseline). Covered by a new "suppresses deltas when methodology differs" test. - [P1] Replace the fixed 750ms SL writer delay with an explicit reader-ready barrier: the reader initiates its SL read then signals on a second stream; the writer blocks on that signal before writing. Measures live propagation, not a late reader catching up on a retained chunk. - [P2] Update render-benchmark-comment.test.js for the P10 column header and new TTFS wording, and add P10-cell coverage. - [P3] Fix the stale benchmarks.yml comment claiming SL needs run.getReadable() from the test process — SL is now measured inside the workflow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No backport to This commit refines a benchmark harness that only exists on To override, re-run the Backport to stable workflow manually via |
Problem
The CI benchmark drove load from a GitHub Actions runner through the public
api.vercel.comingress. Datadog traces of real runs proved the reported latencies were dominated by that external path and the CI runner's clock, not the runtime:steps[0].start − clientStart(GHA), so it included the CI→ingress leg, GHA↔deployment clock skew, and the full VQS dispatch. The earlier revision of this PR anchored TTFS/WO on the serverrun_createdtimestamp and added a flat +80ms RTT estimate — proxy-independent, but a fudge, and it still measured the external-trigger dispatch rather than what a real in-Vercel caller experiences.Goal (hard requirement): no api.vercel.com / CI-runner timing may appear in any measured window.
Approach
Trigger every run from inside the deployment and measure entirely from deployment-side clocks.
POST /api/benchroute on the nextjs-turbopack workbench app resolves a bench workflow from the generated registry, stampsclientStartwith the deployment's clock immediately beforestart(), and returns{ runId, clientStart }. The GHA→route request (and its proxy path) sits beforeclientStart, so it is outside every measured window. Callingstart()in-deployment also engages the runtime's in-process fast path (optimisticStart), so turbo TTFS now reflects the inline path and non-turbo (hook) reflects the dispatch path — both proxy-free.benchSlWorkflowmeasures SL entirely on the deployment: a reader step and a writer step run in parallel on a dedicated namespaced stream. The writer waits a fixed delay so the reader is attached and blocked on the first chunk first (measuring live pub/sub propagation, not a warm read), then writeswrittenAt; the reader stampsreadAt.SL = readAt − writtenAt, both deployment step-body clocks, excluding the api.vercel.com read path.benchmark.test.tstriggers each run via the route (with trusted-sources headers) and pollsreturnValueby runId. TTFS/WO derive from the in-deploymentclientStart+ step timings; SL from the return value. RemovedrunCreatedServerMs, the flat RTT estimate, and the wall-clock diagnostic. STSO was already deployment-clock only and is unchanged.Result
All four metrics (TTFS, STSO, WO, SL) are now computed purely from Vercel-side timestamps. The only residual skew is intra-Vercel (step-runner instances in the same region), NTP-bounded and small relative to the measured values.
Latency targets are provisional — now that the proxy leg and the external-dispatch framing are gone, they will be re-tightened once a few in-deployment baselines land on
main.Notes
pnpm changeset status --since=mainis clean).