From 89bd18990dc38bd7aef18a1b60e1ec5ec23fa89d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:05:41 -0700 Subject: [PATCH] perf(renderer): stop three always-mounted selectors rescanning the store (#18136) Zustand reruns every subscriber's selector on each store write. Three selectors did an O(N) scan of a store collection inside that path, so at 10 repos / 423 worktrees / 382 tabs they were paid thousands of times a second while the app sat idle. - getLocalWorktree / getLocalRuntimeRepoForWorktree now read the shared WeakMap indexes (getIndexedWorktreeById, getIndexedRepoMap) instead of `Object.values(worktreesByRepo).flat().find(...)` and `repos.find(...)`. SidebarTaskNavButton is always mounted and calls this on every write. - selectRepoByIdForActiveWorkspace caches its host-scoped resolution in a WeakMap keyed on the `repos` array, mirroring getIndexedRepoMap. - getProjectRuntimeSessionSummary memoizes per (tabsByWorktree, ptyIdsByTabId, agentStatusByPaneKey, repoId) and reuses the existing identity-cached getTabIdToWorktreeId index. --- .../repository-runtime-session-summary.ts | 45 ++- .../sidebar/worktree-agent-row-selectors.ts | 5 +- .../src/lib/local-preflight-context.ts | 22 +- .../always-mounted-selector-scan-cost.test.ts | 277 ++++++++++++++++++ src/renderer/src/store/selectors.ts | 70 +++-- 5 files changed, 389 insertions(+), 30 deletions(-) create mode 100644 src/renderer/src/store/always-mounted-selector-scan-cost.test.ts diff --git a/src/renderer/src/components/settings/repository-runtime-session-summary.ts b/src/renderer/src/components/settings/repository-runtime-session-summary.ts index 5dffcd3b1e2..62bfd7053e8 100644 --- a/src/renderer/src/components/settings/repository-runtime-session-summary.ts +++ b/src/renderer/src/components/settings/repository-runtime-session-summary.ts @@ -1,5 +1,6 @@ import type { AppState } from '../../store/types' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' +import { getTabIdToWorktreeId } from '../sidebar/worktree-agent-row-selectors' export type ProjectRuntimeSessionSummary = { liveTerminalCount: number @@ -11,16 +12,26 @@ type RuntimeSessionSummaryState = Pick< 'tabsByWorktree' | 'ptyIdsByTabId' | 'agentStatusByPaneKey' > +type SessionSummaryCache = { + ptyIdsByTabId: AppState['ptyIdsByTabId'] + agentStatusByPaneKey: AppState['agentStatusByPaneKey'] + byRepoId: Map +} + +// Why: one RepositoryPane per project reruns this on every store write, and each +// run walked every worktree bucket plus every agent-status pane. Key on the three +// input slices so unrelated writes reuse the answer instead of rescanning. +const sessionSummaryCache = new WeakMap() + function getTabIdFromPaneKey(paneKey: string): string | null { const separator = paneKey.indexOf(':') return separator > 0 ? paneKey.slice(0, separator) : null } -export function getProjectRuntimeSessionSummary( +function computeProjectRuntimeSessionSummary( state: RuntimeSessionSummaryState, repoId: string ): ProjectRuntimeSessionSummary { - const tabWorktreeIds = new Map() const projectWorktreeIds = new Set() let liveTerminalCount = 0 @@ -31,7 +42,6 @@ export function getProjectRuntimeSessionSummary( projectWorktreeIds.add(worktreeId) for (const tab of tabs) { - tabWorktreeIds.set(tab.id, worktreeId) const livePtyIds = new Set(state.ptyIdsByTabId[tab.id] ?? []) if (tab.ptyId) { livePtyIds.add(tab.ptyId) @@ -40,6 +50,9 @@ export function getProjectRuntimeSessionSummary( } } + // Rows outside this project resolve to a worktree the checks below reject, so + // the shared index answers the same question the repo-scoped map used to. + const tabWorktreeIds = getTabIdToWorktreeId(state.tabsByWorktree) let activeTaskCount = 0 for (const [paneKey, entry] of Object.entries(state.agentStatusByPaneKey)) { if (entry.state === 'done') { @@ -57,3 +70,29 @@ export function getProjectRuntimeSessionSummary( return { liveTerminalCount, activeTaskCount } } + +export function getProjectRuntimeSessionSummary( + state: RuntimeSessionSummaryState, + repoId: string +): ProjectRuntimeSessionSummary { + let cache = sessionSummaryCache.get(state.tabsByWorktree) + if ( + !cache || + cache.ptyIdsByTabId !== state.ptyIdsByTabId || + cache.agentStatusByPaneKey !== state.agentStatusByPaneKey + ) { + cache = { + ptyIdsByTabId: state.ptyIdsByTabId, + agentStatusByPaneKey: state.agentStatusByPaneKey, + byRepoId: new Map() + } + sessionSummaryCache.set(state.tabsByWorktree, cache) + } + const cached = cache.byRepoId.get(repoId) + if (cached) { + return cached + } + const summary = computeProjectRuntimeSessionSummary(state, repoId) + cache.byRepoId.set(repoId, summary) + return summary +} diff --git a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts index 9fbbd882e03..06ba3979ffe 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts @@ -80,7 +80,10 @@ export function reuseArrayIfEqual(previous: T[] | undefined, next: T[]): T[] return previous } -function getTabIdToWorktreeId( +// Why exported: the Settings -> Repositories runtime summary needs the same +// tab -> worktree index, and rebuilding it there would re-walk every tab bucket +// on each store write. +export function getTabIdToWorktreeId( tabsByWorktree: WorktreeAgentRowsState['tabsByWorktree'] ): Map { if (tabWorktreeIndexCache?.tabsByWorktree === tabsByWorktree) { diff --git a/src/renderer/src/lib/local-preflight-context.ts b/src/renderer/src/lib/local-preflight-context.ts index 98ac6c1059e..7988e4656a7 100644 --- a/src/renderer/src/lib/local-preflight-context.ts +++ b/src/renderer/src/lib/local-preflight-context.ts @@ -9,6 +9,7 @@ import { import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host' import type { Repo } from '../../../shared/repo-types' import type { Worktree } from '../../../shared/worktree/types' +import { getIndexedRepoMap, getIndexedWorktreeById } from '@/store/worktree-repo-index' import { getProviderRuntimeContextKey } from './provider-runtime-context' import { getRendererAppPlatform } from './renderer-app-platform' import { @@ -36,6 +37,11 @@ type LocalProjectRuntimeState = Pick< 'activeRepoId' | 'activeWorktreeId' | 'projects' | 'repos' | 'settings' | 'worktreesByRepo' > +// Why: the shared indexes are WeakMap-keyed on slice identity, so a fresh `{}` +// or `[]` fallback would miss the cache on every read. +const EMPTY_WORKTREES_BY_REPO: AppState['worktreesByRepo'] = {} +const EMPTY_REPOS: AppState['repos'] = [] + type LocalProjectRuntimeWslContext = { wslAvailable?: boolean availableWslDistros?: readonly string[] | null @@ -120,7 +126,7 @@ export function getLocalRepoProjectExecutionRuntimeContext( return undefined } - const repo = (state.repos ?? []).find((entry) => entry.id === repoId) + const repo = getIndexedRepoMap(state.repos ?? EMPTY_REPOS).get(repoId) if (!isLocalRuntimeRepo(repo)) { return undefined } @@ -270,7 +276,7 @@ function getLocalRuntimeRepoForWorktree( worktree?: Pick | null ): Pick | undefined { const repoId = worktree?.repoId ?? state.activeRepoId - return repoId ? (state.repos ?? []).find((repo) => repo.id === repoId) : undefined + return repoId ? getIndexedRepoMap(state.repos ?? EMPTY_REPOS).get(repoId) : undefined } function isLocalRuntimeRepo( @@ -302,11 +308,13 @@ function getLocalWorktree( worktreeId?: string | null ): Pick | null { const targetWorktreeId = worktreeId ?? state.activeWorktreeId - return targetWorktreeId - ? (Object.values(state.worktreesByRepo ?? {}) - .flat() - .find((worktree) => worktree.id === targetWorktreeId) ?? null) - : null + if (!targetWorktreeId) { + return null + } + return ( + getIndexedWorktreeById(state.worktreesByRepo ?? EMPTY_WORKTREES_BY_REPO, targetWorktreeId) ?? + null + ) } function getLocalPreflightProjectId( diff --git a/src/renderer/src/store/always-mounted-selector-scan-cost.test.ts b/src/renderer/src/store/always-mounted-selector-scan-cost.test.ts new file mode 100644 index 00000000000..2baeb5958ef --- /dev/null +++ b/src/renderer/src/store/always-mounted-selector-scan-cost.test.ts @@ -0,0 +1,277 @@ +/** + * Zustand reruns every subscriber's selector on every store write, so an O(N) + * scan inside an always-mounted selector is paid thousands of times per second + * while the app is idle. These tests count property reads on the store rows to + * prove each selector builds its index once per snapshot instead of per read. + */ +import { describe, expect, it } from 'vitest' +import type { Repo } from '../../../shared/repo-types' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import type { Worktree } from '../../../shared/worktree/types' +import { WORKTREE_ID_SEPARATOR } from '../../../shared/worktree/id' +import { getLocalPreflightContext } from '../lib/local-preflight-context' +import { getProjectRuntimeSessionSummary } from '../components/settings/repository-runtime-session-summary' +import type { AppState } from './types' +import { selectRepoByIdForActiveWorkspace } from './selectors' + +// The user scale that motivated this: 10 repos, 423 worktrees, 382 open tabs. +const REPO_COUNT = 10 +const WORKTREES_PER_REPO = 42 +const TABS_PER_WORKTREE = 1 +const STORE_WRITES = 200 + +type ReadCounter = { count: number } + +function makeRepoRows(counter: ReadCounter): Repo[] { + return Array.from({ length: REPO_COUNT }, (_unused, index) => { + const id = `repo-${index}` + return { + get id() { + counter.count += 1 + return id + }, + path: `/tmp/repo-${index}`, + displayName: `repo-${index}`, + badgeColor: '#737373', + addedAt: 100, + kind: 'git' + } as Repo + }) +} + +function makeWorktreesByRepo(counter: ReadCounter): AppState['worktreesByRepo'] { + const worktreesByRepo: Record = {} + for (let repoIndex = 0; repoIndex < REPO_COUNT; repoIndex += 1) { + const repoId = `repo-${repoIndex}` + worktreesByRepo[repoId] = Array.from({ length: WORKTREES_PER_REPO }, (_unused, index) => { + const path = String.raw`\\wsl.localhost\Ubuntu\home\alice\wt-${repoIndex}-${index}` + const id = `${repoId}${WORKTREE_ID_SEPARATOR}${path}` + return { + get id() { + counter.count += 1 + return id + }, + repoId, + path + } as Worktree + }) + } + return worktreesByRepo +} + +/** The worst case for a first-wins linear scan: the last row of the last repo. */ +function lastWorktreeId(worktreesByRepo: AppState['worktreesByRepo']): string { + const lastBucket = Object.values(worktreesByRepo).at(-1) ?? [] + return (lastBucket.at(-1) as Worktree).id +} + +describe('local preflight context worktree lookup', () => { + it('builds the worktree index once instead of rescanning per store write', () => { + const worktreeReads: ReadCounter = { count: 0 } + const repoReads: ReadCounter = { count: 0 } + const worktreesByRepo = makeWorktreesByRepo(worktreeReads) + const activeWorktreeId = lastWorktreeId(worktreesByRepo) + const state = { + activeRepoId: `repo-${REPO_COUNT - 1}`, + activeWorktreeId, + repos: makeRepoRows(repoReads), + worktreesByRepo, + projects: [] + } as unknown as AppState + const rowCount = REPO_COUNT * WORKTREES_PER_REPO + worktreeReads.count = 0 + repoReads.count = 0 + + for (let write = 0; write < STORE_WRITES; write += 1) { + expect(getLocalPreflightContext(state, 'darwin')).toEqual({ + wslDistro: 'Ubuntu' + }) + } + + // One index build per snapshot, not one scan per store write. + expect(worktreeReads.count).toBeLessThanOrEqual(rowCount) + expect(repoReads.count).toBeLessThanOrEqual(REPO_COUNT) + }) + + it('rebuilds against a replacement snapshot', () => { + const counter: ReadCounter = { count: 0 } + const worktreesByRepo = makeWorktreesByRepo(counter) + const repos = makeRepoRows({ count: 0 }) + const activeWorktreeId = lastWorktreeId(worktreesByRepo) + const before = getLocalPreflightContext( + { + activeRepoId: 'repo-0', + activeWorktreeId, + repos, + worktreesByRepo + } as unknown as AppState, + 'darwin' + ) + expect(before).toEqual({ wslDistro: 'Ubuntu' }) + + const movedWorktree = { + id: activeWorktreeId, + repoId: `repo-${REPO_COUNT - 1}`, + path: String.raw`\\wsl.localhost\Debian\home\alice\moved` + } as Worktree + const after = getLocalPreflightContext( + { + activeRepoId: 'repo-0', + activeWorktreeId, + repos, + worktreesByRepo: { [`repo-${REPO_COUNT - 1}`]: [movedWorktree] } + } as unknown as AppState, + 'darwin' + ) + + expect(after).toEqual({ wslDistro: 'Debian' }) + }) +}) + +describe('selectRepoByIdForActiveWorkspace', () => { + function makeActiveWorkspaceState(counter: ReadCounter): AppState { + return { + repos: makeRepoRows(counter), + activeRepoId: 'repo-0', + // No repo row carries this host, so the fallback branch runs every time. + activeWorkspaceExecutionHostId: 'ssh:host-a' + } as unknown as AppState + } + + it('resolves the active-workspace host once per repos snapshot', () => { + const counter: ReadCounter = { count: 0 } + const state = makeActiveWorkspaceState(counter) + counter.count = 0 + + for (let write = 0; write < STORE_WRITES; write += 1) { + expect(selectRepoByIdForActiveWorkspace(state, 'repo-0')).toBeNull() + } + + // Worst case: the id-keyed map build plus one host-filter pass. + expect(counter.count).toBeLessThanOrEqual(REPO_COUNT * 3) + }) + + it('still prefers the row that carries the active workspace host', () => { + const localRepo = { + id: 'repo-0', + path: '/tmp/a', + displayName: 'a' + } as Repo + const sshRepo = { + id: 'repo-0', + path: '/tmp/a', + displayName: 'a', + connectionId: 'host-a' + } as Repo + const state = { + repos: [localRepo, sshRepo], + activeRepoId: 'repo-0', + activeWorkspaceExecutionHostId: 'ssh:host-a' + } as unknown as AppState + + expect(selectRepoByIdForActiveWorkspace(state, 'repo-0')).toBe(sshRepo) + expect(selectRepoByIdForActiveWorkspace(state, 'repo-0')).toBe(sshRepo) + }) + + it('returns an identical result for repeated reads of one snapshot', () => { + const state = makeActiveWorkspaceState({ count: 0 }) + expect(selectRepoByIdForActiveWorkspace(state, 'repo-0')).toBe( + selectRepoByIdForActiveWorkspace(state, 'repo-0') + ) + expect(selectRepoByIdForActiveWorkspace(state, 'repo-1')).toBe( + selectRepoByIdForActiveWorkspace(state, 'repo-1') + ) + }) +}) + +describe('project runtime session summary', () => { + function makeRuntimeSessionState(counter: ReadCounter): AppState { + const tabsByWorktree: Record = {} + for (let repoIndex = 0; repoIndex < REPO_COUNT; repoIndex += 1) { + for (let index = 0; index < WORKTREES_PER_REPO; index += 1) { + const worktreeId = `repo-${repoIndex}${WORKTREE_ID_SEPARATOR}/tmp/wt-${repoIndex}-${index}` + tabsByWorktree[worktreeId] = Array.from( + { length: TABS_PER_WORKTREE }, + (_unused, tabIndex) => { + const id = `tab-${repoIndex}-${index}-${tabIndex}` + return { + get id() { + counter.count += 1 + return id + }, + ptyId: `pty-${id}`, + worktreeId + } as TerminalTab + } + ) + } + } + return { + tabsByWorktree, + ptyIdsByTabId: {}, + agentStatusByPaneKey: {} + } as unknown as AppState + } + + it('reuses the tab index across repos and store writes', () => { + const counter: ReadCounter = { count: 0 } + const state = makeRuntimeSessionState(counter) + const tabCount = REPO_COUNT * WORKTREES_PER_REPO * TABS_PER_WORKTREE + counter.count = 0 + + // One RepositoryPane per project, all re-running on every store write. + for (let write = 0; write < STORE_WRITES; write += 1) { + for (let repoIndex = 0; repoIndex < REPO_COUNT; repoIndex += 1) { + expect(getProjectRuntimeSessionSummary(state, `repo-${repoIndex}`)).toEqual({ + liveTerminalCount: WORKTREES_PER_REPO * TABS_PER_WORKTREE, + activeTaskCount: 0 + }) + } + } + + // The shared tab index plus one pass over each repo's own tabs. + expect(counter.count).toBeLessThanOrEqual(tabCount * 3) + }) + + it('returns an identical summary for repeated reads of one snapshot', () => { + const state = makeRuntimeSessionState({ count: 0 }) + expect(getProjectRuntimeSessionSummary(state, 'repo-0')).toBe( + getProjectRuntimeSessionSummary(state, 'repo-0') + ) + }) + + it('recomputes when a tab slice is replaced', () => { + const state = makeRuntimeSessionState({ count: 0 }) + const first = getProjectRuntimeSessionSummary(state, 'repo-0') + const worktreeId = `repo-0${WORKTREE_ID_SEPARATOR}/tmp/wt-0-0` + const next = getProjectRuntimeSessionSummary( + { + ...state, + tabsByWorktree: { + [worktreeId]: [{ id: 'tab-new', ptyId: 'pty-new', worktreeId } as TerminalTab] + } + } as unknown as AppState, + 'repo-0' + ) + + expect(first.liveTerminalCount).toBe(WORKTREES_PER_REPO * TABS_PER_WORKTREE) + expect(next.liveTerminalCount).toBe(1) + }) + + it('counts running agents against the owning project only', () => { + const state = makeRuntimeSessionState({ count: 0 }) + const summary = getProjectRuntimeSessionSummary( + { + ...state, + agentStatusByPaneKey: { + 'tab-0-0-0:leaf': { state: 'working', tabId: 'tab-0-0-0' }, + 'tab-1-0-0:leaf': { state: 'working', tabId: 'tab-1-0-0' }, + 'tab-0-1-0:leaf': { state: 'done', tabId: 'tab-0-1-0' } + } + } as unknown as AppState, + 'repo-0' + ) + + expect(summary.activeTaskCount).toBe(1) + }) +}) diff --git a/src/renderer/src/store/selectors.ts b/src/renderer/src/store/selectors.ts index 77c124f562c..b30673a0b39 100644 --- a/src/renderer/src/store/selectors.ts +++ b/src/renderer/src/store/selectors.ts @@ -228,33 +228,65 @@ export const useActiveRepo = () => useAppStore(useShallow((s) => selectRepoByIdForActiveWorkspace(s, s.activeRepoId))) export const useRepoMap = () => useAppStore((s) => getCachedRepoMap(s.repos)) +type ActiveWorkspaceRepoState = Pick< + AppState, + 'repos' | 'activeRepoId' | 'activeWorkspaceExecutionHostId' +> + +// Why: mirrors getIndexedRepoMap above — the host-scoped branch re-filtered every +// repo on each store write even though its answer only moves when `repos` or the +// active workspace host does. +const activeWorkspaceRepoCache = new WeakMap>() + +function resolveRepoOnActiveWorkspaceHost( + state: ActiveWorkspaceRepoState, + repoId: string, + activeWorkspaceExecutionHostId: ExecutionHostId +): Repo | null { + const repoCandidates = state.repos.filter((candidate) => candidate.id === repoId) + const hostMatch = repoCandidates.find( + (candidate) => getRepoExecutionHostId(candidate) === activeWorkspaceExecutionHostId + ) + if (hostMatch) { + return hostMatch + } + // Why: withRepoHostOwnership keeps a paired-hub worktree on its own SSH host while the repo + // stays hub-owned, so that one mismatch still names the right repo; every other stays closed. + if (parseExecutionHostId(activeWorkspaceExecutionHostId)?.kind !== 'ssh') { + return null + } + const pairedHubRepos = repoCandidates.filter( + (candidate) => parseExecutionHostId(getRepoExecutionHostId(candidate))?.kind === 'runtime' + ) + return pairedHubRepos.length === 1 ? pairedHubRepos[0] : null +} + export function selectRepoByIdForActiveWorkspace( - state: Pick, + state: ActiveWorkspaceRepoState, repoId: string | null ): Repo | null { if (!repoId) { return null } const repo = getCachedRepoMap(state.repos).get(repoId) ?? null - if (repoId === state.activeRepoId && state.activeWorkspaceExecutionHostId) { - const repoCandidates = state.repos.filter((candidate) => candidate.id === repoId) - const hostMatch = repoCandidates.find( - (candidate) => getRepoExecutionHostId(candidate) === state.activeWorkspaceExecutionHostId - ) - if (hostMatch) { - return hostMatch - } - // Why: withRepoHostOwnership keeps a paired-hub worktree on its own SSH host while the repo - // stays hub-owned, so that one mismatch still names the right repo; every other stays closed. - if (parseExecutionHostId(state.activeWorkspaceExecutionHostId)?.kind !== 'ssh') { - return null - } - const pairedHubRepos = repoCandidates.filter( - (candidate) => parseExecutionHostId(getRepoExecutionHostId(candidate))?.kind === 'runtime' - ) - return pairedHubRepos.length === 1 ? pairedHubRepos[0] : null + const activeWorkspaceExecutionHostId = state.activeWorkspaceExecutionHostId + if (repoId !== state.activeRepoId || !activeWorkspaceExecutionHostId) { + return repo } - return repo + // The branch below only fires for the active repo, so the host id fully keys it. + let byHost = activeWorkspaceRepoCache.get(state.repos) + if (!byHost) { + byHost = new Map() + activeWorkspaceRepoCache.set(state.repos, byHost) + } + const cacheKey = `${activeWorkspaceExecutionHostId}\u0000${repoId}` + const cached = byHost.get(cacheKey) + if (cached !== undefined) { + return cached + } + const resolved = resolveRepoOnActiveWorkspaceHost(state, repoId, activeWorkspaceExecutionHostId) + byHost.set(cacheKey, resolved) + return resolved } export const useRepoById = (repoId: string | null) =>