perf(runtime): stop the expired-SSH-lease sweep from rescanning every tab layout (#18409)

* perf(runtime): stop the expired-SSH-lease sweep from rescanning every tab layout The `runtime:syncWindowGraph` IPC handler is the most expensive thing the main process does: measured on a real session it costs 20.7 ms per call at 0.71 calls/sec, which is 1.47% of wall and ~17% of all main-thread JS. 76% of that sits in one subtree: `getHydrationTargets` -> `hasRuntimeOwnedPtyCandidate` -> `getRecentExpiredSshLease` -> `findTerminalTabIdForLeaf`. Three pieces of pure waste, none of which change an answer: 1. `getRecentExpiredSshLease` evaluated its cheapest and most selective filter LAST. `SSH_PANE_RECOVERY_GRACE_MS` is 30 s, so nearly every stored expired lease fails it — but only after the predicate had already resolved the lease's leaf to its current tab, which is the expensive part. The freshness and reattach-eligibility gates now run first; the predicate is otherwise identical and side-effect free, so the selected lease is unchanged. 2. The sweep ran once per tab. `workspaceSessionWorktreeHasRuntimeOwnedPtyCandidate` asked "does a recent expired lease name THIS tab" for every tab in a worktree, and each ask re-read and re-filtered the whole lease list. It now resolves the worktree's recoverable tab ids once, lazily, so a worktree whose first tab already owns a serve/SSH pty still never sweeps. 3. `findTerminalTabIdForLeaf` allocated a `Set` and walked a whole pane tree per tab to answer one leaf lookup. It now reads a leafId -> tabId index built once per layouts record and reused until a layout object is replaced, which keeps first-tab-wins ordering identical. Measured by replaying a real 414-worktree / 801-tab / 137-lease session: 2.51 ms -> 0.27 ms per publish for this subtree, a 9.3x cut. No user-facing trade-off: same leases selected, same tabs reported recoverable, same SSH pane recovery affordance. * fix(runtime): revalidate the leaf membership index on root identity persistPtyBinding grafts a leaf by assigning `layout.root` on the SAME layout object inside the SAME layouts record, so the layout-identity revalidation kept serving an index blind to the grafted leaf and findTerminalTabIdForLeaf answered `undefined` where the pre-index linear scan answered the tab. That fed the SSH reattach fence (restoreReattachedPtyRuntime) and the expired-lease pane recovery resolver, both of which then fall back to the frozen lease tabId. 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 — same O(tabs) pointer compare, no new cap, cadence or staleness window. * 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. * docs(runtime): say why the leaf scan keeps Object.keys 'Allocation-free' overstated it — Object.keys does allocate one key array. A guarded for...in trades that for a hasOwn call per tab and measures slower, so record the reason the next reader does not re-litigate it.
2026-09-22 08:02:28 +00:00 · 2026-09-03 21:21:22 -07:00
parent 4101505b6b
commit 6c4797ca9f
5 changed files with 271 additions and 30 deletions
@@ -0,0 +1,86 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
+import { rmSync, mkdtempSync } from 'node:fs'
+import { join } from 'node:path'
+import { tmpdir } from 'node:os'
+import { getDefaultWorkspaceSession } from '../shared/constants'
+import { findTerminalTabIdForLeaf } from './runtime/workspace-session-terminal-membership-authority'
+import { testState, createStore, makeTerminalTab } from './persistence-test-harness'
+import { TEST_LEAF_1, TEST_LEAF_2 } from './persistence-session-fixtures'
+
+vi.mock('electron', () => ({
+  app: { getPath: () => testState.dir },
+  safeStorage: { isEncryptionAvailable: () => false }
+}))
+
+vi.mock('./telemetry/client', () => ({ track: vi.fn() }))
+vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: () => ({}) }))
+
+describe('findTerminalTabIdForLeaf after persistPtyBinding grafts a leaf', () => {
+  beforeEach(() => {
+    testState.dir = mkdtempSync(join(tmpdir(), 'orca-test-'))
+  })
+  afterEach(() => {
+    rmSync(testState.dir, { recursive: true, force: true })
+  })
+
+  // `persistPtyBinding` grafts the leaf by assigning `layout.root` on the SAME layout object inside
+  // 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({
+      ...getDefaultWorkspaceSession(),
+      tabsByWorktree: {
+        wt1: [makeTerminalTab({ id: 'tab1', worktreeId: 'wt1', ptyId: 'pty-source' })]
+      },
+      terminalLayoutsByTabId: {
+        tab1: {
+          root: { type: 'leaf', leafId: TEST_LEAF_1 },
+          activeLeafId: TEST_LEAF_1,
+          expandedLeafId: null,
+          ptyIdsByLeafId: { [TEST_LEAF_1]: 'pty-source' }
+        }
+      }
+    })
+    // A reader runs first, exactly as the syncWindowGraph lease sweep does.
+    expect(findTerminalTabIdForLeaf(store.getWorkspaceSession(), TEST_LEAF_1)).toBe('tab1')
+
+    expect(
+      store.persistPtyBinding({
+        worktreeId: 'wt1',
+        tabId: 'tab1',
+        leafId: TEST_LEAF_2,
+        ptyId: 'pty-split'
+      })
+    ).toBe(true)
+
+    expect(findTerminalTabIdForLeaf(store.getWorkspaceSession(), TEST_LEAF_2)).toBe('tab1')
+    expect(findTerminalTabIdForLeaf(store.getWorkspaceSession(), TEST_LEAF_1)).toBe('tab1')
+  })
+
+  // The other in-place graft: an empty persisted layout gets its first durable root.
+  it('resolves the first leaf grafted onto an empty layout', async () => {
+    const store = await createStore()
+    store.setWorkspaceSession({
+      ...getDefaultWorkspaceSession(),
+      tabsByWorktree: {
+        wt1: [makeTerminalTab({ id: 'tab1', worktreeId: 'wt1', ptyId: null })]
+      },
+      terminalLayoutsByTabId: {
+        tab1: { root: null, activeLeafId: null, expandedLeafId: null, ptyIdsByLeafId: {} }
+      }
+    })
+    expect(findTerminalTabIdForLeaf(store.getWorkspaceSession(), TEST_LEAF_1)).toBeUndefined()
+
+    expect(
+      store.persistPtyBinding({
+        worktreeId: 'wt1',
+        tabId: 'tab1',
+        leafId: TEST_LEAF_1,
+        ptyId: 'pty-first'
+      })
+    ).toBe(true)
+
+    expect(findTerminalTabIdForLeaf(store.getWorkspaceSession(), TEST_LEAF_1)).toBe('tab1')
+  })
+})
@@ -12,6 +12,12 @@ const TARGET = 'ssh-target'
 const TAB_ID = 'tab-candidacy'

 type LeaseReader = {
+  workspaceSessionWorktreeHasRuntimeOwnedPtyCandidate: (
+    session: { terminalLayoutsByTabId?: Record<string, unknown> },
+    worktreeId: string,
+    tabs: { id: string; ptyId: string | null }[]
+  ) => boolean
+  collectRecentExpiredSshLeaseTabIds: (worktreeId: string) => ReadonlySet<string>
  getRecentExpiredSshLease: (
    worktreeId: string,
    tabId: string,
@@ -87,4 +93,45 @@ describe('recent expired SSH lease candidacy', () => {
    )
    expect(reader.hasRecentExpiredSshLeasePane(TEST_WORKTREE_ID, pane)).toBe(true)
  })
+
+  it('collects the same tabs the per-tab reader reports, in one sweep of the leases', () => {
+    const leases = [leaseFor('pty-1', { supersededBy: 'pty-2' }), leaseFor('pty-2')]
+    let sweeps = 0
+    const reader = new OrcaRuntimeService({
+      ...store,
+      getSshRemotePtyLeases: () => {
+        sweeps += 1
+        return leases
+      }
+    }) as unknown as LeaseReader
+    const tabs = Array.from({ length: 8 }, (_, index) => ({
+      id: index === 7 ? TAB_ID : `tab-${index}`,
+      ptyId: null
+    }))
+
+    expect(
+      reader.workspaceSessionWorktreeHasRuntimeOwnedPtyCandidate(
+        { terminalLayoutsByTabId: {} },
+        TEST_WORKTREE_ID,
+        tabs
+      )
+    ).toBe(true)
+    // One sweep answers all eight tabs; the per-tab reader used to sweep once per tab.
+    expect(sweeps).toBe(1)
+    expect([...reader.collectRecentExpiredSshLeaseTabIds(TEST_WORKTREE_ID)]).toEqual([TAB_ID])
+  })
+
+  it('reports no candidate when no lease names any of the worktree tabs', () => {
+    const reader = readerWithLeases([
+      leaseFor('pty-1', { tabId: 'somewhere-else', leafId: undefined })
+    ])
+
+    expect(
+      reader.workspaceSessionWorktreeHasRuntimeOwnedPtyCandidate(
+        { terminalLayoutsByTabId: {} },
+        TEST_WORKTREE_ID,
+        [{ id: TAB_ID, ptyId: null }]
+      )
+    ).toBe(false)
+  })
 })
