From f94d87b5fda891319cbe245c15fafa0e7783c579 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 24 May 2026 13:20:55 +0300 Subject: [PATCH] =?UTF-8?q?perf(git):=20cache=20project-root=20resolution?= =?UTF-8?q?=20to=20stop=20N=C2=B2=20polling=20cascade=20(#1398)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(git): cache project-root resolution to stop N² polling cascade Opening a workspace with many projects/worktrees fired hundreds of `POST /api/fs/exec` requests (e.g. ~700 for 19 projects) within seconds, dominated by repeated `git rev-parse --absolute-git-dir` / `--git-common-dir` for the same directories. Root cause: in `useProjectRepoStatus`, each project's `ensureStatus` settles independently and mutates the git store, which re-derives `projectGitBranchesKey` and re-runs `getRootBranch` for *all* projects on every change. `getRootBranch` had no caching, so this produced an N×N burst of uncached git plumbing calls. Changes: - worktreeStatus: extract `resolveProjectRoot` to module scope with a 60s TTL cache + in-flight dedupe (root resolution is static within a session). Combine the two `rev-parse` queries into one subprocess. Add `getRootBranch(dir, { knownBranch })` fast-path that skips a redundant git status when the directory is its own root, while still resolving the primary-root branch correctly for linked worktrees. Export `invalidateResolvedProjectRootCache`. - useProjectRepoStatus: replace the cascade effect with a debounced, diff-based pass that only resolves projects that are new or whose branch actually changed, passing the known branch through. - worktreeManager: invalidate the root cache on worktree create/remove. - Add unit tests for caching, dedupe, invalidation, rev-parse precedence, non-git fallback, linked-worktree resolution and the knownBranch fast-path. Reduces startup from hundreds of requests to roughly one root resolution per project. * fix(git): clear in-flight resolves and guard write-back on cache invalidation `invalidateResolvedProjectRootCache` cleared `resolvedRootCache` but left `inFlightRootResolves` intact, so during a worktree topology change a resolution already in flight could (1) be handed to callers arriving after invalidation and (2) re-seed the cache with the pre-invalidation root when it settled, defeating invalidation for up to the full TTL. Drop the in-flight entry on invalidation and add an epoch guard so a resolve that was invalidated mid-flight does not write its now-stale result back. Add a regression test for the concurrent-invalidation scenario. * fix(git): bound root cache and avoid early sidebar resolves --- .../sidebar/hooks/useProjectRepoStatus.ts | 90 ++++++++-- .../ui/src/lib/worktrees/worktreeManager.ts | 7 + .../src/lib/worktrees/worktreeStatus.test.ts | 153 +++++++++++++++++ .../ui/src/lib/worktrees/worktreeStatus.ts | 156 ++++++++++++++++-- 4 files changed, 371 insertions(+), 35 deletions(-) create mode 100644 packages/ui/src/lib/worktrees/worktreeStatus.test.ts diff --git a/packages/ui/src/components/session/sidebar/hooks/useProjectRepoStatus.ts b/packages/ui/src/components/session/sidebar/hooks/useProjectRepoStatus.ts index 4addc961..ab21242f 100644 --- a/packages/ui/src/components/session/sidebar/hooks/useProjectRepoStatus.ts +++ b/packages/ui/src/components/session/sidebar/hooks/useProjectRepoStatus.ts @@ -55,29 +55,83 @@ export const useProjectRepoStatus = (args: Args): void => { .join('|'); }, [normalizedProjects, gitRepoStatus]); + // Tracks the project path + input branch we last resolved against, per project. + // Used to resolve `getRootBranch` only for projects that are new or whose + // input actually changed — rather than re-resolving every project whenever + // any single project's branch settles (the old N² cascade). + const resolvedInputKeyByProjectId = React.useRef>(new Map()); + React.useEffect(() => { let cancelled = false; - const run = async () => { - const entries = await mapWithConcurrency(normalizedProjects, 2, async (project) => { - const branch = await getRootBranch(project.normalizedPath).catch(() => null); - return { id: project.id, branch }; - }); - if (cancelled) { - return; - } - setProjectRootBranches((prev) => { - const next = new Map(prev); - entries.forEach(({ id, branch }) => { - if (branch) { - next.set(id, branch); + + // Debounce so the initial burst of per-project `ensureStatus` updates + // settles into a single resolution pass instead of one pass per project. + const timer = setTimeout(() => { + const run = async () => { + const validIds = new Set(normalizedProjects.map((project) => project.id)); + // Drop bookkeeping for projects that are no longer present. + for (const id of resolvedInputKeyByProjectId.current.keys()) { + if (!validIds.has(id)) { + resolvedInputKeyByProjectId.current.delete(id); } + } + + const pending = normalizedProjects.filter((project) => { + const status = gitRepoStatus.get(project.normalizedPath); + if (status?.isGitRepo === false) { + resolvedInputKeyByProjectId.current.delete(project.id); + return false; + } + if (status?.isGitRepo !== true || status.branch === null) { + return false; + } + const currentBranch = status.branch.trim(); + const currentInputKey = `${project.normalizedPath}\0${currentBranch}`; + const lastInputKey = resolvedInputKeyByProjectId.current.get(project.id); + return lastInputKey === undefined || lastInputKey !== currentInputKey; }); - return next; - }); - }; - void run(); + + if (pending.length === 0) { + return; + } + + const entries = await mapWithConcurrency(pending, 2, async (project) => { + const inputBranch = gitRepoStatus.get(project.normalizedPath)?.branch?.trim() ?? ''; + const inputKey = `${project.normalizedPath}\0${inputBranch}`; + const branch = await getRootBranch( + project.normalizedPath, + inputBranch ? { knownBranch: inputBranch } : undefined, + ).catch(() => null); + return { id: project.id, inputKey, branch }; + }); + if (cancelled) { + return; + } + + const resolved = entries.filter((entry) => entry.branch); + if (resolved.length === 0) { + return; + } + + setProjectRootBranches((prev) => { + const next = new Map(prev); + resolved.forEach(({ id, branch }) => { + if (branch) { + next.set(id, branch); + } + }); + return next; + }); + resolved.forEach(({ id, inputKey }) => { + resolvedInputKeyByProjectId.current.set(id, inputKey); + }); + }; + void run(); + }, 150); + return () => { cancelled = true; + clearTimeout(timer); }; - }, [normalizedProjects, projectGitBranchesKey, setProjectRootBranches]); + }, [normalizedProjects, projectGitBranchesKey, gitRepoStatus, setProjectRootBranches]); }; diff --git a/packages/ui/src/lib/worktrees/worktreeManager.ts b/packages/ui/src/lib/worktrees/worktreeManager.ts index 50ee1bec..b54d5c81 100644 --- a/packages/ui/src/lib/worktrees/worktreeManager.ts +++ b/packages/ui/src/lib/worktrees/worktreeManager.ts @@ -9,6 +9,7 @@ import { clearWorktreeBootstrapState, markWorktreeBootstrapPending, } from '@/lib/worktrees/worktreeBootstrap'; +import { invalidateResolvedProjectRootCache } from '@/lib/worktrees/worktreeStatus'; import type { CreateGitWorktreePayload, GitWorktreeValidationResult, @@ -316,6 +317,9 @@ export async function createWorktree(project: ProjectRef, args: CreateWorktreeAr markWorktreeBootstrapPending(metadata.path); _worktreeListCache.delete(projectDirectory); + // The new worktree changes the repo's worktree topology; drop cached root + // resolutions so root-branch lookups re-resolve against the new layout. + invalidateResolvedProjectRootCache(); // Update sidebar store so new worktree appears immediately const sidebarProjectKey = projectDirectory; @@ -358,6 +362,9 @@ export async function removeProjectWorktree(project: ProjectRef, worktree: Workt clearWorktreeBootstrapState(worktree.path); _worktreeListCache.delete(normalizePath(project.path)); + // Removing a worktree changes the repo's worktree topology; drop cached root + // resolutions so root-branch lookups re-resolve against the new layout. + invalidateResolvedProjectRootCache(); // Update sidebar store so removed worktree disappears immediately const normalizedWorktreePath = normalizePath(worktree.path); diff --git a/packages/ui/src/lib/worktrees/worktreeStatus.test.ts b/packages/ui/src/lib/worktrees/worktreeStatus.test.ts new file mode 100644 index 00000000..13ec0b51 --- /dev/null +++ b/packages/ui/src/lib/worktrees/worktreeStatus.test.ts @@ -0,0 +1,153 @@ +import { beforeEach, describe, expect, mock, test } from 'bun:test'; + +type ExecResult = { command: string; success: boolean; stdout?: string }; + +// Per-test controllable behaviour plus manual call tracking (the project's +// tsconfig does not load bun-test's mock matcher types, so existing tests track +// calls via plain arrays rather than `toHaveBeenCalled*`). +let execImpl: (command: string, cwd: string) => ExecResult | Promise = () => ({ command: '', success: false }); +let statusImpl: (directory: string) => { current: string } = () => ({ current: 'HEAD' }); + +const execCalls: Array<{ command: string; cwd: string }> = []; +const statusCalls: string[] = []; + +mock.module('@/lib/execCommands', () => ({ + execCommand: (command: string, cwd: string) => { + execCalls.push({ command, cwd }); + return Promise.resolve(execImpl(command, cwd)); + }, + execCommands: () => Promise.resolve({ success: false, results: [] }), +})); + +mock.module('@/lib/gitApi', () => ({ + getGitStatus: (directory: string) => { + statusCalls.push(directory); + return Promise.resolve(statusImpl(directory)); + }, +})); + +const { getRootBranch, invalidateResolvedProjectRootCache } = await import('./worktreeStatus'); + +// Helper: a single `git rev-parse --absolute-git-dir --git-common-dir` reply. +const revParse = (absoluteGitDir: string, commonDir: string): ExecResult => ({ + command: 'git rev-parse --absolute-git-dir --git-common-dir', + success: true, + stdout: `${absoluteGitDir}\n${commonDir}`, +}); + +describe('worktreeStatus.getRootBranch', () => { + beforeEach(() => { + invalidateResolvedProjectRootCache(); + execCalls.length = 0; + statusCalls.length = 0; + execImpl = () => ({ command: '', success: false }); + statusImpl = () => ({ current: 'HEAD' }); + }); + + test('derives root from absolute-git-dir and returns its branch', async () => { + execImpl = () => revParse('/repo/.git', '.git'); + statusImpl = () => ({ current: 'main' }); + + expect(await getRootBranch('/repo')).toBe('main'); + expect(statusCalls).toEqual(['/repo']); + }); + + test('caches root resolution across repeated calls', async () => { + execImpl = () => revParse('/repo/.git', '.git'); + statusImpl = () => ({ current: 'main' }); + + await getRootBranch('/repo'); + await getRootBranch('/repo'); + await getRootBranch('/repo'); + + // rev-parse runs once; the static root resolution is cached. + expect(execCalls.length).toBe(1); + }); + + test('dedupes concurrent resolutions of the same directory', async () => { + execImpl = () => revParse('/repo/.git', '.git'); + statusImpl = () => ({ current: 'main' }); + + await Promise.all([getRootBranch('/repo'), getRootBranch('/repo'), getRootBranch('/repo')]); + + expect(execCalls.length).toBe(1); + }); + + test('invalidation forces re-resolution', async () => { + execImpl = () => revParse('/repo/.git', '.git'); + statusImpl = () => ({ current: 'main' }); + + await getRootBranch('/repo'); + invalidateResolvedProjectRootCache('/repo'); + await getRootBranch('/repo'); + + expect(execCalls.length).toBe(2); + }); + + test('falls back to the directory itself in a non-git folder', async () => { + execImpl = () => ({ command: '', success: false }); + statusImpl = () => ({ current: 'HEAD' }); + + expect(await getRootBranch('/plain')).toBe('HEAD'); + expect(statusCalls).toEqual(['/plain']); + }); + + test('resolves a linked worktree to its primary root and fetches that branch', async () => { + // Worktree's own git dir lives under the primary repo's .git/worktrees. + execImpl = () => revParse('/repo/.git/worktrees/wt', '/repo/.git'); + statusImpl = () => ({ current: 'main' }); + + // knownBranch is the *worktree* branch, which must NOT be returned for the root. + expect(await getRootBranch('/repo-wt', { knownBranch: 'feature/x' })).toBe('main'); + expect(statusCalls).toEqual(['/repo']); + }); + + test('invalidation mid-flight does not let a stale resolve re-seed the cache', async () => { + let releaseExec: (result: ExecResult) => void = () => {}; + execImpl = () => + new Promise((resolve) => { + releaseExec = resolve; + }); + statusImpl = () => ({ current: 'main' }); + + // Start a resolution and leave it in flight. + const pending = getRootBranch('/repo'); + // A worktree topology change invalidates the cache while the resolve runs. + invalidateResolvedProjectRootCache(); + // Now let the original resolve settle — it must NOT populate the cache. + releaseExec(revParse('/repo/.git', '.git')); + await pending; + + execImpl = () => revParse('/repo/.git', '.git'); + await getRootBranch('/repo'); + + // Second call recomputes because the stale in-flight result was discarded. + expect(execCalls.length).toBe(2); + }); + + test('bounds the root cache by evicting the least-recently-used entry past the count cap', async () => { + execImpl = (_command, cwd) => revParse(`${cwd}/.git`, '.git'); + statusImpl = () => ({ current: 'main' }); + + for (let i = 0; i < 500; i += 1) { + await getRootBranch(`/repo-${i}`); + } + const afterFill = execCalls.length; + expect(afterFill).toBe(500); + + await getRootBranch('/repo-overflow'); + await getRootBranch('/repo-0'); + await getRootBranch('/repo-499'); + + // /repo-overflow and evicted /repo-0 re-run; /repo-499 remains cached. + expect(execCalls.length).toBe(afterFill + 2); + }); + + test('uses knownBranch fast-path when the directory is its own root', async () => { + execImpl = () => revParse('/repo/.git', '.git'); + + expect(await getRootBranch('/repo', { knownBranch: 'develop' })).toBe('develop'); + // No git status round-trip needed in the fast path. + expect(statusCalls).toEqual([]); + }); +}); diff --git a/packages/ui/src/lib/worktrees/worktreeStatus.ts b/packages/ui/src/lib/worktrees/worktreeStatus.ts index aed667be..24ef19b3 100644 --- a/packages/ui/src/lib/worktrees/worktreeStatus.ts +++ b/packages/ui/src/lib/worktrees/worktreeStatus.ts @@ -57,37 +57,159 @@ export async function getWorktreeStatus(worktreePath: string): Promise { - const normalizedPath = normalizePath(projectDirectory); - if (!normalizedPath) { - return 'HEAD'; +// Resolving a project's root (primary worktree) requires shelling out to +// `git rev-parse`, whose answer is effectively static for the lifetime of a +// session — the location of a repo's git directory does not change while the +// app is open. Caching it (with in-flight dedupe) collapses what used to be an +// N² burst of `/api/fs/exec` calls into roughly one resolution per directory. +const RESOLVED_ROOT_TTL_MS = 60_000; +const RESOLVED_ROOT_CACHE_MAX_ENTRIES = 500; +const RESOLVED_ROOT_CACHE_MAX_BYTES = 1024 * 1024; +const resolvedRootCache = new Map(); +const inFlightRootResolves = new Map>(); +// Bumped on every invalidation so a resolution that was already in flight when +// the cache was invalidated does not write its now-stale result back. +let resolveCacheEpoch = 0; + +const resolvedRootEntryBytes = (directory: string, root: string): number => directory.length + root.length; + +const setResolvedRootCacheEntry = (directory: string, root: string): void => { + resolvedRootCache.delete(directory); + resolvedRootCache.set(directory, { root, resolvedAt: Date.now() }); + + let totalBytes = 0; + for (const [key, entry] of resolvedRootCache) { + totalBytes += resolvedRootEntryBytes(key, entry.root); } - const resolveProjectRoot = async (directory: string): Promise => { - const absoluteGitDirResult = await execCommand('git rev-parse --absolute-git-dir', directory); - const absoluteGitDir = normalizePath((absoluteGitDirResult.stdout || '').trim()); - if (absoluteGitDirResult.success && absoluteGitDir) { - const rootFromAbsoluteGitDir = derivePrimaryWorktreeRootFromGitDir(absoluteGitDir); - if (rootFromAbsoluteGitDir) { - return rootFromAbsoluteGitDir; - } + while ( + resolvedRootCache.size > RESOLVED_ROOT_CACHE_MAX_ENTRIES || + (totalBytes > RESOLVED_ROOT_CACHE_MAX_BYTES && resolvedRootCache.size > 1) + ) { + const oldest = resolvedRootCache.entries().next().value; + if (!oldest) { + break; } + totalBytes -= resolvedRootEntryBytes(oldest[0], oldest[1].root); + resolvedRootCache.delete(oldest[0]); + } +}; - const commonDirResult = await execCommand('git rev-parse --git-common-dir', directory); - const rawCommonDir = normalizePath((commonDirResult.stdout || '').trim()); - if (!commonDirResult.success || !rawCommonDir) return directory; +/** + * Invalidate cached project-root resolutions. Call this when the worktree + * topology changes (e.g. after creating or removing a worktree), since that can + * alter which primary root a directory resolves to. With no argument, the whole + * cache is cleared. + */ +export function invalidateResolvedProjectRootCache(directory?: string): void { + // Bumping the epoch prevents any in-flight resolution from re-seeding the + // cache with its pre-invalidation result once it settles. + resolveCacheEpoch += 1; + if (typeof directory === 'string' && directory) { + const normalized = normalizePath(directory); + resolvedRootCache.delete(normalized); + // Drop the in-flight entry too, so callers arriving during the window + // trigger a fresh resolution instead of receiving the stale in-flight one. + inFlightRootResolves.delete(normalized); + return; + } + resolvedRootCache.clear(); + inFlightRootResolves.clear(); +} +const computeProjectRoot = async (directory: string): Promise => { + // A single `git rev-parse` invocation returns both paths (absolute-git-dir on + // the first line, git-common-dir on the second), halving subprocess spawns + // versus issuing the two queries separately. In a non-git directory the whole + // command fails, mirroring the previous fall-through to `directory`. + const result = await execCommand('git rev-parse --absolute-git-dir --git-common-dir', directory); + if (!result.success) { + return directory; + } + + const lines = (result.stdout || '') + .split('\n') + .map((line) => line.trim()) + .filter(Boolean); + + const absoluteGitDir = normalizePath(lines[0] || ''); + if (absoluteGitDir) { + const rootFromAbsoluteGitDir = derivePrimaryWorktreeRootFromGitDir(absoluteGitDir); + if (rootFromAbsoluteGitDir) { + return rootFromAbsoluteGitDir; + } + } + + const rawCommonDir = normalizePath(lines[1] || ''); + if (rawCommonDir) { const commonDir = toAbsolutePath(directory, rawCommonDir); const rootFromCommonDir = derivePrimaryWorktreeRootFromGitDir(commonDir); if (rootFromCommonDir) { return rootFromCommonDir; } + } - return directory; - }; + return directory; +}; + +const resolveProjectRoot = async (directory: string): Promise => { + const cached = resolvedRootCache.get(directory); + if (cached && Date.now() - cached.resolvedAt < RESOLVED_ROOT_TTL_MS) { + // Refresh recency without extending TTL. + resolvedRootCache.delete(directory); + resolvedRootCache.set(directory, cached); + return cached.root; + } + if (cached) { + resolvedRootCache.delete(directory); + } + + const inflight = inFlightRootResolves.get(directory); + if (inflight) { + return inflight; + } + + const startEpoch = resolveCacheEpoch; + const promise = computeProjectRoot(directory) + .then((root) => { + // Skip the write-back if the cache was invalidated while we resolved. + if (resolveCacheEpoch === startEpoch) { + setResolvedRootCacheEntry(directory, root); + } + return root; + }) + .catch(() => directory) + .finally(() => { + if (inFlightRootResolves.get(directory) === promise) { + inFlightRootResolves.delete(directory); + } + }); + + inFlightRootResolves.set(directory, promise); + return promise; +}; + +export async function getRootBranch( + projectDirectory: string, + options?: { knownBranch?: string }, +): Promise { + const normalizedPath = normalizePath(projectDirectory); + if (!normalizedPath) { + return 'HEAD'; + } try { const projectRoot = await resolveProjectRoot(normalizedPath).catch(() => normalizedPath); + + // Fast path: when a project directory *is* its own root (i.e. not a linked + // worktree), the caller's already-known branch refers to the root branch, + // so we can skip a redundant git status round-trip. For linked worktrees the + // root branch differs from the worktree's branch, so we must fetch it. + const knownBranch = options?.knownBranch?.trim(); + if (knownBranch && projectRoot === normalizedPath) { + return knownBranch; + } + const status = await getGitStatus(projectRoot); const branch = typeof status.current === 'string' ? status.current.trim() : ''; return branch || 'HEAD';