From edadfcae9656ad0e4ea36d16b4f73cca26e82cea Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 16:53:20 -0500 Subject: [PATCH] perf(desktop): stop every zone subscribing to the whole layout tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TreeGroup called useStore($layoutTree) to build its right-click menu's move/split directions. That subscribes every zone — and therefore every mounted pane and its entire transcript — to the whole layout tree. A sash drag rewrites the tree once per frame, so dragging the sidebar re-rendered all five tiles' message lists on every pointermove, for a context menu nobody had open. The directions are only read when the menu renders, so read the tree there with .get() instead. Same lazy shape the neighbouring `closable` prop already uses. Measured over one 60px sash drag with five busy tiles: commits 83 -> 12 ChatView 150 -> 10 (4465ms -> 353ms) AuiProvider 9450 -> 630 (9868ms -> 774ms) TreeGroup 180 -> 12 TreeSplit 90 -> 6 Also fixes an observer effect in the harness: idle-cost recorded render attribution *during* the timed gesture, and the counter walks the fiber tree on every commit. That was large enough to hide this 15x reduction behind an unchanged fps, so timing and attribution are separate passes now and `record` defaults off. Adds scripts/diag-drag-churn.mjs — the probe that found this. It reports the transcript chain (who above the messages re-rendered) plus every atom that notified, which is what named TreeGroup instead of leaving it to be guessed at. Notably the atom list came back EMPTY: this was never store churn, so the render-attribution path was the only thing that could have found it. --- apps/desktop/scripts/diag-drag-churn.mjs | 158 ++++++++++++++++++ .../scripts/perf/scenarios/idle-cost.mjs | 20 ++- .../pane-shell/tree/renderer/tree-group.tsx | 65 ++++--- 3 files changed, 211 insertions(+), 32 deletions(-) create mode 100644 apps/desktop/scripts/diag-drag-churn.mjs diff --git a/apps/desktop/scripts/diag-drag-churn.mjs b/apps/desktop/scripts/diag-drag-churn.mjs new file mode 100644 index 000000000000..009c34e3d2c1 --- /dev/null +++ b/apps/desktop/scripts/diag-drag-churn.mjs @@ -0,0 +1,158 @@ +// Who re-renders the transcript during a sash drag? +// +// Standalone probe, not a benchmark: seeds tiles, then drags the sash while +// recording (a) render attribution and (b) every nanostores atom that notifies +// during the gesture. The idle-cost scenario proved the transcript re-renders +// ~18x above baseline during a drag but that the sash HANDLER is not the cause +// (identical counts at 0px and 60px displacement) — so this names the store +// that actually fires. +// +// node scripts/perf/diag-drag-churn.mjs [--port 9222] + +import { attach } from './perf/lib/launch.mjs' +import { sleep } from './perf/lib/cdp.mjs' + +const TILES = 5 +const TURNS = 20 + +const setup = ` + (() => { + const hook = window.__HERMES_SESSION_TILES__ + if (!hook) return 'no-hook' + const turn = (sid, i) => ([ + { id: sid + '-u' + i, role: 'user', timestamp: Date.now(), + parts: [{ type: 'text', text: 'Question ' + i }] }, + { id: sid + '-a' + i, role: 'assistant', timestamp: Date.now(), pending: false, + parts: [{ type: 'text', text: '## Finding ' + i + '\\n\\nSome prose with **bold** and \`code\`.\\n' }] } + ]) + window.__D__ = { ids: [] } + for (let n = 1; n <= ${TILES}; n++) { + const sid = 'diag-tile-' + n + const rid = 'diag-rt-' + n + const messages = [] + for (let i = 0; i < ${TURNS}; i++) messages.push(...turn(sid, i)) + messages.push({ id: sid + '-stream', role: 'assistant', timestamp: Date.now(), pending: true, + parts: [{ type: 'text', text: 'Working.' }] }) + window.__D__.ids.push({ sid, rid }) + hook.open(sid, 'center') + hook.patch(sid, { runtimeId: rid }) + hook.publish(rid, { + storedSessionId: sid, messages, branch: '', cwd: '', model: '', provider: '', + reasoningEffort: '', serviceTier: '', fast: false, yolo: false, personality: '', + busy: true, awaitingResponse: false, streamId: sid + '-stream', sawAssistantPayload: true, + pendingBranchGroup: null, interrupted: false, interimBoundaryPending: false, + needsInput: false, turnStartedAt: Date.now(), usage: null + }) + } + return 'ok' + })() +` + +const reveal = sid => `window.__HERMES_LAYOUT_TREE__.reveal(${JSON.stringify(`session-tile:${sid}`)})` + +// Drag, recording renders AND atom notifications together. +const DRAG = ` + (async () => { + const rc = window.__RENDER_COUNTS__ + const ac = window.__ATOM_CHURN__ + rc.start(); ac.start() + + const handle = document.querySelector('[role="separator"]') + if (!handle) { rc.stop(); ac.stop(); return JSON.stringify({ error: 'no sash' }) } + + const box = handle.getBoundingClientRect() + const y = box.top + box.height / 2 + const x0 = box.left + box.width / 2 + let x = x0 + const opts = { bubbles: true, cancelable: true, pointerId: 1, pointerType: 'mouse', isPrimary: true, button: 0, buttons: 1 } + handle.dispatchEvent(new PointerEvent('pointerdown', { ...opts, clientX: x, clientY: y })) + for (let i = 0; i < 30; i++) { + x += 2 + window.dispatchEvent(new PointerEvent('pointermove', { ...opts, clientX: x, clientY: y })) + await new Promise(r => requestAnimationFrame(r)) + } + window.dispatchEvent(new PointerEvent('pointerup', { ...opts, buttons: 0, clientX: x, clientY: y })) + + rc.stop(); ac.stop() + const all = rc.report(400) + const named = n => all.find(r => r.name === n) || null + return JSON.stringify({ + moved: Math.round(x - x0), + commits: rc.commits(), + renders: rc.report(14), + // The suspects: who at the TOP of the transcript tree re-rendered? + chain: ['ChatView', 'ChatRuntimeBoundary', 'AuiProvider', 'Thread', 'SessionTile', 'TileChat', + 'LayoutTreeRoot', 'TreeNode', 'TreeSplit', 'TreeGroup', 'SessionView', 'PaneShell'] + .map(n => ({ name: n, hit: named(n) })).filter(x => x.hit), + atoms: ac.report(30) + }) + })() +` + +const CLEANUP = ` + (() => { + if (window.__D__) { + for (const { sid, rid } of window.__D__.ids) { + const s = window.__HERMES_SESSION_TILES__.states() + window.__HERMES_SESSION_TILES__.publish(rid, { ...s[rid], busy: false, streamId: null }) + window.__HERMES_SESSION_TILES__.close(sid) + } + window.__D__ = null + } + return 'cleaned' + })() +` + +const port = Number(process.argv.includes('--port') ? process.argv[process.argv.indexOf('--port') + 1] : 9222) +const { cdp, teardown } = await attach({ port }) + +try { + await cdp.send('Runtime.enable') + + const ok = await cdp.eval(setup) + + if (ok !== 'ok') { + throw new Error(`setup failed: ${ok}`) + } + + for (let n = 1; n <= TILES; n++) { + await cdp.eval(reveal(`diag-tile-${n}`)) + await sleep(300) + } + + await sleep(1500) + + const data = JSON.parse(await cdp.eval(DRAG)) + await cdp.eval(CLEANUP) + + console.log(`moved ${data.moved}px, ${data.commits} commits\n`) + console.log('RENDERS during drag:') + + for (const r of data.renders) { + console.log( + ` ${r.name.padEnd(28)} r=${String(r.renders).padStart(6)} wasted=${String(r.wasted).padStart(6)} ` + + `props=${String(r.propsChanged).padStart(5)} state=${String(r.stateChanged).padStart(5)} ` + + `ctx=${String(r.contextChanged ?? 0).padStart(4)} ms=${r.totalMs}` + ) + } + + console.log('\nTRANSCRIPT CHAIN (who above the messages re-rendered):') + + for (const { name, hit } of data.chain) { + console.log( + ` ${name.padEnd(24)} r=${String(hit.renders).padStart(6)} wasted=${String(hit.wasted).padStart(6)} ` + + `props=${String(hit.propsChanged).padStart(5)} state=${String(hit.stateChanged).padStart(5)} ms=${hit.totalMs}` + ) + } + + console.log('\nATOMS that notified during drag:') + + for (const a of data.atoms) { + console.log( + ` ${a.name.padEnd(26)} notifies=${String(a.notifies).padStart(5)} wasted=${String(a.wasted).padStart(5)} ` + + `fanout=${String(a.fanout).padStart(6)} peakListeners=${a.peakListeners}` + ) + } +} finally { + teardown?.() +} diff --git a/apps/desktop/scripts/perf/scenarios/idle-cost.mjs b/apps/desktop/scripts/perf/scenarios/idle-cost.mjs index ab8cfcf37d24..d26fb7661c5e 100644 --- a/apps/desktop/scripts/perf/scenarios/idle-cost.mjs +++ b/apps/desktop/scripts/perf/scenarios/idle-cost.mjs @@ -78,13 +78,17 @@ const idleCost = seconds => ` })() ` -/** Record frame pacing across an interaction driven from the page, plus WHAT - * rendered during it — a slow interaction is almost never the handler, it is - * whatever re-renders underneath while the handler runs. */ -const withFrames = body => ` +/** Record frame pacing across an interaction driven from the page. + * + * `record` MUST be false for any fps number you intend to believe: the render + * counter walks the whole fiber tree on every commit, so recording during a + * gesture measures the instrumentation as much as the app. Attribution and + * timing therefore run as two separate passes — the observer effect here is + * large enough to hide a 15x render reduction behind an unchanged fps. */ +const withFrames = (body, record = false) => ` (async () => { const rc = window.__RENDER_COUNTS__ - rc.start() + ${record ? 'rc.start()' : ''} const frames = [] let last = performance.now() let stop = false @@ -98,7 +102,7 @@ const withFrames = body => ` requestAnimationFrame(tick) ${body} stop = true - rc.stop() + ${record ? 'rc.stop()' : ''} const total = frames.reduce((a, b) => a + b, 0) const sorted = [...frames].sort((a, b) => a - b) const pct = p => sorted.length ? sorted[Math.min(sorted.length - 1, Math.floor(sorted.length * p))] : 0 @@ -108,8 +112,8 @@ const withFrames = body => ` worst: sorted.length ? sorted[sorted.length - 1] : 0, slow33: frames.filter(f => f > 33).length, n: frames.length, - commits: rc.commits(), - top: rc.report(10) + commits: ${record ? 'rc.commits()' : '0'}, + top: ${record ? 'rc.report(10)' : '[]'} }) })() ` diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx index 1ed752f1dda6..74a278ed515c 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx @@ -89,7 +89,11 @@ function ZoneMenu({ /** False for the zone hosting the uncloseable workspace — collapsing the * MAIN pane strands the app behind a strip. */ minimizable?: boolean - directions: ZoneMenuDirection[] + /** Called when the menu renders, not on every zone re-render: resolving the + * neighbor zones has to read the layout tree, and subscribing every zone to + * it made a sash drag re-render every mounted pane. Same lazy shape as + * `closable`. */ + directions: () => ZoneMenuDirection[] headerHidden?: boolean minimized?: boolean nodeId: string @@ -100,7 +104,7 @@ function ZoneMenu({ {children} - {directions.map(direction => ( + {directions().map(direction => ( {direction.label} @@ -255,30 +259,43 @@ export function TreeGroup({ // - a single pane -> "Move ": join the zone visually adjacent on that // side (splitting here would only make an invisible empty zone). Sides // with no visible neighbor are omitted entirely. - const tree = useStore($layoutTree) + // NOT `useStore($layoutTree)`: this subscribes every zone — and therefore + // every mounted pane and its whole transcript — to the entire layout tree. + // A sash drag rewrites the tree once per frame, so dragging the sidebar + // re-rendered all five tiles' message lists on every pointermove (measured: + // TreeGroup 180 renders cascading into ChatView/Thread/TileChat at ~4.5s + // each, holding the drag at ~3fps). + // + // The tree is only read to build the zone context menu's move/split + // directions, which are consumed when the menu OPENS — so read it at that + // moment with `.get()` instead of subscribing to every intermediate frame. + const menuDirections = (): ZoneMenuDirection[] => { + if (shown.length > 1) { + return DIRECTION_ORDER.map(side => ({ + side, + label: `${t.zones.split(dirWord[side])} ${DIRECTION_ARROW[side]}`, + run: () => splitTreeZone(node.id, side, menuPane ?? activeId) + })) + } - const menuDirections: ZoneMenuDirection[] = - shown.length > 1 - ? DIRECTION_ORDER.map(side => ({ + const tree = $layoutTree.get() + + return DIRECTION_ORDER.flatMap(side => { + const neighbor = tree ? adjacentGroup(tree, node.id, side, g => g.panes.some(paneShown)) : null + + if (!neighbor || neighbor.id === node.id) { + return [] + } + + return [ + { side, - label: `${t.zones.split(dirWord[side])} ${DIRECTION_ARROW[side]}`, - run: () => splitTreeZone(node.id, side, menuPane ?? activeId) - })) - : DIRECTION_ORDER.flatMap(side => { - const neighbor = tree ? adjacentGroup(tree, node.id, side, g => g.panes.some(paneShown)) : null - - if (!neighbor || neighbor.id === node.id) { - return [] - } - - return [ - { - side, - label: `${t.zones.move(dirWord[side])} ${DIRECTION_ARROW[side]}`, - run: () => moveTreePane(activeId, { groupId: neighbor.id, pos: 'center' }) - } - ] - }) + label: `${t.zones.move(dirWord[side])} ${DIRECTION_ARROW[side]}`, + run: () => moveTreePane(activeId, { groupId: neighbor.id, pos: 'center' }) + } + ] + }) + } // Close targets the right-clicked chip (falling back to the active pane); // only panes that declare `uncloseable` (the main workspace) are exempt.