From 9760de01f86f934725bce96a6c4cac3d5ff759c7 Mon Sep 17 00:00:00 2001 From: Victor Velazquez Date: Wed, 2 Sep 2026 23:50:27 +0200 Subject: [PATCH] perf: stop the gallery re-listing the whole store once per card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured on the real store (178 canvases across 15 projects), the worst project page took 4.2s cold and 2.2s warm. It now takes 0.09s. The cause was not the score cache, which is where I expected it. Scoring a canvas costs ~8ms and is cached. Computing the CACHE KEY cost ~95ms, because `evalCacheKey` asks `designedStatesFor` which state variants a canvas has, and that calls `listCanvases()` — which version-hashes every canvas in the store on every call. Once per card, twice on a cache miss. A 43-canvas project on a 178-canvas store did roughly 7,600 whole-canvas hashes per request, so the page was quadratic in the size of the whole store rather than the project being viewed. The cache was costing eleven times what it saved. The fix is to take one listing per render and thread it through: `designedStatesFor`, `evalCacheKey` and `evalFor` all accept states the caller already has, and `renderProjectPage` derives them from the listing it was already fetching. Both single-argument forms still work, so the detail page and the tests are unaffected. Everything else measured clean and was left alone: aggregation is 185ms cold and 65ms warm, and the score cache's 200-entry cap is not thrashing at 178 canvases. Neither was worth touching. test-viewer-perf.ts pins the shape rather than a wall-clock budget, so it means the same thing on a CI runner as on a laptop: quadrupling the store must not cost ~16x. Verified against the pre-fix code, which measures 14.7x (45ms → 662ms); the fix measures 4.0x (3ms → 12ms). --- src/viewer.ts | 36 +++++++++++------ test-viewer-perf.ts | 96 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 test-viewer-perf.ts diff --git a/src/viewer.ts b/src/viewer.ts index 7e01cce..458debc 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -1,5 +1,5 @@ import { createServer, type IncomingMessage, type Server } from 'node:http'; -import { getCanvas, listCanvases, archiveCanvas, unarchiveCanvas, deleteCanvas, ensureFresh, touchCanvas } from './scene-graph.js'; +import { getCanvas, listCanvases, archiveCanvas, unarchiveCanvas, deleteCanvas, ensureFresh, touchCanvas, type CanvasSummary } from './scene-graph.js'; import { resolveVariables } from './variables.js'; import { renderToHtml } from './renderer.js'; import { getProject, listProjects, listWorkspaces, getCanvasTokens, getProjectDesignSystem, getWorkspaceDesignSystem } from './workspaces.js'; @@ -484,7 +484,12 @@ export async function renderProjectPage(projectId: string, port: number): Promis const ws = listWorkspaces().find((w) => w.id === project.workspaceId); const wsName = ws?.name ?? 'Personal'; - const canvases = listCanvases().filter((c) => c.projectId === projectId && !c.archived); + // One listing for the whole render: it version-hashes every canvas in the + // store, so it is the most expensive call on this path and must not be + // repeated per card. + const allRows = listCanvases(); + const statesById = new Map(allRows.map((r) => [r.id, r.variants?.map((v) => v.state) ?? []])); + const canvases = allRows.filter((c) => c.projectId === projectId && !c.archived); // Phase 24 slice A — a screen and its state variants are ONE card: variants // whose base is in this listing fold into the base card as chips. A variant // whose base is missing (orphan) or lives elsewhere stays a standalone card. @@ -497,7 +502,7 @@ export async function renderProjectPage(projectId: string, port: number): Promis const date = new Date(c.createdAt).toLocaleString(); const isEmpty = !canvas.root.children || canvas.root.children.length === 0; // Phase 19 Slice A — gallery score badge (fast eval, cached; empty → none). - const ev = await evalFor(canvas); + const ev = await evalFor(canvas, statesById.get(c.id) ?? []); const scoreBadge = ev ? `
${ev.overallScore}
` : ''; @@ -829,19 +834,25 @@ ${FAVICON_HTML} const scoreCache = new Map(); /** FNV-1a over the evaluation's actual inputs. Exported for tests. */ -/** The state variants designed for a canvas (sibling lookup, non-archived). */ -function designedStatesFor(canvasId: string): string[] { - return listCanvases().find((r) => r.id === canvasId)?.variants?.map((v) => v.state) ?? []; +/** The state variants designed for a canvas (sibling lookup, non-archived). + * + * `listing` exists because this is called once per canvas while rendering a + * gallery, and `listCanvases()` version-hashes every canvas in the store on + * every call — so looking it up per card made a page render quadratic (a + * 43-canvas project did ~7,600 whole-canvas hashes and spent 2s of a 2.2s + * response inside this one line). Pass the listing the caller already has. */ +function designedStatesFor(canvasId: string, listing?: CanvasSummary[]): string[] { + return (listing ?? listCanvases()).find((r) => r.id === canvasId)?.variants?.map((v) => v.state) ?? []; } -export function evalCacheKey(canvas: Canvas): string { +export function evalCacheKey(canvas: Canvas, states?: string[]): string { const genre = (canvas.metadata?.provenance as { preset?: string } | undefined)?.preset ?? ''; let tokens = ''; try { tokens = JSON.stringify(getCanvasTokens(canvas)); } catch { /* mirrored canvas with no live project — tree + genre still key it */ } // Phase 24 slice C — the coverage check reads the canvas's designed state // variants, so adding/removing a variant must invalidate the base's score. - const states = designedStatesFor(canvas.id).sort().join(','); - const payload = `${JSON.stringify(canvas.root)}|${tokens}|${genre}|${JSON.stringify(canvas.components ?? {})}|${states}`; + const stateKey = (states ?? designedStatesFor(canvas.id)).slice().sort().join(','); + const payload = `${JSON.stringify(canvas.root)}|${tokens}|${genre}|${JSON.stringify(canvas.components ?? {})}|${stateKey}`; let h = 0x811c9dc5; for (let i = 0; i < payload.length; i++) { h ^= payload.charCodeAt(i); @@ -850,15 +861,16 @@ export function evalCacheKey(canvas: Canvas): string { return `${canvas.id}:${(h >>> 0).toString(36)}`; } -async function evalFor(canvas: Canvas): Promise { +async function evalFor(canvas: Canvas, states?: string[]): Promise { if (!canvas.root.children || canvas.root.children.length === 0) return null; // empty → no score - const key = evalCacheKey(canvas); + const designed = states ?? designedStatesFor(canvas.id); + const key = evalCacheKey(canvas, designed); const hit = scoreCache.get(key); if (hit) return hit; try { // Match the agent: relax tells for the canvas's own genre (provenance preset). const genre = (canvas.metadata?.provenance as { preset?: string } | undefined)?.preset; - const result = await evaluateCanvas(canvas, { mode: 'fast', genre, designedStates: designedStatesFor(canvas.id) }); + const result = await evaluateCanvas(canvas, { mode: 'fast', genre, designedStates: designed }); if (scoreCache.size > 200) scoreCache.delete(scoreCache.keys().next().value as string); scoreCache.set(key, result); return result; diff --git a/test-viewer-perf.ts b/test-viewer-perf.ts new file mode 100644 index 0000000..1679118 --- /dev/null +++ b/test-viewer-perf.ts @@ -0,0 +1,96 @@ +import './test-env.js'; +/** + * The gallery must not re-list the whole store once per card. + * + * `listCanvases()` version-hashes every canvas in the store on every call, so + * it is the most expensive thing on the render path. The score badge used to + * reach for it once per card — through `designedStatesFor` inside both + * `evalCacheKey` and `evalFor` — which made a page render quadratic in the size + * of the WHOLE store, not just the project being viewed. Measured on the real + * store (178 canvases, 43 on the project): 4.2s cold and 2.2s warm for one + * page, ~7,600 whole-canvas hashes per request. Hoisting the listing to one + * call per render took the same page to 0.09s. + * + * The scaling check below is a ratio rather than a wall-clock budget so it + * means the same thing on a slow CI runner as on a fast laptop. Quadratic + * growth over a 4x larger store is ~16x; linear is ~4x. The bound sits at 8x — + * clear of honest variance, nowhere near quadratic. + * + * Run with: npx tsx test-viewer-perf.ts + */ +import { createCanvas } from './src/scene-graph.js'; +import { parseAndExecute } from './src/operations.js'; +import { renderProjectPage, evalCacheKey } from './src/viewer.js'; +import { DEFAULT_PROJECT_ID } from './src/types.js'; +import { ensureDefaultWorkspaceAndProject } from './src/workspaces.js'; + +let allPass = true; +function check(name: string, cond: boolean, extra?: string) { + if (!cond) allPass = false; + console.log(`${cond ? 'PASS' : 'FAIL'} ${name}${extra ? ` — ${extra}` : ''}`); +} + +/** A canvas with enough substance that scoring and hashing both do real work. */ +function seed(n: number): void { + for (let i = 0; i < n; i++) { + const c = createCanvas(`Screen ${i}`); + parseAndExecute(c.root, ` +U("document", {width:1440, height:900, layout:"vertical", gap:24, padding:32, fill:"#FFFFFF"}) +h=I("document", {type:"frame", width:"100%", layout:"horizontal", gap:16, alignItems:"center"}) +I(h, {type:"text", content:"Section ${i}", fontSize:24, fontWeight:600, color:"#0F172A"}) +b=I("document", {type:"frame", width:"100%", layout:"vertical", gap:12, padding:24, fill:"#F8FAFC", cornerRadius:12}) +I(b, {type:"text", content:"Supporting copy for screen ${i}.", fontSize:16, color:"#334155"}) +I(b, {type:"text", content:"A second line of body copy.", fontSize:16, color:"#334155"}) +`, c); + } +} + +async function timeRender(): Promise { + const t = Date.now(); + const html = await renderProjectPage(DEFAULT_PROJECT_ID, 3001); + if (!html) throw new Error('project page did not render'); + return Date.now() - t; +} + +/** Median of several renders — one sample at these speeds is mostly noise. */ +async function medianRender(runs = 5): Promise { + const times: number[] = []; + for (let i = 0; i < runs; i++) times.push(await timeRender()); + return times.sort((a, b) => a - b)[Math.floor(runs / 2)]; +} + +// ── the shape: render cost grows with the store, but not quadratically ─────── +ensureDefaultWorkspaceAndProject(); +seed(40); +await timeRender(); // warm the score cache + JIT +const small = Math.max(await medianRender(), 1); + +seed(120); // 4x the store +await timeRender(); // warm again — we are measuring shape +const large = await medianRender(); + +const ratio = large / small; +check('a 4x larger store does not cost ~16x to render', + ratio < 8, `40 canvases ${small}ms → 160 canvases ${large}ms (${ratio.toFixed(1)}x)`); + +// ── the threading that made it possible stays correct ──────────────────────── +{ + const c = createCanvas('Key check'); + parseAndExecute(c.root, `I("document", {type:"text", content:"Hello", fontSize:16, color:"#0F172A"})`, c); + + // Callers that pass the states they already have must agree with callers that + // let the function look them up — otherwise the gallery and the detail page + // would key the same canvas differently and never share a cache entry. + check('an explicit empty state list matches the looked-up form', + evalCacheKey(c, []) === evalCacheKey(c)); + + // And the states still have to participate: a canvas that gains a designed + // state must not keep serving the score it had before. + check('different designed states → different key', + evalCacheKey(c, ['empty']) !== evalCacheKey(c, [])); + check('state order does not matter', + evalCacheKey(c, ['empty', 'loading']) === evalCacheKey(c, ['loading', 'empty'])); +} + +console.log(allPass ? '\nAll viewer-perf tests passed.' : '\nSOME TESTS FAILED'); +process.exit(allPass ? 0 : 1);