From 149732df6df37ddc17d90a2cd4aa691358c46b1b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 4 Sep 2026 23:39:02 -0700 Subject: [PATCH] perf(persistence): stop the session write re-scanning and rebuilding unchanged state (#18739) --- .../session-write-hot-path-benchmark.mjs | 221 ++++++++++++++++++ package.json | 1 + .../pane-key-remapping.test.ts | 13 ++ .../restoring-sessions/pane-key-remapping.ts | 69 +++--- src/shared/utf8-byte-limits.test.ts | 31 +++ src/shared/utf8-byte-limits.ts | 14 ++ .../workspace-session-terminal-buffers.ts | 9 +- 7 files changed, 316 insertions(+), 42 deletions(-) create mode 100644 config/scripts/session-write-hot-path-benchmark.mjs diff --git a/config/scripts/session-write-hot-path-benchmark.mjs b/config/scripts/session-write-hot-path-benchmark.mjs new file mode 100644 index 00000000000..361e7eb7098 --- /dev/null +++ b/config/scripts/session-write-hot-path-benchmark.mjs @@ -0,0 +1,221 @@ +#!/usr/bin/env node +// Benchmarks two CPU costs `setLocalWorkspaceSession` pays on every session write — the write +// that fires on something as ordinary as clicking between two terminal split panes. +// +// 1. capTerminalScrollbackSessionBuffer — UTF-8 budget scan per retained scrollback buffer +// 2. remapPaneKeys — pane-key map rebuild that steady state throws away +// +// The snapshot disk rewrite on the same path is measured separately (#18764). +// +// Each scenario runs the production export against a baseline that reproduces the pre-change +// shape, so the reported speedup cannot drift away from what production actually does. +import { spawnSync } from 'node:child_process' +import { performance } from 'node:perf_hooks' +import fs from 'node:fs' +import nodeModule from 'node:module' +import path from 'node:path' +import process from 'node:process' +import { fileURLToPath } from 'node:url' + +if (!process.execArgv.includes('--experimental-transform-types')) { + const result = spawnSync( + process.execPath, + ['--experimental-transform-types', '--no-warnings', import.meta.filename], + { stdio: 'inherit' } + ) + process.exit(result.status ?? 1) +} + +// The app's TS sources import siblings without an extension; Node's ESM resolver needs it. +nodeModule.registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier.startsWith('.') && !/\.[cm]?[jt]s$/.test(specifier) && context.parentURL) { + const candidate = new URL(`${specifier}.ts`, context.parentURL) + if (fs.existsSync(fileURLToPath(candidate))) { + return { url: candidate.href, shortCircuit: true } + } + } + return nextResolve(specifier, context) + } +}) + +const ROOT = path.resolve(import.meta.dirname, '../..') +const ROUNDS = Number(process.env.ORCA_SESSION_WRITE_BENCH_ROUNDS ?? '9') +const LEAVES = Number(process.env.ORCA_SESSION_WRITE_BENCH_LEAVES ?? '8') +const PANE_KEYS = Number(process.env.ORCA_SESSION_WRITE_BENCH_PANE_KEYS ?? '2000') + +for (const [name, value] of [ + ['ORCA_SESSION_WRITE_BENCH_ROUNDS', ROUNDS], + ['ORCA_SESSION_WRITE_BENCH_LEAVES', LEAVES], + ['ORCA_SESSION_WRITE_BENCH_PANE_KEYS', PANE_KEYS] +]) { + if (!Number.isSafeInteger(value) || value <= 0) { + throw new Error(`${name} must be a positive integer, got ${value}`) + } +} + +const { capTerminalScrollbackSessionBuffer } = await import( + path.join(ROOT, 'src/shared/workspace-session-terminal-buffers.ts') +) +const { TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT } = await import( + path.join(ROOT, 'src/shared/terminal-scrollback-limits.ts') +) +const { remapAcknowledgedAgentPaneKeys } = await import( + path.join(ROOT, 'src/main/persistence/restoring-sessions/pane-key-remapping.ts') +) +const { clampUtf8TextTail, measureUtf8ByteLength } = await import( + path.join(ROOT, 'src/shared/utf8-byte-limits.ts') +) +const { isTerminalLeafId, makePaneKey, parsePaneKey } = await import( + path.join(ROOT, 'src/shared/stable-pane-id.ts') +) + +function median(samples) { + const sorted = [...samples].sort((left, right) => left - right) + return sorted[Math.floor(sorted.length / 2)] +} + +function timeRounds(run) { + const samples = [] + run() + for (let round = 0; round < ROUNDS; round += 1) { + const start = performance.now() + run() + samples.push(performance.now() - start) + } + return median(samples) +} + +function report(label, baselineMs, currentMs, extra = '') { + const speedup = baselineMs / currentMs + console.log( + `${label}\n before ${baselineMs.toFixed(3)} ms → after ${currentMs.toFixed(3)} ms (${speedup.toFixed(1)}x)${extra}` + ) + return speedup +} + +// ---------------------------------------------------------------- scenario 1 + +// Verbatim pre-change capTerminalScrollbackSessionBuffer; measureUtf8ByteLength itself is unchanged. +function baselineCapScrollbackBuffer(buffer) { + if ( + buffer.length <= TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT && + !measureUtf8ByteLength(buffer, { + stopAfterBytes: TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT + }).exceededLimit + ) { + return buffer + } + return clampUtf8TextTail(buffer, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT).text +} + +// A terminal that has been running a while sits at the cap, which is the case that scanned in full. +const scrollbackLine = `${''}build output line with a path /Users/dev/project/src/index.ts and a status ok\n` +let atCapBuffer = '' +while (atCapBuffer.length < TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT) { + atCapBuffer += scrollbackLine +} +atCapBuffer = atCapBuffer.slice(0, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT) + +if (capTerminalScrollbackSessionBuffer(atCapBuffer) !== baselineCapScrollbackBuffer(atCapBuffer)) { + throw new Error('scrollback cap disagreed with the baseline implementation') +} + +// The session write runs the prune twice, once per retained leaf. +const CAP_CALLS_PER_WRITE = LEAVES * 2 +const capBaselineMs = timeRounds(() => { + for (let call = 0; call < CAP_CALLS_PER_WRITE; call += 1) { + baselineCapScrollbackBuffer(atCapBuffer) + } +}) +const capCurrentMs = timeRounds(() => { + for (let call = 0; call < CAP_CALLS_PER_WRITE; call += 1) { + capTerminalScrollbackSessionBuffer(atCapBuffer) + } +}) + +console.log( + `Session-write hot path — ${LEAVES} retained scrollback leaves, ${PANE_KEYS} accumulated pane keys\n` +) +report( + `1. scrollback UTF-8 budget scan (${CAP_CALLS_PER_WRITE} calls/write @ ${(atCapBuffer.length / 1024).toFixed(0)} KB)`, + capBaselineMs, + capCurrentMs +) + +// ---------------------------------------------------------------- scenario 2 + +const paneKeys = {} +const leafIdByInputLeafIdByTabId = new Map() +for (let index = 0; index < PANE_KEYS; index += 1) { + const tabId = `tab-${index % 64}` + const leafId = `${(index % 64).toString(16).padStart(8, '0')}-0000-4000-8000-${index.toString(16).padStart(12, '0')}` + paneKeys[makePaneKey(tabId, leafId)] = index + let leaves = leafIdByInputLeafIdByTabId.get(tabId) + if (!leaves) { + leaves = new Map() + leafIdByInputLeafIdByTabId.set(tabId, leaves) + } + // Steady state: a stable UUID leaf maps to itself. + leaves.set(leafId, leafId) +} + +// Verbatim pre-change remapPaneKeys: parses every key, then rebuilds the object regardless. +function baselineRemapPaneKeys(values, remap) { + if (!values || Object.keys(values).length === 0) { + return { values, changed: false } + } + let changed = false + const next = {} + const setValue = (paneKey, value) => { + const existing = next[paneKey] + next[paneKey] = existing === undefined ? value : Math.max(existing, value) + } + for (const [paneKey, value] of Object.entries(values)) { + if (parsePaneKey(paneKey)) { + setValue(paneKey, value) + continue + } + const delimiter = paneKey.indexOf(':') + if (delimiter <= 0 || delimiter === paneKey.length - 1) { + setValue(paneKey, value) + continue + } + const tabId = paneKey.slice(0, delimiter) + const remappedLeafId = remap.get(tabId)?.get(paneKey.slice(delimiter + 1)) + if (!remappedLeafId || !isTerminalLeafId(remappedLeafId)) { + setValue(paneKey, value) + continue + } + try { + setValue(makePaneKey(tabId, remappedLeafId), value) + changed = true + } catch { + setValue(paneKey, value) + } + } + return { values: next, changed } +} + +// The write remaps three of these maps: acknowledgements, activity cutoffs, manual unread. +const REMAP_CALLS_PER_WRITE = 3 +const remapBaselineMs = timeRounds(() => { + for (let call = 0; call < REMAP_CALLS_PER_WRITE; call += 1) { + baselineRemapPaneKeys(paneKeys, leafIdByInputLeafIdByTabId) + } +}) +const remapCurrentMs = timeRounds(() => { + for (let call = 0; call < REMAP_CALLS_PER_WRITE; call += 1) { + remapAcknowledgedAgentPaneKeys(paneKeys, leafIdByInputLeafIdByTabId) + } +}) +const remapResult = remapAcknowledgedAgentPaneKeys(paneKeys, leafIdByInputLeafIdByTabId) +if (remapResult.changed || remapResult.acknowledgements !== paneKeys) { + throw new Error('steady-state remap should return the input map untouched') +} +report( + `2. pane-key remap (${REMAP_CALLS_PER_WRITE} maps/write @ ${PANE_KEYS} keys)`, + remapBaselineMs, + remapCurrentMs, + ' — and 3 discarded objects/write become 0' +) diff --git a/package.json b/package.json index c519d7ea6b1..24a5ade2c7c 100644 --- a/package.json +++ b/package.json @@ -141,6 +141,7 @@ "bench:main-thread-jank": "pnpm run ensure:electron-runtime && node tests/tools/benchmarks/main-thread-jank-bench.mjs", "bench:worktree-deletion": "node tests/tools/benchmarks/worktree-deletion-dev-bench.mjs", "bench:zustand-selector-fanout": "node config/scripts/zustand-selector-fanout-benchmark.mjs", + "bench:session-write-hot-path": "node config/scripts/session-write-hot-path-benchmark.mjs", "bench:worktree-refresh-churn": "node --disable-warning=MODULE_TYPELESS_PACKAGE_JSON config/scripts/worktree-refresh-churn-benchmark.mjs", "bench:multi-workspace-typing": "pnpm run ensure:electron-runtime && node config/scripts/run-multi-workspace-typing-bench.mjs", "bench:ai-vault-typing": "pnpm run ensure:electron-runtime && node config/scripts/run-ai-vault-typing-bench.mjs", diff --git a/src/main/persistence/restoring-sessions/pane-key-remapping.test.ts b/src/main/persistence/restoring-sessions/pane-key-remapping.test.ts index c0359efd612..2c73045d901 100644 --- a/src/main/persistence/restoring-sessions/pane-key-remapping.test.ts +++ b/src/main/persistence/restoring-sessions/pane-key-remapping.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { makePaneKey } from '../../../shared/stable-pane-id' import { + remapAcknowledgedAgentPaneKeys, remapActivityClearedAtPaneKeys, remapManuallyUnreadTurnPaneKeys } from './pane-key-remapping' @@ -30,4 +31,16 @@ describe('remapManuallyUnreadTurnPaneKeys', () => { changed: false }) }) + + it('returns the caller map untouched when no key needs remapping', () => { + const remap = new Map([['tab-1', new Map([[STABLE_LEAF_ID, STABLE_LEAF_ID]])]]) + const stable = { [makePaneKey('tab-1', STABLE_LEAF_ID)]: 1 } + + const result = remapAcknowledgedAgentPaneKeys(stable, remap) + + // Why identity and not just equality: this runs on every session write against a map that + // grows with every pane ever opened, so a rebuilt-then-discarded copy is pure garbage. + expect(result.acknowledgements).toBe(stable) + expect(result.changed).toBe(false) + }) }) diff --git a/src/main/persistence/restoring-sessions/pane-key-remapping.ts b/src/main/persistence/restoring-sessions/pane-key-remapping.ts index 4f3440f087e..8e43184b0c5 100644 --- a/src/main/persistence/restoring-sessions/pane-key-remapping.ts +++ b/src/main/persistence/restoring-sessions/pane-key-remapping.ts @@ -3,51 +3,50 @@ import { isTerminalLeafId, makePaneKey, parsePaneKey } from '../../../shared/sta type PaneLeafRemap = Map> +/** Resolves the pane key a legacy entry should move to, or `null` when it stays put. */ +function resolveRemappedPaneKey( + paneKey: string, + leafIdByInputLeafIdByTabId: PaneLeafRemap +): string | null { + if (parsePaneKey(paneKey)) { + return null + } + const delimiter = paneKey.indexOf(':') + if (delimiter <= 0 || delimiter === paneKey.length - 1) { + return null + } + const tabId = paneKey.slice(0, delimiter) + const remappedLeafId = leafIdByInputLeafIdByTabId.get(tabId)?.get(paneKey.slice(delimiter + 1)) + // makePaneKey cannot throw here: tabId is non-empty and colon-free by construction. + return remappedLeafId && isTerminalLeafId(remappedLeafId) + ? makePaneKey(tabId, remappedLeafId) + : null +} + function remapPaneKeys( values: Record | undefined, leafIdByInputLeafIdByTabId: PaneLeafRemap ): { values: Record | undefined; changed: boolean } { - if (!values || Object.keys(values).length === 0) { + // Why the classify-first pass: these maps grow with every pane ever opened and this runs on + // every session write, but post-migration no key is ever rewritten. Rebuilding the whole + // object only to discard it was pure garbage; the rewrite below is unchanged. + if ( + !values || + !Object.keys(values).some( + (paneKey) => resolveRemappedPaneKey(paneKey, leafIdByInputLeafIdByTabId) !== null + ) + ) { return { values, changed: false } } - let changed = false const next: Record = {} - const setValue = (paneKey: string, value: T): void => { - const existing = next[paneKey] - next[paneKey] = existing === undefined ? value : (Math.max(existing, value) as T) - } for (const [paneKey, value] of Object.entries(values)) { - const parsed = parsePaneKey(paneKey) - if (parsed) { - setValue(paneKey, value) - continue - } - - const delimiter = paneKey.indexOf(':') - if (delimiter <= 0 || delimiter === paneKey.length - 1) { - setValue(paneKey, value) - continue - } - - const tabId = paneKey.slice(0, delimiter) - const legacyLeafId = paneKey.slice(delimiter + 1) - const remappedLeafId = leafIdByInputLeafIdByTabId.get(tabId)?.get(legacyLeafId) - if (!remappedLeafId || !isTerminalLeafId(remappedLeafId)) { - setValue(paneKey, value) - continue - } - - try { - // Carry values over when a legacy leaf is promoted to a UUID. - setValue(makePaneKey(tabId, remappedLeafId), value) - changed = true - } catch { - setValue(paneKey, value) - } + // Carry values over when a legacy leaf is promoted to a UUID; keep the max on collision. + const target = resolveRemappedPaneKey(paneKey, leafIdByInputLeafIdByTabId) ?? paneKey + const existing = next[target] + next[target] = existing === undefined ? value : (Math.max(existing, value) as T) } - - return { values: next, changed } + return { values: next, changed: true } } export function remapAcknowledgedAgentPaneKeys( diff --git a/src/shared/utf8-byte-limits.test.ts b/src/shared/utf8-byte-limits.test.ts index 9556a5c39bc..f5cd2c83ed8 100644 --- a/src/shared/utf8-byte-limits.test.ts +++ b/src/shared/utf8-byte-limits.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { clampUtf8TextPrefix, + getUtf8ByteLength, getUtf8ChunkEndIndex, isUtf8ByteLengthWithinLimit, measureUtf8ByteLength, @@ -129,3 +130,33 @@ describe('isUtf8ByteLengthWithinLimit', () => { expect(isUtf8ByteLengthWithinLimit(text, maxBytes)).toBe(expected) }) }) + +describe('isUtf8ByteLengthWithinLimit native fast path', () => { + // The bounded check runs through TextEncoder.encodeInto; it must agree with the scan it replaced + // for every boundary shape, lone surrogates included. + const samples = [ + '', + 'a', + 'ascii only text', + 'caf\u00e9', + '\u20ac\u20ac\u20ac', + '\ud83d\ude00\ud83d\ude00', + '\ud83d', + '\ude00', + 'mixed \u00e9 \u20ac \ud83d\ude00 tail', + 'x'.repeat(64) + ] + + it('matches the scanning implementation at every limit around the boundary', () => { + for (const text of samples) { + const exactBytes = getUtf8ByteLength(text) + for (let maxBytes = 1; maxBytes <= exactBytes + 2; maxBytes += 1) { + expect({ text, maxBytes, within: isUtf8ByteLengthWithinLimit(text, maxBytes) }).toEqual({ + text, + maxBytes, + within: text.length <= maxBytes && exactBytes <= maxBytes + }) + } + } + }) +}) diff --git a/src/shared/utf8-byte-limits.ts b/src/shared/utf8-byte-limits.ts index df8f99f68ad..767d5bfb877 100644 --- a/src/shared/utf8-byte-limits.ts +++ b/src/shared/utf8-byte-limits.ts @@ -51,6 +51,14 @@ export function getUtf8ByteLength(text: string): number { return measureUtf8ByteLength(text).byteLength } +// Why a native encode: the per-code-unit JS scan walks whole terminal scrollback buffers on the +// session-write path. `encodeInto` answers "does this fit in maxBytes?" in C++ — it stops at the +// destination's end, so `read < text.length` means the text needs more than maxBytes. The scratch +// buffer is reused across calls and grows to the largest limit asked for, up to this cap. +const MAX_UTF8_SCRATCH_BYTES = 1024 * 1024 +const utf8Encoder = new TextEncoder() +let utf8Scratch = new Uint8Array(0) + export function isUtf8ByteLengthWithinLimit(text: string, maxBytes: number): boolean { if (text.length === 0) { return true @@ -58,6 +66,12 @@ export function isUtf8ByteLengthWithinLimit(text: string, maxBytes: number): boo if (text.length > maxBytes) { return false } + if (Number.isSafeInteger(maxBytes) && maxBytes <= MAX_UTF8_SCRATCH_BYTES) { + if (utf8Scratch.length < maxBytes) { + utf8Scratch = new Uint8Array(maxBytes) + } + return utf8Encoder.encodeInto(text, utf8Scratch.subarray(0, maxBytes)).read === text.length + } return !measureUtf8ByteLength(text, { stopAfterBytes: maxBytes }).exceededLimit } diff --git a/src/shared/workspace-session-terminal-buffers.ts b/src/shared/workspace-session-terminal-buffers.ts index 96bb76cafe2..ef8ac6a273b 100644 --- a/src/shared/workspace-session-terminal-buffers.ts +++ b/src/shared/workspace-session-terminal-buffers.ts @@ -3,7 +3,7 @@ import type { WorkspaceSessionState } from './workspace-session-state-types' import { FLOATING_TERMINAL_WORKTREE_ID } from './constants' import { getRepoIdFromWorktreeId } from './worktree/id' import { TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT } from './terminal-scrollback-limits' -import { clampUtf8TextTail, measureUtf8ByteLength } from './utf8-byte-limits' +import { clampUtf8TextTail, isUtf8ByteLengthWithinLimit } from './utf8-byte-limits' import { parseExecutionHostId } from './execution-host' export type RepoConnection = Pick @@ -50,12 +50,7 @@ export function shouldPreserveTerminalScrollbackBuffers( } export function capTerminalScrollbackSessionBuffer(buffer: string): string { - if ( - buffer.length <= TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT && - !measureUtf8ByteLength(buffer, { - stopAfterBytes: TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT - }).exceededLimit - ) { + if (isUtf8ByteLengthWithinLimit(buffer, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT)) { return buffer } return clampUtf8TextTail(buffer, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT).text