diff --git a/config/scripts/hydrate-worktree-lookup-benchmark.mjs b/config/scripts/hydrate-worktree-lookup-benchmark.mjs new file mode 100644 index 000000000..76b737501 --- /dev/null +++ b/config/scripts/hydrate-worktree-lookup-benchmark.mjs @@ -0,0 +1,178 @@ +#!/usr/bin/env node +// Benchmark: the per-id worktree/tab lookups in session hydration and terminal reconnect. +// +// Four sites in store/slices/terminals.ts re-flattened worktreesByRepo (or tabsByWorktree) +// and linearly searched it once per loop iteration -- O(rows x ids) for O(rows + ids) +// distinct work. The fix builds one first-wins index per loop. +// +// This runs on the renderer's synchronous cold-start path and gates workspaceSessionReady, +// which blocks terminal pane mounting, so the cost is paid before the first frame. +// +// Both arms produce the resolved rows and are compared for equality before timing, so an +// index that resolved differently could not be reported as a win. +// +// Run with: node config/scripts/hydrate-worktree-lookup-benchmark.mjs +import { performance } from 'node:perf_hooks' + +const ITERATIONS = Number(process.env.ORCA_HYDRATE_BENCH_ITERATIONS ?? '60') +const WARMUP = Number(process.env.ORCA_HYDRATE_BENCH_WARMUP ?? '10') +const ROUNDS = 6 + +for (const [name, value] of [ + ['ORCA_HYDRATE_BENCH_ITERATIONS', ITERATIONS], + ['ORCA_HYDRATE_BENCH_WARMUP', WARMUP] +]) { + if (!Number.isSafeInteger(value) || value <= 0) { + throw new Error(`${name} must be a positive integer, received ${value}`) + } +} + +// Pre-fix: re-flatten and linear-search per id. +function resolveByFlatten(worktreesByRepo, ids) { + const resolved = [] + for (const id of ids) { + const worktree = Object.values(worktreesByRepo) + .flat() + .find((entry) => entry.id === id) + resolved.push(worktree ? worktree.repoId : null) + } + return resolved +} + +// Post-fix: mirrors buildWorktreeByIdIndex in store/slices/worktree-by-id-index.ts. +function resolveByIndex(worktreesByRepo, ids) { + const index = new Map() + for (const worktrees of Object.values(worktreesByRepo)) { + for (const worktree of worktrees) { + if (!index.has(worktree.id)) { + index.set(worktree.id, worktree) + } + } + } + const resolved = [] + for (const id of ids) { + const worktree = index.get(id) + resolved.push(worktree ? worktree.repoId : null) + } + return resolved +} + +function makeStore(repoCount, worktreesPerRepo) { + const worktreesByRepo = {} + for (let repo = 0; repo < repoCount; repo += 1) { + const repoId = `repo-${repo}` + worktreesByRepo[repoId] = Array.from({ length: worktreesPerRepo }, (_value, index) => ({ + id: `${repoId}/wt-${index}`, + repoId, + path: `/Users/dev/worktrees/${repoId}/wt-${index}`, + branch: `feature/branch-${index}` + })) + } + // Why a deliberate duplicate: `.find()` is first-wins, so an index that overwrote on + // collision would resolve a different repo. Without a collision in the fixture that + // difference is unobservable and the equality check below would pass a broken index. + if (repoCount > 1) { + const [firstRepo, secondRepo] = Object.keys(worktreesByRepo) + worktreesByRepo[secondRepo] = [ + { ...worktreesByRepo[firstRepo][0], repoId: secondRepo }, + ...worktreesByRepo[secondRepo] + ] + } + return worktreesByRepo +} + +// Why a miss fraction: SSH worktrees are absent from worktreesByRepo at cold start, so +// the real workload includes ids that scan the whole list without matching -- the worst +// case for the linear arm, and the one the code comments call out explicitly. +function makeIds(worktreesByRepo, count) { + const all = Object.values(worktreesByRepo).flat() + const ids = Array.from({ length: count }, (_value, index) => + index % 7 === 0 ? `absent/wt-${index}` : all[(index * 31) % all.length].id + ) + // Always look up the duplicated id, so first-wins is exercised, not just present. + ids[1] = all[0].id + return ids +} + +function timeArm(resolve, worktreesByRepo, ids) { + let sink = 0 + const start = performance.now() + for (let index = 0; index < ITERATIONS; index += 1) { + // Consume the result so V8 cannot drop the call as dead. + sink += resolve(worktreesByRepo, ids).length + } + const elapsed = (performance.now() - start) / ITERATIONS + if (sink === -1) { + throw new Error('unreachable') + } + return elapsed +} + +function median(samples) { + const sorted = [...samples].sort((a, b) => a - b) + const mid = sorted.length / 2 + return (sorted[mid - 1] + sorted[mid]) / 2 +} + +// Arms alternate which one leads so within-round drift cannot favour either. +function measure(worktreesByRepo, ids) { + for (let index = 0; index < WARMUP; index += 1) { + resolveByFlatten(worktreesByRepo, ids) + resolveByIndex(worktreesByRepo, ids) + } + const flattenSamples = [] + const indexSamples = [] + for (let round = 0; round < ROUNDS; round += 1) { + if (round % 2 === 0) { + flattenSamples.push(timeArm(resolveByFlatten, worktreesByRepo, ids)) + indexSamples.push(timeArm(resolveByIndex, worktreesByRepo, ids)) + } else { + indexSamples.push(timeArm(resolveByIndex, worktreesByRepo, ids)) + flattenSamples.push(timeArm(resolveByFlatten, worktreesByRepo, ids)) + } + } + return { flattenMs: median(flattenSamples), indexMs: median(indexSamples) } +} + +const pad = (value, width) => String(value).padStart(width) +console.log('Session-hydration worktree lookup, per cold start. Lower is better.') +console.log(`iterations=${ITERATIONS} warmup=${WARMUP} rounds=${ROUNDS} (per-arm medians)`) +console.log( + `${pad('repos', 6)} ${pad('worktrees', 10)} ${pad('ids', 5)} ${pad('flatten', 11)} ${pad('indexed', 11)} ${pad('speedup', 9)}` +) + +// Row shapes are synthetic, sized against a real orca-data.json on a heavy machine +// (10 repos / 423 worktrees / 188 pending reconnect). They are not that dataset: the +// generator spreads worktrees evenly and injects one duplicate id, so treat the counts +// as "about this scale", not a replay. +for (const [repoCount, worktreesPerRepo, idCount] of [ + [1, 5, 5], + [3, 20, 20], + [10, 42, 188], + [10, 100, 400] +]) { + const worktreesByRepo = makeStore(repoCount, worktreesPerRepo) + const ids = makeIds(worktreesByRepo, idCount) + const flattenResult = resolveByFlatten(worktreesByRepo, ids) + const indexResult = resolveByIndex(worktreesByRepo, ids) + if (JSON.stringify(flattenResult) !== JSON.stringify(indexResult)) { + throw new Error(`resolution differs at ${repoCount} repos x ${worktreesPerRepo} worktrees`) + } + if (!flattenResult.some((value) => value !== null)) { + throw new Error(`fixture resolved nothing at ${repoCount} repos`) + } + if (!flattenResult.some((value) => value === null)) { + throw new Error(`fixture had no absent ids at ${repoCount} repos`) + } + // Count the generated rows rather than multiplying: makeStore injects a duplicate + // id for multi-repo cases, so the product would misreport the fixture by one. + const worktreeCount = Object.values(worktreesByRepo).reduce((sum, rows) => sum + rows.length, 0) + const { flattenMs, indexMs } = measure(worktreesByRepo, ids) + console.log( + `${pad(repoCount, 6)} ${pad(worktreeCount, 10)} ${pad(idCount, 5)} ${pad(`${flattenMs.toFixed(4)} ms`, 11)} ${pad(`${indexMs.toFixed(4)} ms`, 11)} ${pad(`${(flattenMs / indexMs).toFixed(1)}x`, 9)}` + ) +} + +console.log( + '\nFixtures are synthetic at real-world scale, not a replay of a real session.\nThis times one of the four lookup sites. A one-repo session sees almost nothing;\nthe win scales with worktrees x pending ids, and lands on the cold-start path that\ngates terminal pane mounting.' +) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 5c8806185..a0cfcea98 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -34,6 +34,7 @@ import { parsePaneKey } from '../../../../shared/stable-pane-id' import { isValidHostTerminalTabId, isValidTerminalTabId } from '../../../../shared/terminal-tab-id' +import { buildByIdIndex, buildWorktreeByIdIndex } from './worktree-by-id-index' import { getRepoIdFromWorktreeId, splitWorktreeIdForFilesystem @@ -3210,13 +3211,13 @@ export const createTerminalSlice: StateCreator // Why: preserve each tab's prior ptyId so reconnect passes it as sessionId to the daemon's createOrAttach, triggering reattach instead of a fresh spawn. const pendingReconnectPtyIdByTabId: Record = {} + const placeholderWorktreeById = buildWorktreeByIdIndex( + runtimeSessionPlaceholders.worktreesByRepo + ) + const placeholderRepoById = buildByIdIndex(runtimeSessionPlaceholders.repos) for (const worktreeId of pendingReconnectWorktreeIds) { - const worktree = Object.values(runtimeSessionPlaceholders.worktreesByRepo) - .flat() - .find((entry) => entry.id === worktreeId) - const repo = worktree - ? runtimeSessionPlaceholders.repos.find((entry) => entry.id === worktree.repoId) - : null + const worktree = placeholderWorktreeById.get(worktreeId) + const repo = worktree ? placeholderRepoById.get(worktree.repoId) : null if (repo?.connectionId) { continue } @@ -3304,6 +3305,11 @@ export const createTerminalSlice: StateCreator nextEverActivated.add(activeWorktreeId) } + // Why indexed: the layout map below looks up a tab per persisted layout, and + // re-flattening tabsByWorktree per entry is O(tabs x layouts). + const allTabs = Object.values(tabsByWorktree).flat() + const tabById = buildByIdIndex(allTabs) + return { activeRepoId, activeWorktreeId, @@ -3328,11 +3334,7 @@ export const createTerminalSlice: StateCreator // Why: seed nav history with the hydrated active worktree so the first activation has a Back target; hydration bypasses recordWorktreeVisit, so otherwise Back stays disabled until a second click. worktreeNavHistory: activeWorktreeId ? [activeWorktreeId] : [], worktreeNavHistoryIndex: activeWorktreeId ? 0 : -1, - ptyIdsByTabId: Object.fromEntries( - Object.values(tabsByWorktree) - .flat() - .map((tab) => [tab.id, []] as const) - ), + ptyIdsByTabId: Object.fromEntries(allTabs.map((tab) => [tab.id, []] as const)), // Why: daemon ptyIds survive app restart; preserve ptyIdsByLeafId so reconnect can reattach each split-pane leaf to its own session, not just the tab-level ptyId. terminalLayoutsByTabId: Object.fromEntries( Object.entries(session.terminalLayoutsByTabId) @@ -3340,9 +3342,7 @@ export const createTerminalSlice: StateCreator .map(([tabId, layout]) => { // Why: old sessions can contain renderer-local pane:1-style leaf ids; normalize before runtime/mobile surfaces read them. const normalized = normalizeTerminalLayoutSnapshot(layout).snapshot - const tab = Object.values(tabsByWorktree) - .flat() - .find((entry) => entry.id === tabId) + const tab = tabById.get(tabId) const sanitized = tab ? sanitizeTerminalLayoutPaneTitles(normalized, tab) : normalized const activeLeafId = sanitized.root ? resolvePtyBoundActiveLeafId({ @@ -3382,12 +3382,14 @@ export const createTerminalSlice: StateCreator // Why: defer daemon createOrAttach to connectPanePty (real fitAddon dims) instead of eager-spawning at 80×24 and garbling on flush; this loop only records the session IDs to reattach. let reconnectedTabsByWorktree: Record | null = null let reconnectedPtyIdsByTabId: Record | null = null + // Why indexed: the loop neither sets state nor awaits, so one index over the + // whole store snapshot serves every iteration. + const worktreeById = buildWorktreeByIdIndex(get().worktreesByRepo) + const repoById = buildByIdIndex(get().repos) for (const worktreeId of ids) { const tabs = tabsByWorktree[worktreeId] ?? [] - const worktree = Object.values(get().worktreesByRepo) - .flat() - .find((entry) => entry.id === worktreeId) - const repo = worktree ? get().repos.find((entry) => entry.id === worktree.repoId) : null + const worktree = worktreeById.get(worktreeId) + const repo = worktree ? (repoById.get(worktree.repoId) ?? null) : null // Why: only allow deferred reattach when the SSH connection is active; reattaching to a not-yet-connected relay (deferred/passphrase targets) would fail. const sshState = repo?.connectionId ? get().sshConnectionStates.get(repo.connectionId) : null const sshConnected = repo?.connectionId != null && sshState?.status === 'connected' @@ -3441,12 +3443,10 @@ export const createTerminalSlice: StateCreator // Why: deferred SSH targets haven't connected yet, so their ptyIds weren't restored above; stash session IDs in a map that survives cleanup for pty-connection.ts's deferred reconnect. const deferredSshSessionIdsByTabId: Record = {} for (const worktreeId of ids) { - const worktree = Object.values(get().worktreesByRepo) - .flat() - .find((entry) => entry.id === worktreeId) + const worktree = worktreeById.get(worktreeId) // Why: SSH worktrees aren't in worktreesByRepo at cold start; fall back to the repo id in the composite worktree id so sessions still reach the deferred map. const repoId = worktree?.repoId ?? getRepoIdFromWorktreeId(worktreeId) - const repo = repoId ? get().repos.find((entry) => entry.id === repoId) : null + const repo = repoId ? (repoById.get(repoId) ?? null) : null if (!repo?.connectionId) { continue } diff --git a/src/renderer/src/store/slices/worktree-by-id-index.test.ts b/src/renderer/src/store/slices/worktree-by-id-index.test.ts new file mode 100644 index 000000000..1c5d28e62 --- /dev/null +++ b/src/renderer/src/store/slices/worktree-by-id-index.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from 'vitest' +import type { Worktree } from '../../../../shared/types' +import { buildByIdIndex, buildWorktreeByIdIndex } from './worktree-by-id-index' + +function worktree(id: string, repoId: string): Worktree { + return { id, repoId, path: `/repos/${repoId}/${id}`, branch: id } as Worktree +} + +// Reference: the pre-change lookup these indexes replace. +function findByFlatten(worktreesByRepo: Record, id: string): Worktree | null { + return ( + Object.values(worktreesByRepo) + .flat() + .find((entry) => entry.id === id) ?? null + ) +} + +describe('buildWorktreeByIdIndex', () => { + const worktreesByRepo = { + repoA: [worktree('wt-1', 'repoA'), worktree('wt-2', 'repoA')], + repoB: [worktree('wt-3', 'repoB')], + repoC: [] + } + + // Why: the index replaces a per-iteration flatten+find, so the contract that + // matters is that it resolves exactly what that find resolved. + it('resolves every id identically to flatten-and-find', () => { + for (const id of ['wt-1', 'wt-2', 'wt-3', 'missing', '']) { + expect(buildWorktreeByIdIndex(worktreesByRepo).get(id) ?? null).toEqual( + findByFlatten(worktreesByRepo, id) + ) + } + }) + + it('returns undefined for an id in no repo', () => { + expect(buildWorktreeByIdIndex(worktreesByRepo).get('nope')).toBeUndefined() + }) + + it('handles an empty map', () => { + expect(buildWorktreeByIdIndex({}).size).toBe(0) + }) + + it('skips repos with no worktrees', () => { + expect(buildWorktreeByIdIndex(worktreesByRepo).size).toBe(3) + }) + + // Why first-wins: Array.prototype.find returns the first match, and repo order is + // Object.values order. A duplicate id across repos must resolve the same way. + it('keeps the first entry when an id appears in two repos', () => { + const duplicated = { + repoA: [worktree('shared', 'repoA')], + repoB: [worktree('shared', 'repoB')] + } + expect(buildWorktreeByIdIndex(duplicated).get('shared')?.repoId).toBe('repoA') + expect(buildWorktreeByIdIndex(duplicated).get('shared')).toEqual( + findByFlatten(duplicated, 'shared') + ) + }) +}) + +describe('buildByIdIndex', () => { + it('resolves every id identically to find', () => { + const rows = [ + { id: 'a', n: 1 }, + { id: 'b', n: 2 }, + { id: 'c', n: 3 } + ] + for (const id of ['a', 'b', 'c', 'zzz']) { + expect(buildByIdIndex(rows).get(id)).toEqual(rows.find((row) => row.id === id)) + } + }) + + it('keeps the first row when ids repeat', () => { + const rows = [ + { id: 'dup', n: 1 }, + { id: 'dup', n: 2 } + ] + expect(buildByIdIndex(rows).get('dup')).toEqual(rows.find((row) => row.id === 'dup')) + expect(buildByIdIndex(rows).get('dup')?.n).toBe(1) + }) + + it('handles an empty list', () => { + expect(buildByIdIndex([]).size).toBe(0) + }) +}) diff --git a/src/renderer/src/store/slices/worktree-by-id-index.ts b/src/renderer/src/store/slices/worktree-by-id-index.ts new file mode 100644 index 000000000..a7eaf1723 --- /dev/null +++ b/src/renderer/src/store/slices/worktree-by-id-index.ts @@ -0,0 +1,34 @@ +import type { Worktree } from '../../../../shared/types' + +/** + * Flattens worktreesByRepo into an id lookup. + * + * Why: session hydration and terminal reconnect loop over pending worktree ids and + * previously re-flattened + linear-searched the whole map per iteration, which is + * O(worktrees x ids) for O(worktrees + ids) distinct work. First entry wins, matching + * the `.find()` it replaces. + */ +export function buildWorktreeByIdIndex( + worktreesByRepo: Record +): Map { + const index = new Map() + for (const worktrees of Object.values(worktreesByRepo)) { + for (const worktree of worktrees) { + if (!index.has(worktree.id)) { + index.set(worktree.id, worktree) + } + } + } + return index +} + +/** Same first-wins contract as `buildWorktreeByIdIndex`, for any id-bearing row. */ +export function buildByIdIndex(rows: readonly T[]): Map { + const index = new Map() + for (const row of rows) { + if (!index.has(row.id)) { + index.set(row.id, row) + } + } + return index +}