perf: cut main-process and renderer costs that grow with cards, hidden output and orchestration - #89
Conversation
scripts/bench-runtime.mjs runs the built app in hidden windows with a throw-away HOME (idle, N terminal cards, a fixed-rate output flood in every card, 10 s after it, a canvas pan) and micro-benchmarks of the main-process hot paths, and prints medians. docs/performance-benchmark.md explains the scenarios.
appendScrollback advanced bufferStart past a dropped chunk but left the chunk in the array until more than 256 slots were dead, so a busy session kept up to 256 extra chunks alive (823 552 characters instead of 240 000 with 16 KB reads). The slot is now emptied when the chunk leaves the ring.
…ible setVisible(true) sent the renderer the whole retained scrollback (up to 240 000 characters) although the card already had everything up to the hide point and drops what it wrote before. Crossing the 0.5 zoom threshold did that for every card at once: 8 cards with full history sent 1.9 MB for 8 KB of new output. The replay now carries the output produced since the card was hidden; a stretch longer than the ring still arrives as the whole ring, so the truncation marker is unchanged.
…ting every card AgentControlService used terminals.list() as a lookup: every status, ownership check, observe_agent and list_agents joined the scrollback of every card on the canvas and threw it away (29 MB allocated per observe_agent call on 20 cards with full history). It now reads metadata through getMetadata(id) and listMetadata(); only the observed card's own scrollback is read. The plugin sessions.list answer is built from listMetadata() as well.
…gins read observe_agent, get_agent_result and the plugin screen ran every redaction rule over the whole 240 000- character scrollback (about 12 ms on an M1 Max) to keep its last 8 192 or 4 000 characters. SecretRedactionRegistry.redactTail masks a window that starts at least 16 384 characters before the tail (more when a held value could wrap over that), at a line start, and at the header of a private-key block still open there. A secret split by the tail's cut lies inside the window and is masked whole, as before; tests compare redactTail with masking the whole text for secrets placed around the tail cut and the window start.
…rst launch
The first Kimi launch ran spawnSync("kimi --help") on the main process (up to 3 s) to learn whether the CLI
takes a per-run MCP config; nothing else moved meanwhile. ProviderLaunchAdapters.warmKimiProbe runs the
same probe with execFile (same command, environment, limits and answer) 5 s after startup and after a
CLI recheck, and the launch uses that answer. A launch that comes before it still probes as before.
With the companion on, TerminalPresentation made an @xterm/headless screen for every session on its first event and parsed all of its output, although the glasses read only the sessions shared with them. The headless screen is now made on the first read, from the scrollback (the same text the stream carried), and fed from then on; the answer state is still kept for every session.
The camera lives in App state, so each pointer move of a pan renders App and the workspace, and every TerminalCard rendered again with it: the workspace passed new arrow functions and a rebuilt snap target list on every render, and nothing was memoized. TerminalCard is now React.memo'd (snap targets compared by value), and the workspace hands it callbacks that stay the same functions and call its latest handlers through a ref. A card still renders when its session, zoom, focus, selection or a neighbour's bounds change. With 8 cards a pan move rendered 8 TerminalCards (97 components); now none (48 components).
…x variant hermes.png was the 1024 px original (574 KB) and codex.png 544 px (146 KB), drawn at 24 to about 125 CSS px. Both are scaled down to 128 px plus a 256 px variant chosen through srcset on high-density displays: 574 KB + 146 KB become 12 + 42 KB and 12 + 37 KB. The marks are not otherwise changed; the assets README says how they were scaled.
electron-vite leaves every bundle unminified, so each window load parsed 1.83 MB of renderer JavaScript. The renderer build now minifies with esbuild (1.83 MB to 1.04 MB, CSS 149 KB to 126 KB); source maps stay off and audit:secrets passes on the built output. Main and preload bundles are unchanged.
…tting Quitting hung every card up and let Electron free the Node environment at once. A shell that exited during that teardown made node-pty's native exit watcher call into JavaScript that could no longer run; node-pty rethrew the failure as a C++ Napi::Error and the app aborted (SIGABRT). The benchmark's quit after the output flood hit it in most runs, on main as well. TerminalManager now tracks every PTY until its exit is reported, and waitForProcessExits() resolves once all have exited: after 2 s it sends SIGKILL to the rest and waits 1 s more. shutdownServices() starts that wait right after the terminal shutdown and awaits it before quitting finishes. The exit handler also never lets an exception reach node-pty's native callback, where a throw aborts the app too.
…built from them Each held value became a regular expression with a wrap-gap class between every two characters. From about 3,700 characters the pattern exceeded what V8 accepts, and the SyntaxError broke every redaction call (agent observe and result, plugin screens, failure details) as long as the value was held. Held values are now found in the text with its wrap characters (whitespace, box sides) taken out, by Knuth-Morris-Pratt per value, with a position map back to the original text and the same 64-character gap limit. The cost is linear in the text and the value, so the limit on a held value rises from 4,096 to 65,536 characters (PEM keys, service-account JSON). redactTail keeps masking a window at least as wide as the longest possible match, so the tail is still exactly what masking the whole text and cutting it gives.
|
I reviewed
Could you add regression cases for both, including the long-assignment equality check against |
… leave too few others The wrap-tolerant search matches each held value with its wrap characters taken out, and dropped a form that kept fewer than eight characters that way: `abc defg` was accepted by add() but then matched neither as a held value nor as a credential shape. A value whose own gaps are wider than a wrap gap (64 characters) was kept but could never match, since the search refuses to cross such a gap. Each value and JSON-escaped form that holds wrap characters is now also searched exactly as written (Knuth- Morris-Pratt over the text itself, so still linear). Both searches feed one leftmost-longest selection. A value that is mostly spaces still matches only as written, never as the fragment left without its spaces.
redactTail cut its window at a line start near the margin and relied on every rule matching at most a few thousand characters. Several do not: quoted and unquoted assignments, URL query values and userinfo, Authorization/Bearer/assignment/JSON separators followed by any amount of whitespace, and wrapped tokens that go on over many lines. When such a match began before the window, the window saw only its middle, nothing masked it, and the fallback did not fire because masking had not shortened the window (`password="` followed by 40 000 characters of `a ` came back unmasked). The window now starts at a line start that no match can cross, found by walking back over line starts: it lies in no private-key block (blocks found from the text's start, the way the PEM rule sees them), the last non-whitespace character before it is none a wrapped run or a separator's whitespace continues after, no JSON secret value is open over it, and no held-value match crosses it or touches what those checks read. From there every pass finds the same matches as on the whole text, so the tail is exactly redact(text).slice(-maxChars). With no such line start within 64k characters, with less than the tail left after masking, or with a held value holding PEM armour and a private-key header in the text, the whole text is masked.
|
Thanks for the review. Both findings reproduced; they are fixed as two new commits on top of Final head: 1. Registered values dropped after whitespace normalizationCommit: Root cause. Fix.
Test (
2. The tail window could start inside an unbounded matchCommit: Root cause. The window started at a line start near
When such a match started before the window, the window saw only its middle. Your Fix. The window now starts at a line start that no match can cross. The search walks back over line starts from the desired cut. A line start qualifies only if all of the following hold:
From such a line start, every pass (held values, JSON values, each rule) finds the same matches in the window as in the whole text. At the cut, a lookbehind sees a line break in the whole text and the start of input in the window, and every rule treats those alike. Masking before the cut only inserts markers ending in The whole text is masked instead in three cases:
Ordinary scrollback still masks a bounded window: the existing "masks a window around the tail" test keeps its bound of Test: "redactTail equals masking the whole text when a match with no length bound starts before the window". It includes your case: an empty registry and the long quoted value with
Each shape is placed so that its match ends inside the tail, just before it, or far before it, with tails of 300 and 8 192 characters. The test also covers a registry holding a private key (plain and inside JSON) with PEM blocks in the text. All existing Also checked.
Gates on |
The probe test compares the blocking and background answers. Both used the 3 s production limit, which a loaded machine can hit while starting the stub CLI, so the test failed on load rather than on a wrong answer. The limit is now a parameter (default unchanged) and the test passes 30 s.
…e base for this change
2 participants
Goal
Remove runtime costs that grow with the number of cards and with scrollback size, keep every visible behaviour the same, and add a benchmark so the next change can be measured the same way.
What was wrong, what changed
Line numbers are at
31f287d.TerminalManager.ts:1741-1755(appendScrollback)bufferChunksuntil more than 256 slots were dead. With 16 KB reads a session kept 1 376 768 characters alive for a 240 000 ring.tests/terminal-output-costs.test.mjs: referenced text equalsbufferLengthfor 1/16/64 KB chunks; history reads back unchangedTerminalManager.ts:821(setVisible)scrollbackTail). A stretch longer than the ring still sends the whole ring, so the truncation marker is unchanged.AgentControlService.ts:96,116,124,191,registerIpc.ts:480terminals.list()used as a lookup by id: every status check, ownership check,observe_agentandlist_agentsjoined the scrollback of every card.TerminalManager.getMetadata(id)andlistMetadata(); only the observed card's own scrollback is read. The pluginsessions.listanswer useslistMetadata().tests/agent-control.test.mjs: a tool call reads nolist()and only the observed card's bufferAgentControlService.ts:145,165,PluginSessions.ts:207SecretRedactionRegistry.redactTailmasks a window starting at least 16 384 chars before the tail (more when a held value could wrap over that: each held value can matchlen × 65chars), moved to a line start and to the header of a private-key block still open there. A secret split by the tail cut lies inside the window and is masked whole, as the item-6 fix requires.tests/secret-redaction.test.mjs:redactTail(text, n) === redact(text).slice(-n)for 14 secret kinds (custom value, wrapped value, tokens, JSON, assignment, URL, bearer, closed and open PEM) at 8 offsets around both the tail cut and the window start, 3 tail sizes (672 cases); the regexes see at most ~28 K chars; observe/result/plugin screen stay under 40 K per callProviderLaunch.ts:379spawnSync("kimi --help")on the main process, up to 3 s.warmKimiProbe()runs the same probe withexecFile(same command, env, limits, answer) 5 s after startup and after a CLI recheck; the launch uses that answer. A launch before it finishes still probes as before.tests/agent-browser-provider-launch.test.mjs: warmed answer used with no blocking probe, recheck discards it; async and sync probes agree on three stand-in CLIsApp.tsx:204,WorkspaceCanvas.tsx:827-859,TerminalCard.tsx:107memo).TerminalCardisReact.memo(snap targets compared by value); the workspace hands it callbacks that stay the same functions and call its latest handlers through a ref.tests/canvas-card-render-cost.test.mjs(equality rules; memo and stable callbacks in source); benchmark: TerminalCard renders per pan move 7.9 → 0electron.vite.config.tsaudit:secretspasses onout/.tests/renderer-build-config.test.mjsProviderIcon.tsx:7hermes.pngwas the 1024 px original (574 KB), drawn at 24-125 CSS px.codex.png(544 px, 146 KB) likewise.srcset, scaled withsips -Zonly; assets README says so.tests/provider-icon-assets.test.mjscompanion/TerminalPresentation.ts:41-65,86-108@xterm/headlessscreen on its first event and parsed all its output.tests/companion-presentation.test.mjs: only read sessions are parsed; a late first read equals a live-parsed screenNot changed, on purpose:
Benchmark
scripts/bench-runtime.mjs(docs:docs/performance-benchmark.md) runs the built app in hidden, off-screen, unfocusable windows with a throw-away HOME, keychain andsafeStorageoff, and micro-benchmarks the hot paths in plain Node on fake PTYs.Machine: macOS 26.6.2, Apple M1 Max (10 cores), 64 GB RAM, Node 23.3.0, Electron 43.2.0. 3 runs each, medians. BEFORE =
origin/main+ the benchmark commit, AFTER = this branch. App scenarios: 8 plain terminal cards, flood = 1 MB/s of coloured log lines per card for 20 s (8 MB/s total).observe_agentcall, 20 cards with full scrollbackobserve_agentalone /list_agents--helpin 1 sWhere to expect it:
Reproduce:
npx electron-vite build node scripts/bench-runtime.mjs --runs 3 --json bench.json # add --pan-only or --micro-only for partsNot in this PR: native helpers
Every hook, permission gate and MCP helper call starts Electron as Node. A Go prototype of
hook-helper.mjs(same argv, env, stdin, socket message and exit code; message checked identical against a stand-in gateway), 40 runs each on the machine above:hook-helper.mjsA helper runs on every lifecycle event and every gated tool call, so this is the largest remaining per-event cost. It needs a build and signing story per platform; to be discussed in an issue first.
Observed, not addressed
libc++abi: terminating due to uncaught exception of type Napi::Errorat quit after the flood (4 of 6 runs), after the report was complete. Looks like node-pty during shutdown; worth its own issue.redact()then throws. Pre-existing; not changed here.SettingsPanel,HomeZoneand their icons (about 48 per move). Memoizing them needs the same stable-callback treatment inApp.tsx.Checks
Typecheck, full suite 1028/1028,
electron-vite build,audit:secrets,test:even47/47, all with a fake HOME. Every commit typechecks and passes the tests it touches. Benchmarks ran in hidden, unfocused windows with keychain access refused.Fixes for two bugs the benchmark found
Two commits on top of the performance work, each with its own regression tests.
Crash on quit after an output flood (
fix(terminal): wait for every PTY to exit before the app finishes quitting)Symptom. After the flood scenario, quitting sometimes aborted with
libc++abi: terminating due to uncaught exception of type Napi::Error(SIGABRT). This also happens on main. The crash report shows node-pty's exitThreadSafeFunction::CallJSrunning insidenode::FreeEnvironment, while other pty exit watchers were still waiting inkevent.Root cause.
TerminalManager.shutdown()(src/main/services/TerminalManager.ts,disposeAll→session.process.kill()) sends SIGHUP to every shell and returns straight away.shutdownServices()insrc/main/index.tsthen lets the app quit. A shell that exits while Electron is freeing the Node environment makes node-pty's native exit callback call into JavaScript that can no longer run. node-pty rethrows that failure as a C++ exception, which aborts the app.Fix.
TerminalManagernow keeps track of every PTY until its exit is reported. The newwaitForProcessExits()waits up to 2 s for those exits, sends SIGKILL to any process still running, and then waits up to 1 s more.shutdownServices()starts this wait right after the terminal shutdown and awaits it before quitting finishes. The exit handler is also wrapped so that no exception can reach node-pty's native callback, where a throw would abort the app too.Tests.
tests/terminal-quit-exits.test.mjsuses fake PTYs. It checks that the wait lasts until the last exit, that a process ignoring SIGHUP gets SIGKILL, that the wait is bounded, that a card closed just before quitting is still waited for, that a throwing exit handler is contained, and that the quit path awaits the wait. All six tests fail without the fix.Live evidence. Each run used a hidden app with 8 cards, a 1 MB/s flood, and
app.quit()with no forced exit.Napi::Error, and all 3 of 3 benchmark runs (--app-only --runs 3) did.~/Library/Logs/DiagnosticReportsduring these 23 runs. Quitting takes about 100 ms longer.Long stored secrets broke redaction (
fix(redaction): find held values with a linear search, not a pattern built from them)Symptom. Once a stored secret longer than about 3,700 characters was held, every redaction call threw
SyntaxError: Invalid regular expression. That broke agent observe/result, plugin screens and failure details. A secret longer than 4,096 characters was silently not masked at all.Root cause.
SecretRedactionRegistry.knownPattern()(src/main/services/safety/SecretRedaction.ts) built one regular expression from each value, with a[\s─-╿]{0,64}wrap-gap class between every two characters. For long values the pattern grew beyond V8's limit.Fix. Held values are no longer turned into a pattern. The text is searched with its wrap characters taken out, using Knuth-Morris-Pratt for each value, and a position map leads back to the original text. The 64-character gap limit and the "leftmost match, longest first" behaviour stay the same. The cost is linear in the text plus the value, so the maximum length of a held value rises to 65,536 characters.
redactTailstill masks a window at least as wide as the longest possible match, so the guarantee to mask the full text before cutting it still holds.Tests. New cases in
tests/secret-redaction.test.mjsall fail on the previous code:redactTailequals masking the whole text and cutting it, for 4k and 16k values placed across the tail cut and the window cut, with no fragment survivingA plain
redact()of a 240k scrollback holding 10 values takes about 14 ms, compared with 12 ms before.Gates on the branch: typecheck, full suite (1038/1038),
electron-vite build,audit:secrets,test:even(47/47).