@@ -82,17 +82,24 @@ export class OrcaRuntimeWithReconcileHeadlessMobileSessionBrowserTabs extends Or
    worktreeId: string,
    tabs: WorkspaceSessionState['tabsByWorktree'][string]
  ): boolean {
+    // Why resolved lazily and reused: the per-tab question is the same lease sweep with a
+    // different tabId, so asking it once per worktree answers every tab. Kept lazy so a
+    // worktree whose first tab already owns a serve/SSH pty never sweeps at all.
+    let recoverableTabIds: ReadonlySet<string> | undefined
    return tabs.some((tab) => {
      if (this.isServeOrSshOwnedPtyId(tab.ptyId)) {
        return true
      }
      const leafPtyIds = session.terminalLayoutsByTabId?.[tab.id]?.ptyIdsByLeafId
-      return (
-        (leafPtyIds &&
-          Object.values(leafPtyIds).some((ptyId) => this.isServeOrSshOwnedPtyId(ptyId))) ||
-        // Why: expiry keeps pane coordinates so paired viewers can request a fresh shell.
-        this.getRecentExpiredSshLease(worktreeId, tab.id, undefined) !== null
-      )
+      if (
+        leafPtyIds &&
+        Object.values(leafPtyIds).some((ptyId) => this.isServeOrSshOwnedPtyId(ptyId))
+      ) {
+        return true
+      }
+      // Why: expiry keeps pane coordinates so paired viewers can request a fresh shell.
+      recoverableTabIds ??= this.collectRecentExpiredSshLeaseTabIds(worktreeId)
+      return recoverableTabIds.has(tab.id)
    })
  }

@@ -128,6 +135,54 @@ export class OrcaRuntimeWithReconcileHeadlessMobileSessionBrowserTabs extends Or
    )
  }

+  /**
+   * Why eligibility belongs in the selection, not after it: a pane accumulates leases as it
+   * re-leases under new relay ids, so `(worktreeId, tabId, leafId)` names several. A superseded or
+   * relay-id-recycled predecessor is `expired` for a reason that already names its successor, and
+   * the unqualified callers use this answer to decide a pane is still recoverable — reporting one
+   * would offer paired viewers a recovery `recoverTerminalPane` then refuses. Picking the first
+   * ELIGIBLE orphan also keeps a predecessor from shadowing the successor that is genuinely
+   * reattachable.
+   */
+  private isRecentExpiredSshLeaseForWorktree(
+    lease: ReturnType<NonNullable<RuntimeStore['getSshRemotePtyLeases']>>[number],
+    worktreeId: string,
+    now: number
+  ): boolean {
+    return (
+      lease.state === 'expired' &&
+      lease.worktreeId === worktreeId &&
+      sshRemotePtyLeaseAllowsReattach(lease) &&
+      lease.updatedAt <= now &&
+      now - lease.updatedAt <= SSH_PANE_RECOVERY_GRACE_MS
+    )
+  }
+
+  /**
+   * Leaf is the pane's identity; the frozen tabId is only trustworthy while nothing else can say
+   * where the leaf actually lives.
+   */
+  private resolveExpiredSshLeaseTabId(
+    lease: ReturnType<NonNullable<RuntimeStore['getSshRemotePtyLeases']>>[number]
+  ): string {
+    const currentTabId = lease.leafId
+      ? this.findCurrentTerminalTabIdForLeaf(lease.targetId, lease.leafId)
+      : undefined
+    return currentTabId ?? lease.tabId
+  }
+
+  /** The tabs a recent eligible expired lease still names, resolved in one sweep of the leases. */
+  protected collectRecentExpiredSshLeaseTabIds(worktreeId: string): ReadonlySet<string> {
+    const now = Date.now()
+    const tabIds = new Set<string>()
+    for (const lease of this.store?.getSshRemotePtyLeases?.() ?? []) {
+      if (this.isRecentExpiredSshLeaseForWorktree(lease, worktreeId, now)) {
+        tabIds.add(this.resolveExpiredSshLeaseTabId(lease))
+      }
+    }
+    return tabIds
+  }
+
  protected getRecentExpiredSshLease(
    worktreeId: string,
    tabId: string,
@@ -137,34 +192,17 @@ export class OrcaRuntimeWithReconcileHeadlessMobileSessionBrowserTabs extends Or
    const now = Date.now()
    return (
      this.store?.getSshRemotePtyLeases?.().find((lease) => {
-        if (lease.state !== 'expired' || lease.worktreeId !== worktreeId) {
+        if (!this.isRecentExpiredSshLeaseForWorktree(lease, worktreeId, now)) {
          return false
        }
-        // Why eligibility belongs in the selection, not after it: a pane accumulates leases as it
-        // re-leases under new relay ids, so `(worktreeId, tabId, leafId)` names several. A
-        // superseded or relay-id-recycled predecessor is `expired` for a reason that already names
-        // its successor, and the unqualified callers use this answer to decide a pane is still
-        // recoverable — reporting one would offer paired viewers a recovery `recoverTerminalPane`
-        // then refuses. Picking the first ELIGIBLE orphan also keeps a predecessor from shadowing
-        // the successor that is genuinely reattachable.
-        if (!sshRemotePtyLeaseAllowsReattach(lease)) {
-          return false
-        }
-        // Leaf is the pane's identity; the frozen tabId is only trustworthy while nothing else can
-        // say where the leaf actually lives.
-        const currentTabId = lease.leafId
-          ? this.findCurrentTerminalTabIdForLeaf(lease.targetId, lease.leafId)
-          : undefined
        return (
-          (currentTabId ?? lease.tabId) === tabId &&
+          this.resolveExpiredSshLeaseTabId(lease) === tabId &&
          // Leases store RELAY form (`toStoredPtyId` -> `toRelaySshPtyId`); the runtime hands us
          // the APP form (`ssh:<target>@@pty-3`). A raw `===` therefore never held for an SSH
          // pane, which is what kept this reader's only ptyId-qualified caller inert.
          (ptyId === undefined ||
            lease.ptyId === toComparableRelaySshPtyId(lease.targetId, ptyId)) &&
-          (leafId === undefined || lease.leafId === undefined || lease.leafId === leafId) &&
-          lease.updatedAt <= now &&
-          now - lease.updatedAt <= SSH_PANE_RECOVERY_GRACE_MS
+          (leafId === undefined || lease.leafId === undefined || lease.leafId === leafId)
        )
      }) ?? null
    )
@@ -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,6 +5,7 @@ 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'

 function collectLeafIds(node: TerminalPaneLayoutNode | null, ids: Set<string>): void {
@@ -164,15 +165,22 @@ 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.
+ *
+ * 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. `Object.keys` over a guarded
+ * `for...in` is deliberate too: the key array is cheaper than a `hasOwn` call per tab (measured).
 */
 export function findTerminalTabIdForLeaf(
  session: WorkspaceSessionState | undefined,
  leafId: string
 ): string | undefined {
-  for (const [tabId, layout] of Object.entries(session?.terminalLayoutsByTabId ?? {})) {
-    const leafIds = new Set<string>()
-    collectLeafIds(layout.root, leafIds)
-    if (leafIds.has(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
    }
  }