From f7aee9dc8c570336a0bce1fc8af023a8819642cb Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 15:54:18 -0500 Subject: [PATCH] fix(desktop): make render-churn measure streaming, not boot churn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems, both found by distrusting the harness's own numbers. 1. The scenario slept a fixed 1s after mounting tabs, then recorded. Boot and session hydration are not reliably done by then, so a variable amount of unrelated work landed inside the measurement window. Three back-to-back runs on identical code spread 2.2x on total_renders and 3.8x on wasted_renders — wide enough that a single-run before/after delta could be mostly noise. Replaced with a quiesce gate that waits for commits to hold still before recording, and reports 'quiet:N' or 'timeout:...' so a contaminated run is visible instead of silent. 2. The counter attributed a context-driven re-render as 'wasted', which pointed at memo() as the fix when memo cannot block context at all. Adds contextChanged via the fiber's context dependency list, and excludes it from wasted. The gate also turned up a finding worth more than the fix: with five busy tiles and NO driver running, the renderer still commits ~18x/sec. The report now names the cascade roots (own state changed, props did not) rather than leaving them to be guessed at — Streamdown re-renders itself 105 times while idle, which is what drives Block/Ct. --- .../scripts/perf/scenarios/render-churn.mjs | 48 +++++++++++++++++-- apps/desktop/src/debug/render-counter.ts | 39 +++++++++++++-- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/apps/desktop/scripts/perf/scenarios/render-churn.mjs b/apps/desktop/scripts/perf/scenarios/render-churn.mjs index d1e940cbed7e..d1322d58a659 100644 --- a/apps/desktop/scripts/perf/scenarios/render-churn.mjs +++ b/apps/desktop/scripts/perf/scenarios/render-churn.mjs @@ -107,6 +107,43 @@ const drive = (chunk, intervalMs, totalTokens) => ` })() ` +/** Wait until the renderer stops committing on its own, so the recording window + * captures STREAMING cost and not whatever boot/hydration work happened to + * still be in flight. Returns `quiet:N` once commits hold still for `quietMs`. + * + * If it returns `timeout:...` the app never went idle at all — with tiles + * marked busy and NO driver running, that means something is ticking on its + * own. The report of what rendered during the wait is attached so the culprit + * is named rather than guessed at. */ +const quiesce = (quietMs, timeoutMs) => ` + (async () => { + const rc = window.__RENDER_COUNTS__ + rc.start() + const deadline = Date.now() + ${timeoutMs} + const startedAt = Date.now() + let last = -1 + let stableSince = Date.now() + while (Date.now() < deadline) { + await new Promise(r => setTimeout(r, 100)) + const n = rc.commits() + if (n !== last) { last = n; stableSince = Date.now(); continue } + if (Date.now() - stableSince >= ${quietMs}) { rc.stop(); return 'quiet:' + n } + } + const idle = { + commits: last, + seconds: (Date.now() - startedAt) / 1000, + top: rc.report(8), + // Who OWNS the update? The component whose own hook state changed with + // no changed props is the root of a churn cascade; everything under it + // is collateral. Naming it is the difference between fixing the cause + // and memoizing a symptom. + owners: rc.report(200).filter(r => r.stateChanged > 0 && r.propsChanged === 0).slice(0, 8) + } + rc.stop() + return 'timeout:' + JSON.stringify(idle) + })() +` + const START = ` (() => { window.__RENDER_COUNTS__.start() @@ -173,9 +210,11 @@ export default { await sleep(350) } - await sleep(1000) - // Start recording AFTER mount so the numbers are steady-state streaming - // cost, not one-off mount cost. + // Let the app go quiet before recording, so boot/hydration commits that + // happen to still be in flight don't land in the streaming window. This is + // what makes runs comparable — a fixed sleep let 2-4x of hydration churn + // leak in depending on machine load. + const settle = await cdp.eval(quiesce(600, 15000)) await cdp.eval(START) await cdp.eval(drive(chunk, intervalMs, tokens)) await sleep(tokens * intervalMs + 1500) @@ -208,6 +247,9 @@ export default { detail: { tiles, tokens, + // 'quiet:N' = the app went idle before recording (comparable run). + // 'timeout:N' = it never did, so boot churn is mixed into the numbers. + settle, sidebar: sidebarRows, topRenders: data.renders.slice(0, 15), topAtoms: data.atoms.slice(0, 15) diff --git a/apps/desktop/src/debug/render-counter.ts b/apps/desktop/src/debug/render-counter.ts index 30dcae9146d9..2a41d792a245 100644 --- a/apps/desktop/src/debug/render-counter.ts +++ b/apps/desktop/src/debug/render-counter.ts @@ -27,9 +27,15 @@ export interface RenderRecord { /** ...of those, how many had changed hook state (useState/useMemo/store). */ stateChanged: number /** - * ...of those, how many had NEITHER changed props nor changed state. These - * re-rendered purely because a parent did — the wasted work a `memo()` or a - * narrower store subscription would eliminate. + * ...of those, how many consumed a context whose value changed. `memo()` + * cannot block these — the fix is to narrow or split the provider, not to + * add a memo boundary. + */ + contextChanged: number + /** + * ...of those, how many had NEITHER changed props, changed state, nor a + * changed context. These re-rendered purely because a parent did — the + * wasted work a `memo()` or a narrower store subscription would eliminate. */ wasted: number /** Sum of `actualDuration` across counted renders, in ms. */ @@ -41,6 +47,7 @@ let commits = 0 let recording = false const blank = (): RenderRecord => ({ + contextChanged: 0, propsChanged: 0, renders: 0, stateChanged: 0, @@ -84,6 +91,25 @@ function stateChanged(fiber: Fiber): boolean { return false } +/** Did any consumed context value change? A `memo()` cannot block a re-render + * caused by context, so distinguishing this from a parent-driven render is + * the difference between "add a memo" and "split the provider". */ +function contextChanged(fiber: Fiber): boolean { + let dep = fiber.dependencies?.firstContext + + while (dep) { + const context = dep.context as { _currentValue?: unknown } | undefined + + if (context && 'memoizedValue' in dep && !Object.is(dep.memoizedValue, context._currentValue)) { + return true + } + + dep = dep.next + } + + return false +} + function record(fiber: Fiber) { const name = getDisplayName(fiber) @@ -94,6 +120,7 @@ function record(fiber: Fiber) { const entry = counts.get(name) ?? blank() const props = propsChanged(fiber) const state = stateChanged(fiber) + const context = contextChanged(fiber) entry.renders += 1 entry.totalMs += fiber.actualDuration ?? 0 @@ -106,7 +133,11 @@ function record(fiber: Fiber) { entry.stateChanged += 1 } - if (!props && !state) { + if (context) { + entry.contextChanged += 1 + } + + if (!props && !state && !context) { entry.wasted += 1 }