Skip to content

perf(website): gateway-free render benchmark with counter budgets - #23431

Open
Pearcekieser wants to merge 1 commit into
kirodotdev:mainfrom
Pearcekieser:perf/perf-bench-ci
Open

Pearcekieser wants to merge 1 commit into
kirodotdev:mainfrom
Pearcekieser:perf/perf-bench-ci

Conversation

@Pearcekieser

@Pearcekieser Pearcekieser commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

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

  • The CI job that runs the check. It follows in a separate PR, so perf work can use the script now.
  • Any change to app code. The only store change lives in the profiling build's transform. The prod build and website/dist are untouched.
  • Wall-time or frame-rate budgets. They depend on the runner.
  • Fixing anything the benchmark finds.

What changed (motivation → approach → change)

npm run bench:render builds the SPA with react-dom/profiling into node_modules/.cache/kc-perf/dist, then runs scripts/bench-render.mjs against it. The build config adds two store seams to that build only: a middleware that counts dispatches and a useAppSelector wrapper that counts selector calls. The script serves the build with serve-dist and answers /api/** with stub-dashboard-api, using a seeded world from lib/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 an onWebSocket option, so the script can push frames into the mocked dashboard socket. Four scenarios, idle, s1x1, s1x5 and s2, 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.mjs runs 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.json compares the 3-run medians with the budgets and exits 1 when a counter is more than 15% over. package.json gains the two bench:render scripts and no dependencies; Playwright and react-dom are already there.

Backwards compatibility

Compatible: stubDashboardApi's new onWebSocket option 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.ts mounts 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.

Scenario, counter pod run at a9898a0 this script at a9898a0 this script on main
s1x5 dispatches 3,102 3,165 3,173
s1x5 selector calls 1,853,817 1,819,815 1,913,509
s1x5 commits 219 68 * 248
s1x1 dispatches 631 641 647
s1x1 selector calls 375,174 367,867 389,303
s2 dispatches 314 311 320
s2 selector calls 241,307 221,877 244,118
s2 commits 3,643 2,799 3,068

* 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-s000 in 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 in idle: 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:render passes on main. The same run against a budget file with idle.commits: 20 exits 1 with Over 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.json holds 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.

semgrep is not installed on this host, so the repo's semgrep rules were not run locally. CI's SAST job runs them.

Related Issues

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@Pearcekieser
Pearcekieser requested review from a team and KiroCrewCaptain as code owners October 23, 2026 10:31
@Pearcekieser
Pearcekieser requested a review from buluoray October 23, 2026 10:31
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention fork Pull request from a fork (external contributor) readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 23, 2026
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5, fork) — ⏭️ skipped

Revision e0b426e9678c70c93ba2547783ba4fd77dfa0e08 ships no reviewable capability (docs, tests or generated files only), so there is nothing to inventory. Does not block merge.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed e0b426e9678c70c93ba2547783ba4fd77dfa0e08 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] e0b426e

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review (fork) — ✅ no blocking findings

Reviewed e0b426e9678c70c93ba2547783ba4fd77dfa0e08 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] e0b426e

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

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 workflow_run and has no manual dispatch, so I can't restart it from the fork side. Could a maintainer re-run the Fork Design Review run for this head, or record a design override if the lane has nothing to judge here? The diff adds a benchmark script, its fixture and probe, a budget file, a test and a docs section. It changes no app code.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

note (non-blocking): pushed f382d48, a same-tree retrigger of a871e52 with only the commit date changed, because Fork Design Review never started on the previous head.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Oct 24, 2026
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5, fork) — ✅ PASS

UX-level review of e0b426e9678c70c93ba2547783ba4fd77dfa0e08 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

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 .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of e0b426e9678c70c93ba2547783ba4fd77dfa0e08 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

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

  • The s2 commit counts depend on frame rate. The author saw them vary from 2,719 to 3,579 between runs on one host, and that spread is wider than the 15% tolerance. Before the follow-up CI job gates s2 commits on a runner of a different speed, either re-baseline on that runner or leave s2 commits ungated.
  • storeSeams patches store/index.ts by exact string match. It throws when the match fails, so it breaks loudly rather than silently. Still, any change to configureStore formatting will break bench:render until someone edits the config.

[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 .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 24, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: per-selector wrapper cache in the profiling store seam

issue (blocking): fixed in 938e295.

fixed: __kcSelWrap now returns one cached wrapper per selector from a WeakMap, so react-redux 9 keeps its selector memo across renders.
A new test in src/test/renderProbe.test.ts asserts the same selector gets the same wrapper and a different selector a different one.
Re-measured 3 runs: idle selector calls 5,272 to 5,162, streaming scenarios under 1% change; budgets in perf-budgets.json re-based on that run.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

self-added: yes
mechanism: per-selector wrapper cache in the profiling store seam

issue (blocking): fixed in 938e295.

fixed: __kcSelWrap caches one wrapper per original selector in a WeakMap, so a stable selector keeps its identity and react-redux's reference cache holds.
src/test/renderProbe.test.ts asserts wrapper identity per selector.
selectorCalls re-measured after the fix and perf-budgets.json re-based (idle 5,200; s2 250,000).

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

note (non-blocking): agreed. App code and the prod build are unchanged; the store seams exist only in the profiling build under node_modules/.cache/kc-perf/dist, and stubDashboardApi's new onWebSocket option defaults to the old no-op.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

note (non-blocking): agreed. The diff adds developer tooling only and changes no rendered UI, so there is nothing to screenshot.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 24, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

note (non-blocking): rebased on main, no behaviour change. Main had changed website/package.json since this branch's green, and UX Review never started on 938e295, so this push re-runs every lane.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Oct 24, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

note (non-blocking): agreed. The store seams exist only in the profiling build under node_modules/.cache/kc-perf/dist; the prod build and app code are unchanged.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

note (non-blocking): agreed. The diff is developer tooling only and renders no UI.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: passed Eligible automated validation passed for the current revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant