From bb8b1c3859a43b7b1d00a5d9abd99b1ae597ccaa Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 5 Jul 2026 23:08:31 -0700 Subject: [PATCH] perf(renderer): index tab lookup in hook-completion prune (O(C*T) -> O(T)) (#7496) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent hook-completion notification subscriber runs syncAgentHookCompletionNotificationSettings() -> pruneClosedPaneCoordinators() on every store notify — which includes every OSC title/spinner frame, since tabsByWorktree reallocates on each. The prune looped every coordinator and, for each, re-flattened Object.values(tabsByWorktree).flat().find(...) to resolve its tab, i.e. O(coordinators x total-tabs) of array allocation + scan per notify. Unlike the sibling mobile-sync path, it had no gate. Build the paneKey->tab index once per prune pass and thread it through paneCanReceiveHookCompletion / paneKeyHasUnsuppressedPtyHint (single-call sites keep the direct lookup). First-wins index matches the previous flat().find() semantics exactly. Also skip the pass entirely when no coordinators are tracked (the common idle case). Tests: selective prune across many coordinators still evicts only the panes that lost liveness, and tabsByWorktree is read exactly once per prune pass regardless of coordinator count (pre-fix: once per coordinator). Co-authored-by: Orca --- ...gent-hook-completion-notifications.test.ts | 77 +++++++++++++++++++ .../agent-hook-completion-notifications.ts | 60 +++++++++++++-- 2 files changed, 129 insertions(+), 8 deletions(-) diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts index 5dc61b1e6..9717f4ff6 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts @@ -727,4 +727,81 @@ describe('agent hook completion notifications', () => { }) ) }) + + const MANY_PANES = [ + { tabId: 'tab-1', leafId: '11111111-1111-4111-8111-111111111111', ptyId: 'pty-1' }, + { tabId: 'tab-2', leafId: '22222222-2222-4222-8222-222222222222', ptyId: 'pty-2' }, + { tabId: 'tab-3', leafId: '33333333-3333-4333-8333-333333333333', ptyId: 'pty-3' }, + { tabId: 'tab-4', leafId: '44444444-4444-4444-8444-444444444444', ptyId: 'pty-4' }, + { tabId: 'tab-5', leafId: '55555555-5555-4555-8555-555555555555', ptyId: 'pty-5' } + ] + + function seedManyLivePanes(): void { + mockStoreState.ptyIdsByTabId = Object.fromEntries(MANY_PANES.map((p) => [p.tabId, [p.ptyId]])) + mockStoreState.tabsByWorktree = { + 'wt-1': MANY_PANES.map((p) => ({ id: p.tabId, ptyId: p.ptyId })) + } + } + + it('prunes only the coordinators whose panes lost liveness, keeping the rest', async () => { + seedManyLivePanes() + const { + _getAgentHookCompletionNotificationCoordinatorCountForTest, + observeAgentHookCompletionForNotification, + syncAgentHookCompletionNotificationSettings + } = await import('./agent-hook-completion-notifications') + + for (const pane of MANY_PANES) { + observeAgentHookCompletionForNotification({ + paneKey: `${pane.tabId}:${pane.leafId}`, + worktreeId: 'wt-1', + payload: hookStatus('working') + }) + } + expect(_getAgentHookCompletionNotificationCoordinatorCountForTest()).toBe(MANY_PANES.length) + + // Remove liveness for two panes (both the tab hint and the pty list). + mockStoreState.tabsByWorktree = { + 'wt-1': MANY_PANES.slice(0, 3).map((p) => ({ id: p.tabId, ptyId: p.ptyId })) + } + mockStoreState.ptyIdsByTabId = Object.fromEntries( + MANY_PANES.slice(0, 3).map((p) => [p.tabId, [p.ptyId]]) + ) + syncAgentHookCompletionNotificationSettings() + + expect(_getAgentHookCompletionNotificationCoordinatorCountForTest()).toBe(3) + }) + + it('reads tabsByWorktree once per prune pass regardless of coordinator count', async () => { + seedManyLivePanes() + const { + observeAgentHookCompletionForNotification, + syncAgentHookCompletionNotificationSettings + } = await import('./agent-hook-completion-notifications') + + for (const pane of MANY_PANES) { + observeAgentHookCompletionForNotification({ + paneKey: `${pane.tabId}:${pane.leafId}`, + worktreeId: 'wt-1', + payload: hookStatus('working') + }) + } + + // Count tabsByWorktree reads during a single prune pass. Pre-fix this was + // O(coordinators) because each pane re-flattened tabsByWorktree; the index + // makes it exactly one read for the whole pass. + const realTabs = mockStoreState.tabsByWorktree + let tabsReadCount = 0 + Object.defineProperty(mockStoreState, 'tabsByWorktree', { + configurable: true, + get() { + tabsReadCount += 1 + return realTabs + } + }) + + syncAgentHookCompletionNotificationSettings() + + expect(tabsReadCount).toBe(1) + }) }) diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.ts index 3e05ae7df..00575397e 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.ts @@ -16,6 +16,11 @@ type CoordinatorEntry = { } type StoreSnapshot = ReturnType +type WorktreeTab = NonNullable[string][number] +// Why: a paneKey resolves to a tab by id. Prebuilding this index once per prune +// pass avoids re-flattening tabsByWorktree per coordinator (O(coordinators x +// tabs)) — the prune runs on every store notify, including every OSC title frame. +type TabIndex = ReadonlyMap const coordinatorsByPaneKey = new Map() const paneKeysRequiringFreshWorking = new Set() @@ -28,16 +33,36 @@ function disposeCoordinatorForPaneKey(paneKey: string): void { paneKeysRequiringFreshWorking.delete(paneKey) } +function buildTabIndex(state: StoreSnapshot): TabIndex { + const index = new Map() + for (const tabs of Object.values(state.tabsByWorktree ?? {})) { + for (const tab of tabs) { + // Why: first-wins to match the previous Array.flat().find() semantics + // exactly, even in the degenerate case of a tab id shared across worktrees. + if (!index.has(tab.id)) { + index.set(tab.id, tab) + } + } + } + return index +} + function pruneClosedPaneCoordinators(): void { // Why: hook-completion coordinators are module-scoped and may outlive a pane // unless liveness changes from close/sleep paths evict them here. + if (coordinatorsByPaneKey.size === 0 && paneKeysRequiringFreshWorking.size === 0) { + return + } + // Why: build the paneKey->tab index once for the whole pass instead of + // re-flattening tabsByWorktree inside paneCanReceiveHookCompletion per entry. + const tabIndex = buildTabIndex(useAppStore.getState()) for (const paneKey of coordinatorsByPaneKey.keys()) { - if (!paneCanReceiveHookCompletion(paneKey)) { + if (!paneCanReceiveHookCompletion(paneKey, tabIndex)) { disposeCoordinatorForPaneKey(paneKey) } } for (const paneKey of paneKeysRequiringFreshWorking) { - if (!paneCanReceiveHookCompletion(paneKey)) { + if (!paneCanReceiveHookCompletion(paneKey, tabIndex)) { paneKeysRequiringFreshWorking.delete(paneKey) } } @@ -112,14 +137,33 @@ function paneHasLivePty(paneKey: string): boolean { return getPtyIdForPaneKey(paneKey) !== null } -function paneKeyHasUnsuppressedPtyHint(state: StoreSnapshot, paneKey: string): boolean { +function resolveTabById( + state: StoreSnapshot, + tabId: string, + tabIndex?: TabIndex +): WorktreeTab | undefined { + if (tabIndex) { + return tabIndex.get(tabId) + } + for (const tabs of Object.values(state.tabsByWorktree ?? {})) { + const found = tabs.find((candidate) => candidate.id === tabId) + if (found) { + return found + } + } + return undefined +} + +function paneKeyHasUnsuppressedPtyHint( + state: StoreSnapshot, + paneKey: string, + tabIndex?: TabIndex +): boolean { const parsed = parsePaneKey(paneKey) if (!parsed) { return false } - const tab = Object.values(state.tabsByWorktree ?? {}) - .flat() - .find((candidate) => candidate.id === parsed.tabId) + const tab = resolveTabById(state, parsed.tabId, tabIndex) if (!tab) { return false } @@ -135,11 +179,11 @@ function paneKeyHasUnsuppressedPtyHint(state: StoreSnapshot, paneKey: string): b return ptyHints.length === 0 || ptyHints.some((ptyId) => !state.suppressedPtyExitIds?.[ptyId]) } -function paneCanReceiveHookCompletion(paneKey: string): boolean { +function paneCanReceiveHookCompletion(paneKey: string, tabIndex?: TabIndex): boolean { const state = useAppStore.getState() // Why: native hook IPC is itself a live status signal. Inactive worktrees can // have accepted hook updates before their renderer PTY map catches up. - return paneKeyHasUnsuppressedPtyHint(state, paneKey) || paneHasLivePty(paneKey) + return paneKeyHasUnsuppressedPtyHint(state, paneKey, tabIndex) || paneHasLivePty(paneKey) } function createCoordinator(paneKey: string, worktreeId: string): AgentCompletionCoordinator {