Repository navigation
perf(website): gateway-free render benchmark with counter budgets - #23431
Pearcekieser wants to merge 1 commit into
Conversation
First Principles Review (Opus 5.5, fork) — ⏭️ skippedRevision |
Opus 5.5 Review (fork) — ✅ no blocking findingsReviewed |
GPT 6.1 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
|
question (blocking): PR Readiness has waited on Design Review since 13:03Z with the status "1 readiness check(s) still pending; waiting on Design Review (not started)". Every other lane finished on head a871e52. Opus and GPT posted no blocking findings, First Principles posted a skip, and CI, Fast Gate and UX Review are green. The Fork Design Review workflow starts only from a Fast Gate |
a871e52 to
f382d48
Compare
UX Review (Opus 5.5, fork) — ✅ PASSUX-level review of UX-Verdict: PASS Nothing to check. The diff adds only a developer benchmark script, fixtures, a profiling build config, a test and docs. It adds or changes no user-visible control, so no screenshots are needed. [UX-REVIEWED] e0b426e Description read[DESCRIPTION-READ] c53565c4625649c64103a9a143dc50d750a8c3a660d03c99f49592f7fbe233ff That sha256 names everything this verdict read: the PR description, plus the 3 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with |
Design Review (Opus 5.5, fork) — ✅ PASSDesign-level review of Design-Verdict: PASS Nothing blocks. This is dev-only tooling that renders no new surface and takes nothing away, and the store seams exist only in the profiling build. Notes
[DESIGN-REVIEWED] e0b426e Description read[DESCRIPTION-READ] 0159c127b1cc6a5e4ee4e9b1d684b60cff89282050f9665a002d94b28f2cce54 That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with |
f382d48 to
938e295
Compare
|
self-added: yes issue (blocking): fixed in 938e295.
|
|
self-added: yes issue (blocking): fixed in 938e295.
|
|
note (non-blocking): agreed. App code and the prod build are unchanged; the store seams exist only in the profiling build under |
|
note (non-blocking): agreed. The diff adds developer tooling only and changes no rendered UI, so there is nothing to screenshot. |
938e295 to
e0b426e
Compare
|
note (non-blocking): rebased on main, no behaviour change. Main had changed |
|
note (non-blocking): agreed. The store seams exist only in the profiling build under |
|
note (non-blocking): agreed. The diff is developer tooling only and renders no UI. |
Problem / Motivation
Goal: Anyone can measure, before and after a change, how many React commits, renders, store dispatches and selector calls the dashboard spends on streaming traffic, with one command and no gateway.
CI checks the web bundle's size and nothing about render work. The only render measurements so far came from scripts that drive a live pod with a seeded home, so they cannot run in CI and are awkward to rerun for a perf PR.
Why it matters
Several perf fixes are queued for the streaming path: composer re-renders, store equality guards, the active-stream cascade and markdown parsing. Without a shared benchmark, each PR measures its own way and nothing stops the next regression. One script with fixed scenarios gives every one of those PRs a before and after number, and the budget file gives a CI job something to check.
Not a goal
website/distare untouched.What changed (motivation → approach → change)
npm run bench:renderbuilds the SPA with react-dom/profiling intonode_modules/.cache/kc-perf/dist, then runsscripts/bench-render.mjsagainst it. The build config adds two store seams to that build only: a middleware that counts dispatches and auseAppSelectorwrapper that counts selector calls. The script serves the build withserve-distand answers/api/**withstub-dashboard-api, using a seeded world fromlib/perf-fixture.mjs: 222 sessions in 8 folders, a 100-row open transcript, and the command bar app enabled as on a default install. The stub gains anonWebSocketoption, so the script can push frames into the mocked dashboard socket. Four scenarios,idle,s1x1,s1x5ands2, each send 300 ticks of 100 ms. They carry agentic background turns or a prose answer in the open chat, plus the gateway's 5 s status frame, which keeps the 20 s silence watchdog from reconnecting.lib/render-probe.mjsruns in the page. It walks each commit and attributes every render to a mount, a changed hook or selector, a context, changed props, or a parent re-render. A run is refused when the page logs an error, when no chunk reached the store, or when the streamed answer stays short, because each of those lowers the counts and would pass any upper bound.--check perf-budgets.jsoncompares the 3-run medians with the budgets and exits 1 when a counter is more than 15% over.package.jsongains the twobench:renderscripts and no dependencies; Playwright and react-dom are already there.Backwards compatibility
Compatible:
stubDashboardApi's newonWebSocketoption defaults to the old no-op, so every existing capture script behaves as before.Removes nothing: the diff only adds files, one npm script pair, one stub option and one docs section.
Tests
src/test/renderProbe.test.tsmounts an 8-component tree on react-dom/profiling under the probe and drives 3 updates. It asserts the cause the probe reports for each component:parent(same props),props:o~for a deep-equal object,props:fƒfor a new function,props:v!for a changed value,ctx:TestCtx!,uSES#0 [snap:getSnap]!for a store read,useState#0!, and no render for a memo child whose props are unchanged. A second test checks that nothing is counted while counting is off. Six runs with--sequence.shuffle, 12 tests, all passed. An earlier version that loaded React once per test failed 4 of 5 shuffled runs, because react-dom keeps the DevTools hook it saw at first load. The file now loads React once.Manual verification
These are 3-run medians on this host, desktop 1440x900. The reference column is the pod profiling run this script replaces, measured at a9898a0.
* The a9898a0 column ran on an earlier fixture, before two changes. First, every streaming row sat below the fold there. The sidebar does not re-render an offscreen row on its slot's status, so 180 status commits were missing. The pod home had one of the five rows on screen, so the fixture now pins
perf-s000in the first folder. Second, the fixture now sends the 5 s status frame, which adds 6 dispatches per scenario. The main column has both changes. On main, s1x5 commits come out 13% above the pod run. Most of the extra 29 commits are idle background work, react-query observer updates against the stub, which also show inidle: 29 to 31 commits here against 10 on the pod.Dispatches and selector calls land within 4% of the pod run on main. s2 commits vary 2,719 to 3,579 between runs on one host, because they follow the frame rate. The s2 budget allows for that.
npm run bench:renderpasses on main. The same run against a budget file withidle.commits: 20exits 1 withOver budget by more than 15%: idle.commits.The selector-counting wrapper is cached per selector, because react-redux 9 memoizes a selector's result by the selector's identity and a new wrapper on every render discarded that memo. After that fix, a 3-run re-measure on this host gave idle 5,162 selector calls, s1x1 389,618, s1x5 1,917,871 and s2 256,678, and
perf-budgets.jsonholds budgets from that run. s2 commits were 3,545 to 3,638 and its renders 661,034 to 673,584 in the same runs.A mobile run (
--viewport mobile --cpu 4, s1x5 and s2) completes. It waits on the store rather than a sidebar row, because the mobile sidebar is a closed drawer. At--cpu 4, s2 sends 60 ticks and gets 41 dispatches, because chunks that land in one frame flush together. The run completes. The 29 vitest files that reference a changed path pass, 854 tests.semgrepis not installed on this host, so the repo's semgrep rules were not run locally. CI's SAST job runs them.Related Issues
Checklist
feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)