perf(app): stop the per-second shell re-render and halve bundled renderer assets - #1352
ozymandiashh wants to merge 4 commits into
Conversation
…n, halve bundled renderer assets The desktop app's three quiet inefficiencies, measured and fixed: - AppMain owned a 1s wall-clock interval whose only consumer was the footer's 'refreshed Ns ago' label, but every tick reconciled the whole tree - sidebar, hero, chart, heatmap, tables - 60 times a minute. The tick now lives in a leaf RefreshedAt component, and the shell re-renders on real state plus a 15s day check that fires only when the local calendar rolls over (the overview memo keys bake in a today/month boundary, so midnight must re-render exactly once). A churn test pins shell render counts: base re-rendered on every clock tick, the branch on none. - Every read resolved the codeburn binary from scratch: a stat sweep over each PATH entry plus an nvm readdir, per request, per poll. Resolution and the derived spawn PATH are memoized against the full env key set that can change the answer; a miss is never cached, so a CLI that appears while the app is open is still discovered, and a spawn error drops the memo so a deleted binary cannot pin the app to a dead path. 26.5us -> 2.6us per resolution on this machine, and the syscalls leave the steady-state path entirely. - The brand mark rendered a 880x880 713kB PNG (3.1MB RGBA decode) into 20-76px boxes and the splash shipped a 2.6MB VP9 clip; a 192px lanczos cut (verified side-by-side at all three rendered sizes) and a CRF 30 re-encode (SSIM 0.994) cut dist/renderer from 4.0MB to 2.2MB.
…d asset-weight changes
38b7078 to
96807e7
Compare
iamtoruk
left a comment
There was a problem hiding this comment.
Measured on my machine. The render churn and the asset cut both check out: AppMain renders over 10 s idle on Overview went 10 to 0, and dist/renderer went 4,200 KB to 2,300 KB with the same 19 assets. Post the proof in the PR body so it survives: render count before and after or a CPU sample, the asset size table per file, and confirmation the "refreshed Ns ago" text still ticks every second.
On the CLI memo: 23 µs to 2 µs per resolution at a multi-second poll cadence is not a user-visible win, and the memo adds stale-path surface. The env key hashes the path of the CLI-path file, not its contents, so once resolution succeeds a newly persisted path is never picked up. The invalidation test writes the successor to the same path, so it passes with a stale memo too. Either drop the memo or key on the file contents and rewrite that test to use a different path.
Also: flame.png is still in the repo at 713 KB with no importer. Delete it.
Heads up: this conflicts with a branch of mine that rewrites the same footer line and still reads now. Whoever lands second merges by hand.
No screenshots needed, nothing visible changed.
…n the footer tick Per review on getagentseal#1352: - The resolution memo is gone entirely (both cli.ts and its tests restore to their pre-PR shape): 23us -> 2us at a multi-second poll cadence was never user-visible, and the env key hashed the CLI-path file's path, not its contents, so a newly persisted path could never displace a successful resolution — the invalidation test wrote its successor to the same path and passed with a stale memo. Drop beats re-keying. - flame.png (713 KB) is deleted from the repo; FlameMark imports the 192px cut and nothing else referenced it. - The churn suite gains the counterpart pin: the 'refreshed Ns ago' label still ticks once a second ('just now' -> '1s ago' -> '2s ago') while the shell's render count stays flat.
|
b260394 addresses the review:
Noted on the footer-line conflict with your branch — happy to be second and merge by hand. |
|
Heads-up: pushes to this branch after 96807e7 (b260394, c98410a) have started no workflow runs at all — |
Three quiet desktop inefficiencies, each measured before and after, each pinned by a regression test.
1. The shell stops re-rendering once a second
AppMainowned a wall-clocksetInterval(1000)whose only consumer was the footer's "refreshed Ns ago" label, but every tick reconciled the whole tree — sidebar, hero, daily chart, activity heatmap, every table — 60 times a minute whether or not any data had changed.The per-second tick now lives in a leaf
RefreshedAtcomponent that owns just that label. The shell re-renders on real state changes plus a 15-second day check that fires exactly when the local calendar rolls over; that rollover matters because the overview memo keys bake in a today/month boundary, so midnight must still produce one re-render to keep "Today" honest.App.renderChurn.test.tsxpins both properties: on the base the shell re-rendered on every clock tick and on same-day checks, on this branch neither.2. CLI resolution stops re-scanning the filesystem per request
Every read (each section poll, each prefetch warm) resolved the
codeburnbinary from scratch: a stat sweep over every PATH entry plus a readdir of the nvm versions tree — per request, purely to return the same path.Resolution and the derived spawn PATH are memoized against the full set of inputs that can change the answer (
CODEBURN_BIN,CODEBURN_BUNDLED_CLI, the dev-server and persisted-path overrides,CODEBURN_PATH_DIRS,PATH,NVM_DIR), so an unchanged environment costs a string compare. Safety valves:__resetCliResolutionForTests()keeps one test's filesystem layout out of the next.Measured on this machine (18 PATH entries): 26.5 µs → 2.6 µs per resolution, and the syscalls leave the steady-state path entirely.
3. Renderer assets halve
The brand mark was an 880×880 713 kB PNG (3.1 MB RGBA decode) rendered into 20–76 px boxes, so every mount decoded megabytes to draw a 20 px sidebar logo; the cold-start splash shipped a 2.6 MB VP9 clip.
flame-mark.png: a 192 px lanczos cut of the same art (53 kB, −93%), verified side-by-side against the original at all three rendered sizes (20/52/76 px).splash-loader.webm: re-encoded at VP9 CRF 30 — 1.29 MB, −50%, SSIM 0.994 against the source; video-only file, no audio track affected.dist/renderer: 4.0 MB → 2.2 MB. The JS bundle is untouched.Proof (per review)
Render churn. Pinned by
app/renderer/App.renderChurn.test.tsx: the shell's overview render count does not move across five 1 s ticks (growth ≤ 2, vs one render per tick before), rolls exactly once across the midnight boundary, and — the counterpart added for this review — the footer label still ticks every second (just now→1s ago→2s ago) while that same shell count stays flat. Reviewer's own measurement on his machine: AppMain renders over 10 s idle, 10 → 0.Asset weight, per file (local builds of
mainvs this branch; same 19 assets):dist/renderer: 4,228 KB → 2,328 KB locally (reviewer measured 4,200 → 2,300 on his machine).CLI memo: dropped. Per the review, the memoization is removed entirely (b260394) —
cli.tsandcli.test.tsrestore to their pre-PR shape. 23 µs → 2 µs at a multi-second poll cadence was never user-visible, and the env key hashed the CLI-path file's path, not its contents, so a newly persisted path could never displace a successful resolution (the invalidation test wrote its successor to the same path and passed with a stale memo). Drop beats re-keying.flame.png: deleted from the repo (713 KB, no importer after the cut).
Verification
tsc --noEmitcleanapp/suite green after the review fixes: 1,000 passed, 4 skippednpm run buildgreenChanged
setInterval(1000)that existed only so the footer could print "refreshed Ns ago", but every tick re-rendered the sidebar, the hero, the daily chart, the heatmap and every table 60 times a minute whether or not any data had changed. The per-second tick now lives in a leafRefreshedAtcomponent that owns just that label, and the shell itself re-renders only on real state changes plus a 15-second day check that fires exactly when the local calendar rolls over. Renderer render-count regressions are pinned by a churn test.Fixed