From 2dd67e83b24ab4004a48505257d82a3bdee92ee1 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 30 Aug 2026 23:04:13 -0700 Subject: [PATCH] perf(worktree): overlap configured path filesystem probes (#17453) --- ...ktree-configured-paths-concurrency.test.ts | 83 +++++++++++++++++++ src/main/git/worktree-include-file.ts | 30 +++++-- src/main/git/worktree-shared-directories.ts | 43 +++++++--- 3 files changed, 135 insertions(+), 21 deletions(-) create mode 100644 src/main/git/worktree-configured-paths-concurrency.test.ts diff --git a/src/main/git/worktree-configured-paths-concurrency.test.ts b/src/main/git/worktree-configured-paths-concurrency.test.ts new file mode 100644 index 00000000000..c3b54044f77 --- /dev/null +++ b/src/main/git/worktree-configured-paths-concurrency.test.ts @@ -0,0 +1,83 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { statMock, lstatMock, readFileMock, loadHooksMock, checkIgnoredPathsMock, concurrency } = + vi.hoisted(() => ({ + statMock: vi.fn(), + lstatMock: vi.fn(), + readFileMock: vi.fn(), + loadHooksMock: vi.fn(), + checkIgnoredPathsMock: vi.fn(), + concurrency: { active: 0, max: 0 } + })) + +vi.mock('node:fs/promises', () => ({ + stat: statMock, + lstat: lstatMock, + readFile: readFileMock +})) + +vi.mock('../hooks', () => ({ + loadHooks: loadHooksMock +})) + +vi.mock('./check-ignored-paths', () => ({ + checkIgnoredPaths: checkIgnoredPathsMock +})) + +import { resolveWorktreeIncludePaths } from './worktree-include-file' +import { resolveWorktreeSharedDirectories } from './worktree-shared-directories' + +const PATH_COUNT = 16 +const configuredPaths = Array.from({ length: PATH_COUNT }, (_, index) => `path-${index}`) + +async function delayedProbe(value: T): Promise { + concurrency.active += 1 + concurrency.max = Math.max(concurrency.max, concurrency.active) + await new Promise((resolve) => setTimeout(resolve, 5)) + concurrency.active -= 1 + return value +} + +describe('configured worktree path probe concurrency', () => { + beforeEach(() => { + statMock.mockReset() + lstatMock.mockReset() + readFileMock.mockReset() + loadHooksMock.mockReset() + checkIgnoredPathsMock.mockReset() + concurrency.active = 0 + concurrency.max = 0 + checkIgnoredPathsMock.mockResolvedValue(configuredPaths) + }) + + it('bounds shared-directory stats while retaining configured order', async () => { + loadHooksMock.mockReturnValue({ worktree: { sharedDirectories: configuredPaths } }) + statMock.mockImplementation(async () => delayedProbe({ isDirectory: () => true })) + + const result = await resolveWorktreeSharedDirectories('/repo') + + expect(result).toEqual([...configuredPaths].sort()) + expect(checkIgnoredPathsMock).toHaveBeenCalledWith('/repo', configuredPaths, {}) + expect(concurrency.max).toBeGreaterThan(1) + expect(concurrency.max).toBeLessThanOrEqual(8) + }) + + it('bounds include-path lstat probes while retaining candidate order', async () => { + const includePath = '/repo/.worktreeinclude' + loadHooksMock.mockReturnValue(null) + lstatMock.mockImplementation(async (path: string) => { + if (path === includePath) { + return { isFile: () => true, size: 1 } + } + return delayedProbe({}) + }) + readFileMock.mockResolvedValue(configuredPaths.join('\n')) + + const result = await resolveWorktreeIncludePaths('/repo') + + expect(result).toEqual([...configuredPaths].sort()) + expect(checkIgnoredPathsMock).toHaveBeenCalledWith('/repo', configuredPaths, {}) + expect(concurrency.max).toBeGreaterThan(1) + expect(concurrency.max).toBeLessThanOrEqual(8) + }) +}) diff --git a/src/main/git/worktree-include-file.ts b/src/main/git/worktree-include-file.ts index b387671fe24..edded1a1ba2 100644 --- a/src/main/git/worktree-include-file.ts +++ b/src/main/git/worktree-include-file.ts @@ -2,6 +2,7 @@ import { lstat, readFile } from 'node:fs/promises' import { isAbsolute, join } from 'node:path' import { checkIgnoredPaths } from './check-ignored-paths' import type { GitRuntimeOptions } from './git-runtime-options' +import { mapWithConcurrency } from '../../shared/map-with-concurrency' /** Project-level list of gitignored paths to copy into each new worktree. * Cross-tool convention (see issue #7549). */ @@ -16,6 +17,9 @@ export const WORKTREE_INCLUDE_FILE = '.worktreeinclude' const WORKTREE_INCLUDE_MAX_FILE_BYTES = 256 * 1024 // Why: bound the work a single repo file can request; entries beyond this are ignored. const WORKTREE_INCLUDE_MAX_ENTRIES = 1000 +// Why: include files can name hundreds of independent paths; bound the local +// stat fan-out while avoiding one serial filesystem round trip per entry. +const WORKTREE_INCLUDE_PATH_STAT_CONCURRENCY = 8 /** Parse `.worktreeinclude` into deduped, repo-root-relative literal paths. * Blank lines and `#` comments are skipped; `\` is normalized to `/`, a `./` @@ -109,16 +113,24 @@ export async function resolveWorktreeIncludePaths( } // Keep only entries present in the primary checkout — a listed but absent - // path (e.g. node_modules before install) has nothing to copy. - const existing: string[] = [] - for (const relativePath of candidates) { - try { - await lstat(join(repoPath, relativePath)) - existing.push(relativePath) - } catch { - // Absent in the primary checkout — nothing to copy. + // path (e.g. node_modules before install) has nothing to copy. The mapper + // retains candidate order for deterministic git-ignore input and output. + const existence = await mapWithConcurrency( + candidates, + WORKTREE_INCLUDE_PATH_STAT_CONCURRENCY, + async (relativePath) => { + try { + await lstat(join(repoPath, relativePath)) + return relativePath + } catch { + // Absent in the primary checkout — nothing to copy. + return null + } } - } + ) + const existing = existence.filter( + (relativePath): relativePath is string => relativePath !== null + ) if (existing.length === 0) { return [] } diff --git a/src/main/git/worktree-shared-directories.ts b/src/main/git/worktree-shared-directories.ts index d725e80c8f8..1dfe990a0e6 100644 --- a/src/main/git/worktree-shared-directories.ts +++ b/src/main/git/worktree-shared-directories.ts @@ -4,11 +4,15 @@ import { checkIgnoredPaths } from './check-ignored-paths' import type { GitRuntimeOptions } from './git-runtime-options' import { loadHooks } from '../hooks' import type { Repo } from '../../shared/repo-types' +import { mapWithConcurrency } from '../../shared/map-with-concurrency' // Why: a fresh worktree has no node_modules/.cache, and copying them is slow and // duplicates disk; `orca.yaml` names the ones every worktree should share instead. const CONFIGURED_SHARED_DIRECTORIES_CACHE_TTL_MS = 30_000 +// Why: resolving a worktree may list many generated directories; overlap +// independent local probes without flooding the filesystem threadpool. +const SHARED_DIRECTORY_STAT_CONCURRENCY = 8 const configuredSharedDirectoriesByRepoPath = new Map< string, { directories: string[]; expiresAt: number } @@ -76,19 +80,34 @@ export async function resolveWorktreeSharedDirectories( } // Keep only entries that exist as directories; a listed but absent path - // (node_modules before install) has nothing to share. - const existing: string[] = [] - for (const relativePath of configured) { - try { - if ((await stat(join(repoPath, relativePath))).isDirectory()) { - existing.push(relativePath) - } else { - console.warn( - `[worktree-shared-directories] Skipping "${relativePath}": sharedDirectories entries must be directories` - ) + // (node_modules before install) has nothing to share. The mapper retains + // configured order; warnings are emitted below in that same order. + const probes = await mapWithConcurrency( + configured, + SHARED_DIRECTORY_STAT_CONCURRENCY, + async (relativePath) => { + try { + return { + relativePath, + exists: true, + isDirectory: (await stat(join(repoPath, relativePath))).isDirectory() + } + } catch { + return { relativePath, exists: false, isDirectory: false } } - } catch { - // Absent in the primary checkout — nothing to share. + } + ) + const existing: string[] = [] + for (const probe of probes) { + if (!probe.exists) { + continue + } + if (probe.isDirectory) { + existing.push(probe.relativePath) + } else { + console.warn( + `[worktree-shared-directories] Skipping "${probe.relativePath}": sharedDirectories entries must be directories` + ) } } if (existing.length === 0) {