perf(renderer): index tab lookup in hook-completion prune (O(C*T) -> O(T)) (#7496)
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 <help@stably.ai>
This commit is contained in:
parent
b47474b036
commit
bb8b1c3859
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -16,6 +16,11 @@ type CoordinatorEntry = {
|
|||
}
|
||||
|
||||
type StoreSnapshot = ReturnType<typeof useAppStore.getState>
|
||||
type WorktreeTab = NonNullable<StoreSnapshot['tabsByWorktree']>[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<string, WorktreeTab>
|
||||
|
||||
const coordinatorsByPaneKey = new Map<string, CoordinatorEntry>()
|
||||
const paneKeysRequiringFreshWorking = new Set<string>()
|
||||
|
|
@ -28,16 +33,36 @@ function disposeCoordinatorForPaneKey(paneKey: string): void {
|
|||
paneKeysRequiringFreshWorking.delete(paneKey)
|
||||
}
|
||||
|
||||
function buildTabIndex(state: StoreSnapshot): TabIndex {
|
||||
const index = new Map<string, WorktreeTab>()
|
||||
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 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue