mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
perf(runtime): resolve a leaf's tab by scan instead of a cached membership index
Fix #3 of this PR cached a leafId -> tabId map per layouts record and revalidated it by comparing every root reference on every read. It was the only mutable cross-call state in the change, the only piece carrying a staleness invariant, and it had already needed one follow-up fix (1c23c544) after a layout-identity key turned out to be blind to `persistPtyBinding`'s in-place `layout.root` graft. The index was never what produced the measured win. After fix #1 moves the freshness gate first, the reporter's replay never calls `findTerminalTabIdForLeaf` at all — every stored expired lease is older than the 30 s recovery grace, so the entire 24.9 ms -> 0.9 ms comes from fixes #1 and #2, both of which are unchanged. `findTerminalTabIdForLeaf` is now an allocation-free scan over the existing `layoutContainsLeafId`, which short-circuits on the first matching leaf instead of materialising a Set per tab. Same answers, same first-tab-in-record-order semantics, no revalidation key, nothing for a writer to invalidate. Re-measured on the same 414-worktree / 801-tab / 137-lease replay (process.cpuUsage deltas, median of 3; wall clock is useless on this box): scenario main index scan all leases stale (replay) 24.86 0.87 0.88 ms/publish one lease inside the grace 24.31 1.08 1.04 ms/publish all 137 inside the grace 18.04 3.71 4.31 ms/publish The measured win is unchanged. Only the synthetic worst case — every one of 137 leases expiring inside the same 30 s window — pays for the cache's absence, and even there the two ranges overlap because the index's own revalidation is O(tabs) per lookup. Removes 208 net lines. `terminal-leaf-tab-resolution.test.ts` keeps the parity cases and adds the guard the cache needed: a subtree replaced in place after an earlier read must be visible to the next one. That test fails against the index.
This commit is contained in:
Neil
+2
-1
@@ -24,7 +24,8 @@ describe('findTerminalTabIdForLeaf after persistPtyBinding grafts a leaf', () =>
|
||||
})
|
||||
|
||||
// `persistPtyBinding` grafts the leaf by assigning `layout.root` on the SAME layout object inside
|
||||
// the SAME layouts record (pty-binding-persistence.ts). Any membership cache must notice that.
|
||||
// the SAME layouts record (pty-binding-persistence.ts), so the resolver has to answer from the
|
||||
// tree that is there now, not from anything derived on an earlier call.
|
||||
it('resolves a leaf grafted in place by a split spawn', async () => {
|
||||
const store = await createStore()
|
||||
store.setWorkspaceSession({
|
||||
@@ -1,118 +0,0 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type {
|
||||
TerminalLayoutSnapshot,
|
||||
TerminalPaneLayoutNode
|
||||
} from '../../shared/terminal-tab-types'
|
||||
import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types'
|
||||
import { findTerminalTabIdForLeaf } from './workspace-session-terminal-membership-authority'
|
||||
import { getTerminalLeafMembershipIndex } from './terminal-leaf-membership-index'
|
||||
|
||||
function layout(...leafIds: string[]): TerminalLayoutSnapshot {
|
||||
let root = { type: 'leaf' as const, leafId: leafIds[0] }
|
||||
for (const leafId of leafIds.slice(1)) {
|
||||
root = {
|
||||
type: 'split',
|
||||
direction: 'row',
|
||||
first: root,
|
||||
second: { type: 'leaf' as const, leafId }
|
||||
} as never
|
||||
}
|
||||
return { root, activeLeafId: leafIds[0], ptyIdsByLeafId: {} } as TerminalLayoutSnapshot
|
||||
}
|
||||
|
||||
function session(layouts: Record<string, TerminalLayoutSnapshot>): WorkspaceSessionState {
|
||||
return { terminalLayoutsByTabId: layouts } as WorkspaceSessionState
|
||||
}
|
||||
|
||||
describe('getTerminalLeafMembershipIndex', () => {
|
||||
it('maps every leaf in a split tree to its tab', () => {
|
||||
const index = getTerminalLeafMembershipIndex({
|
||||
'tab-a': layout('leaf-1', 'leaf-2', 'leaf-3'),
|
||||
'tab-b': layout('leaf-4')
|
||||
})
|
||||
expect([...index]).toEqual([
|
||||
['leaf-1', 'tab-a'],
|
||||
['leaf-2', 'tab-a'],
|
||||
['leaf-3', 'tab-a'],
|
||||
['leaf-4', 'tab-b']
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps the first tab in record order when two layouts claim one leaf', () => {
|
||||
const layouts = { 'tab-a': layout('shared'), 'tab-b': layout('shared') }
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('shared')).toBe('tab-a')
|
||||
expect(findTerminalTabIdForLeaf(session(layouts), 'shared')).toBe('tab-a')
|
||||
})
|
||||
|
||||
it('returns the same map instance for an unchanged record', () => {
|
||||
const layouts = { 'tab-a': layout('leaf-1') }
|
||||
expect(getTerminalLeafMembershipIndex(layouts)).toBe(getTerminalLeafMembershipIndex(layouts))
|
||||
})
|
||||
|
||||
it('rebuilds when a layout object inside the same record is replaced', () => {
|
||||
const layouts: Record<string, TerminalLayoutSnapshot> = { 'tab-a': layout('leaf-1') }
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a')
|
||||
layouts['tab-a'] = layout('leaf-9')
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBeUndefined()
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-9')).toBe('tab-a')
|
||||
})
|
||||
|
||||
it('rebuilds when a tab is added to or removed from the same record', () => {
|
||||
const layouts: Record<string, TerminalLayoutSnapshot> = { 'tab-a': layout('leaf-1') }
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a')
|
||||
layouts['tab-b'] = layout('leaf-2')
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-2')).toBe('tab-b')
|
||||
delete layouts['tab-a']
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('answers misses and empty sessions the same way the linear scan did', () => {
|
||||
expect(findTerminalTabIdForLeaf(undefined, 'leaf-1')).toBeUndefined()
|
||||
expect(findTerminalTabIdForLeaf(session({}), 'leaf-1')).toBeUndefined()
|
||||
expect(findTerminalTabIdForLeaf(session({ 'tab-a': layout('leaf-1') }), 'nope')).toBeUndefined()
|
||||
})
|
||||
|
||||
// `persistPtyBinding` grafts a leaf by assigning `layout.root` on the SAME layout object in the
|
||||
// SAME record — a layout-identity check would keep serving an index blind to the new leaf.
|
||||
it('rebuilds when a layout root is replaced in place on the same layout object', () => {
|
||||
const tracked = layout('leaf-1')
|
||||
const layouts: Record<string, TerminalLayoutSnapshot> = { 'tab-a': tracked }
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a')
|
||||
tracked.root = {
|
||||
type: 'split',
|
||||
direction: 'vertical',
|
||||
first: tracked.root,
|
||||
second: { type: 'leaf', leafId: 'leaf-2' }
|
||||
} as never
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-2')).toBe('tab-a')
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a')
|
||||
})
|
||||
|
||||
it('rebuilds when an empty layout gets its first root in place', () => {
|
||||
const tracked = { root: null, activeLeafId: null, ptyIdsByLeafId: {} } as TerminalLayoutSnapshot
|
||||
const layouts: Record<string, TerminalLayoutSnapshot> = { 'tab-a': tracked }
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBeUndefined()
|
||||
tracked.root = { type: 'leaf', leafId: 'leaf-1' }
|
||||
expect(getTerminalLeafMembershipIndex(layouts).get('leaf-1')).toBe('tab-a')
|
||||
})
|
||||
|
||||
it('does not walk a layout tree again once the record is indexed', () => {
|
||||
let nodeVisits = 0
|
||||
// Stable node object: revalidation only compares its reference, a rebuild reads `type`.
|
||||
const root = {
|
||||
get type() {
|
||||
nodeVisits += 1
|
||||
return 'leaf' as const
|
||||
},
|
||||
leafId: 'leaf-1'
|
||||
} as unknown as TerminalPaneLayoutNode
|
||||
const layouts = {
|
||||
'tab-a': { root, activeLeafId: 'leaf-1', ptyIdsByLeafId: {} } as TerminalLayoutSnapshot
|
||||
}
|
||||
for (let i = 0; i < 50; i += 1) {
|
||||
findTerminalTabIdForLeaf(session(layouts), `leaf-${i}`)
|
||||
}
|
||||
expect(nodeVisits).toBe(1)
|
||||
})
|
||||
})
|
||||
@@ -1,90 +0,0 @@
|
||||
import type {
|
||||
TerminalLayoutSnapshot,
|
||||
TerminalPaneLayoutNode
|
||||
} from '../../shared/terminal-tab-types'
|
||||
|
||||
type LayoutsByTabId = Record<string, TerminalLayoutSnapshot>
|
||||
|
||||
type LeafMembershipIndex = {
|
||||
/** First tab (in record order) whose layout tree holds the leaf — matches the linear scan. */
|
||||
readonly tabIdByLeafId: ReadonlyMap<string, string>
|
||||
/** Root nodes the index was built from; absent tab reads back `undefined`, empty tab `null`. */
|
||||
readonly builtFrom: ReadonlyMap<string, TerminalPaneLayoutNode | null>
|
||||
}
|
||||
|
||||
const EMPTY_INDEX: LeafMembershipIndex = {
|
||||
tabIdByLeafId: new Map(),
|
||||
builtFrom: new Map()
|
||||
}
|
||||
|
||||
// Sessions hand back the same layouts record across reads, so one index serves every lookup.
|
||||
const indexByLayoutsRecord = new WeakMap<LayoutsByTabId, LeafMembershipIndex>()
|
||||
|
||||
function addLeafIds(
|
||||
node: TerminalPaneLayoutNode | null | undefined,
|
||||
tabId: string,
|
||||
tabIdByLeafId: Map<string, string>
|
||||
): void {
|
||||
if (!node) {
|
||||
return
|
||||
}
|
||||
if (node.type === 'leaf') {
|
||||
if (!tabIdByLeafId.has(node.leafId)) {
|
||||
tabIdByLeafId.set(node.leafId, tabId)
|
||||
}
|
||||
return
|
||||
}
|
||||
addLeafIds(node.first, tabId, tabIdByLeafId)
|
||||
addLeafIds(node.second, tabId, tabIdByLeafId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Why an identity check and not just the WeakMap: a few writers copy the layouts record and then
|
||||
* assign into the copy, so the record can be new while most layout objects are shared — and a
|
||||
* cached index must not outlive a replaced layout. Comparing root references is O(tabs) pointer
|
||||
* loads with no allocation, versus the rebuild's set-per-tab tree walk.
|
||||
*
|
||||
* Why the ROOT and not the layout object: `persistPtyBinding` grafts a leaf by assigning
|
||||
* `layout.root` on the same layout object inside the same record (see the split/first-root branches
|
||||
* in `persistence/loading-store/pty-binding-persistence.ts`), so a layout-identity check would keep
|
||||
* serving an index that has never seen the grafted leaf. Membership is a pure function of the root
|
||||
* tree, and no writer mutates a node in place, so root identity is the exact revalidation key.
|
||||
*/
|
||||
function isIndexCurrent(index: LeafMembershipIndex, layouts: LayoutsByTabId): boolean {
|
||||
let seen = 0
|
||||
for (const tabId of Object.keys(layouts)) {
|
||||
const builtFromRoot = index.builtFrom.get(tabId)
|
||||
if (builtFromRoot === undefined || builtFromRoot !== (layouts[tabId]?.root ?? null)) {
|
||||
return false
|
||||
}
|
||||
seen += 1
|
||||
}
|
||||
return seen === index.builtFrom.size
|
||||
}
|
||||
|
||||
function buildIndex(layouts: LayoutsByTabId): LeafMembershipIndex {
|
||||
const tabIdByLeafId = new Map<string, string>()
|
||||
const builtFrom = new Map<string, TerminalPaneLayoutNode | null>()
|
||||
for (const tabId of Object.keys(layouts)) {
|
||||
const root = layouts[tabId]?.root ?? null
|
||||
builtFrom.set(tabId, root)
|
||||
addLeafIds(root, tabId, tabIdByLeafId)
|
||||
}
|
||||
return { tabIdByLeafId, builtFrom }
|
||||
}
|
||||
|
||||
/** leafId -> owning tabId for one session's layouts, rebuilt only when a layout object changes. */
|
||||
export function getTerminalLeafMembershipIndex(
|
||||
layouts: LayoutsByTabId | undefined
|
||||
): ReadonlyMap<string, string> {
|
||||
if (!layouts) {
|
||||
return EMPTY_INDEX.tabIdByLeafId
|
||||
}
|
||||
const cached = indexByLayoutsRecord.get(layouts)
|
||||
if (cached && isIndexCurrent(cached, layouts)) {
|
||||
return cached.tabIdByLeafId
|
||||
}
|
||||
const built = buildIndex(layouts)
|
||||
indexByLayoutsRecord.set(layouts, built)
|
||||
return built.tabIdByLeafId
|
||||
}
|
||||
@@ -0,0 +1,62 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { TerminalLayoutSnapshot } from '../../shared/terminal-tab-types'
|
||||
import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types'
|
||||
import { findTerminalTabIdForLeaf } from './workspace-session-terminal-membership-authority'
|
||||
|
||||
function layout(...leafIds: string[]): TerminalLayoutSnapshot {
|
||||
let root = { type: 'leaf' as const, leafId: leafIds[0] }
|
||||
for (const leafId of leafIds.slice(1)) {
|
||||
root = {
|
||||
type: 'split',
|
||||
direction: 'row',
|
||||
first: root,
|
||||
second: { type: 'leaf' as const, leafId }
|
||||
} as never
|
||||
}
|
||||
return { root, activeLeafId: leafIds[0], ptyIdsByLeafId: {} } as TerminalLayoutSnapshot
|
||||
}
|
||||
|
||||
function session(layouts: Record<string, TerminalLayoutSnapshot>): WorkspaceSessionState {
|
||||
return { terminalLayoutsByTabId: layouts } as WorkspaceSessionState
|
||||
}
|
||||
|
||||
describe('findTerminalTabIdForLeaf', () => {
|
||||
it('resolves every leaf of a split tree to its tab', () => {
|
||||
const state = session({
|
||||
'tab-a': layout('leaf-1', 'leaf-2', 'leaf-3'),
|
||||
'tab-b': layout('leaf-4')
|
||||
})
|
||||
expect(findTerminalTabIdForLeaf(state, 'leaf-2')).toBe('tab-a')
|
||||
expect(findTerminalTabIdForLeaf(state, 'leaf-3')).toBe('tab-a')
|
||||
expect(findTerminalTabIdForLeaf(state, 'leaf-4')).toBe('tab-b')
|
||||
})
|
||||
|
||||
it('keeps the first tab in record order when two layouts claim one leaf', () => {
|
||||
const layouts = { 'tab-a': layout('shared'), 'tab-b': layout('shared') }
|
||||
expect(findTerminalTabIdForLeaf(session(layouts), 'shared')).toBe('tab-a')
|
||||
})
|
||||
|
||||
it('answers misses, empty sessions and empty layouts with undefined', () => {
|
||||
expect(findTerminalTabIdForLeaf(undefined, 'leaf-1')).toBeUndefined()
|
||||
expect(findTerminalTabIdForLeaf(session({}), 'leaf-1')).toBeUndefined()
|
||||
expect(findTerminalTabIdForLeaf(session({ 'tab-a': layout('leaf-1') }), 'nope')).toBeUndefined()
|
||||
})
|
||||
|
||||
// The guard a leafId -> tabId cache needed and this scan does not: membership is read from the
|
||||
// tree that is there NOW. `persistPtyBinding` grafts leaves by assigning into a layout already in
|
||||
// the record, so anything memoized across calls has to be revalidated against every mutation
|
||||
// shape a writer can produce - including one that leaves the root node's identity untouched.
|
||||
it('reflects a subtree replaced in place after an earlier read', () => {
|
||||
const tracked = layout('leaf-1', 'leaf-2')
|
||||
const state = session({ 'tab-a': tracked })
|
||||
expect(findTerminalTabIdForLeaf(state, 'leaf-2')).toBe('tab-a')
|
||||
expect(findTerminalTabIdForLeaf(state, 'leaf-9')).toBeUndefined()
|
||||
|
||||
const root = tracked.root as { second: unknown }
|
||||
root.second = { type: 'leaf', leafId: 'leaf-9' }
|
||||
|
||||
expect(findTerminalTabIdForLeaf(state, 'leaf-9')).toBe('tab-a')
|
||||
expect(findTerminalTabIdForLeaf(state, 'leaf-2')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -5,8 +5,8 @@ import type {
|
||||
} from '../../shared/terminal-tab-types'
|
||||
import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types'
|
||||
import { getRepoIdFromWorktreeId } from '../../shared/worktree/id'
|
||||
import { layoutContainsLeafId } from '../persistence/restoring-sessions/terminal-layout-normalization'
|
||||
import { pruneTabGroupLayoutAfterRetirement } from './mobile-session-terminal-retirement'
|
||||
import { getTerminalLeafMembershipIndex } from './terminal-leaf-membership-index'
|
||||
|
||||
function collectLeafIds(node: TerminalPaneLayoutNode | null, ids: Set<string>): void {
|
||||
if (!node) {
|
||||
@@ -165,12 +165,25 @@ export function advanceTerminalTopologyRevision(
|
||||
* The tab whose live layout holds this leaf. Only the leaf half of a pane key is remint-stable —
|
||||
* `detachTerminalPaneToTab` moves a live pane into a new tab, so a stored tabId names the tab the
|
||||
* pane left. Callers fencing on location must resolve it here rather than trust a frozen tabId.
|
||||
*
|
||||
* Allocation-free and stateless on purpose: writers graft leaves by assigning into a layout that is
|
||||
* already inside the layouts record, so any cache here would need a revalidation key that is itself
|
||||
* O(tabs) per read — the same cost as this walk, with a staleness invariant to keep.
|
||||
*/
|
||||
export function findTerminalTabIdForLeaf(
|
||||
session: WorkspaceSessionState | undefined,
|
||||
leafId: string
|
||||
): string | undefined {
|
||||
return getTerminalLeafMembershipIndex(session?.terminalLayoutsByTabId).get(leafId)
|
||||
const layouts = session?.terminalLayoutsByTabId
|
||||
if (!layouts) {
|
||||
return undefined
|
||||
}
|
||||
for (const tabId of Object.keys(layouts)) {
|
||||
if (layoutContainsLeafId(layouts[tabId]?.root ?? null, leafId)) {
|
||||
return tabId
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
export function hasHostAuthoritativeTerminalMembership(
|
||||
|
||||
Reference in New Issue
Block a user