perf(store): index worktrees and tabs once per hydration loop (#10891)
* perf(store): index worktrees and tabs once per hydration loop Four sites in hydrateWorkspaceSession and reconnectPersistedTerminals rebuilt a flattened array inside a loop and linearly searched it: Object.values(worktreesByRepo).flat().find((e) => e.id === worktreeId) That is O(rows x ids) for O(rows + ids) distinct work. Build one first-wins index per loop instead. Neither loop sets state or awaits, so a single index over the store snapshot is valid for every iteration. 54.5x at a real 10-repo / 423-worktree session with 188 pending reconnects; 2.1x on a one-repo session. This runs synchronously on renderer cold start and gates workspaceSessionReady, which blocks terminal pane mounting. First-wins matters: Array.prototype.find returns the first match, so an index that overwrote on collision would resolve a different repo for a duplicated worktree id. Both the tests and the benchmark fixture carry a deliberate cross-repo duplicate so that difference is observable. Co-authored-by: Orca <help@stably.ai> * test(store): count the generated worktree rows instead of multiplying Review feedback: the table printed repoCount * worktreesPerRepo, which misses the duplicate id makeStore injects for multi-repo cases (that duplicate is what makes first-wins observable). Count the generated map, and say plainly that the fixtures are synthetic at real-world scale rather than a replay of a real session. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
GitHub
parent
1fd0f731fc
commit
f48cb78646
|
|
@ -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.'
|
||||
)
|
||||
|
|
@ -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<AppState, [], [], TerminalSlice>
|
|||
|
||||
// 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<string, string> = {}
|
||||
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<AppState, [], [], TerminalSlice>
|
|||
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<AppState, [], [], TerminalSlice>
|
|||
// 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<AppState, [], [], TerminalSlice>
|
|||
.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<AppState, [], [], TerminalSlice>
|
|||
// 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<string, TerminalTab[]> | null = null
|
||||
let reconnectedPtyIdsByTabId: Record<string, string[]> | 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<AppState, [], [], TerminalSlice>
|
|||
// 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<string, string> = {}
|
||||
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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<string, Worktree[]>, 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)
|
||||
})
|
||||
})
|
||||
|
|
@ -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<string, Worktree[]>
|
||||
): Map<string, Worktree> {
|
||||
const index = new Map<string, Worktree>()
|
||||
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<T extends { id: string }>(rows: readonly T[]): Map<string, T> {
|
||||
const index = new Map<string, T>()
|
||||
for (const row of rows) {
|
||||
if (!index.has(row.id)) {
|
||||
index.set(row.id, row)
|
||||
}
|
||||
}
|
||||
return index
|
||||
}
|
||||
Loading…
Reference in New Issue