From 382801790cf41b12775ca39ae58eb9fea48033db Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Tue, 21 Jul 2026 19:09:25 +0000 Subject: [PATCH] perf(config): memoize core.excludesFile / info/exclude resolution (#2606) loadIgnoreRules is called once per repo, per language/contract extractor during group sync -- an N-repo group fans out to 6+ extractors each calling it, turning an uncached execSync per call into O(extractors x repos) blocking subprocess spawns for the exact many-repos scenario #2606 describes. Both getGitInfoExcludePath and getCoreExcludesFilePath resolve to the same value for the same fromPath for the life of the process, so memoize by fromPath in a process-lifetime Map. One-shot CLI runs are unaffected by staleness; the long-lived MCP server would need explicit invalidation if this becomes a real concern. --- gitnexus/src/storage/git.ts | 37 ++++++++++++++++++++++----- gitnexus/test/unit/git.test.ts | 46 +++++++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/gitnexus/src/storage/git.ts b/gitnexus/src/storage/git.ts index 898211f5c..4b4fdd0f1 100644 --- a/gitnexus/src/storage/git.ts +++ b/gitnexus/src/storage/git.ts @@ -210,6 +210,18 @@ export const getCanonicalRepoRoot = (fromPath: string): string | null => { } }; +// getGitInfoExcludePath/getCoreExcludesFilePath are called once per repo +// PER language/contract extractor during group sync (#2606) — an N-repo +// group fans out to 6+ extractors each calling these, so an uncached +// execSync per call turns into O(extractors × repos) blocking subprocess +// spawns. Both resolve to the same value for the same fromPath for the +// life of the process (git config/exclude files don't change mid-run), so +// memoize by fromPath. ponytail: process-lifetime cache, never invalidated +// — fine for one-shot CLI runs; the long-lived MCP server would need a +// TTL or explicit invalidation if a user edits core.excludesFile mid-session. +const gitInfoExcludePathCache = new Map(); +const coreExcludesFilePathCache = new Map(); + /** * Path to the repo's `$GIT_COMMON_DIR/info/exclude` file — git's own * per-repo, untracked exclude list (same tier as `.gitignore` in @@ -221,6 +233,10 @@ export const getCanonicalRepoRoot = (fromPath: string): string | null => { * is unavailable; callers should treat that the same as "no file". */ export const getGitInfoExcludePath = (fromPath: string): string | null => { + const cached = gitInfoExcludePathCache.get(fromPath); + if (cached !== undefined) return cached; + + let result: string | null; try { const commonDir = chompGitOutput( execSync('git rev-parse --path-format=absolute --git-common-dir', { @@ -229,11 +245,12 @@ export const getGitInfoExcludePath = (fromPath: string): string | null => { windowsHide: true, }), ); - if (!commonDir) return null; - return path.join(path.resolve(commonDir), 'info', 'exclude'); + result = commonDir ? path.join(path.resolve(commonDir), 'info', 'exclude') : null; } catch { - return null; + result = null; } + gitInfoExcludePathCache.set(fromPath, result); + return result; }; /** @@ -247,6 +264,10 @@ export const getGitInfoExcludePath = (fromPath: string): string | null => { * default path, which is always computable without `git`. */ export const getCoreExcludesFilePath = (fromPath: string): string => { + const cached = coreExcludesFilePathCache.get(fromPath); + if (cached !== undefined) return cached; + + let result: string | undefined; try { const configured = chompGitOutput( execSync('git config --get --type=path core.excludesFile', { @@ -255,12 +276,16 @@ export const getCoreExcludesFilePath = (fromPath: string): string => { windowsHide: true, }), ); - if (configured) return configured; + if (configured) result = configured; } catch { // Unset, or git unavailable — fall through to git's documented default. } - const xdgConfigHome = process.env.XDG_CONFIG_HOME || path.join(os.homedir(), '.config'); - return path.join(xdgConfigHome, 'git', 'ignore'); + if (!result) { + const xdgConfigHome = process.env.XDG_CONFIG_HOME || path.join(os.homedir(), '.config'); + result = path.join(xdgConfigHome, 'git', 'ignore'); + } + coreExcludesFilePathCache.set(fromPath, result); + return result; }; /** diff --git a/gitnexus/test/unit/git.test.ts b/gitnexus/test/unit/git.test.ts index 3acab2bba..b99a75566 100644 --- a/gitnexus/test/unit/git.test.ts +++ b/gitnexus/test/unit/git.test.ts @@ -346,7 +346,10 @@ describe('git utilities', () => { throw new Error('key not set'); // git config --get exits 1 when unset }); process.env.XDG_CONFIG_HOME = '/home/user/.config'; - expect(getCoreExcludesFilePath('/repo')).toBe( + // Different fromPath than the "configured" test above — each function + // caches by fromPath (see below), so reusing '/repo' here would return + // that test's cached result instead of exercising the fallback. + expect(getCoreExcludesFilePath('/repo-unconfigured')).toBe( path.join('/home/user/.config', 'git', 'ignore'), ); }); @@ -361,4 +364,45 @@ describe('git utilities', () => { ); }); }); + + // A group sync calls loadIgnoreRules (and therefore these two functions) + // once per repo, per extractor — repeated calls with the same fromPath + // are the normal case, not an edge case. Both functions memoize by + // fromPath so a second call never spawns a second subprocess (#2606). + describe('getGitInfoExcludePath / getCoreExcludesFilePath caching (#2606)', () => { + it('getGitInfoExcludePath only spawns git once for repeated calls with the same fromPath', () => { + mockExecSync.mockReturnValueOnce(Buffer.from('/cached-repo/.git\n')); + const first = getGitInfoExcludePath('/cached-repo'); + const second = getGitInfoExcludePath('/cached-repo'); + expect(first).toBe(path.join('/cached-repo/.git', 'info', 'exclude')); + expect(second).toBe(first); + expect(mockExecSync).toHaveBeenCalledTimes(1); + }); + + it('getGitInfoExcludePath caches a null result too (not-a-git-repo stays cheap)', () => { + mockExecSync.mockImplementationOnce(() => { + throw new Error('not a git repo'); + }); + expect(getGitInfoExcludePath('/cached-non-repo')).toBeNull(); + expect(getGitInfoExcludePath('/cached-non-repo')).toBeNull(); + expect(mockExecSync).toHaveBeenCalledTimes(1); + }); + + it('getCoreExcludesFilePath only spawns git once for repeated calls with the same fromPath', () => { + mockExecSync.mockReturnValueOnce(Buffer.from('/home/user/.gitignore_global\n')); + const first = getCoreExcludesFilePath('/cached-repo-2'); + const second = getCoreExcludesFilePath('/cached-repo-2'); + expect(first).toBe('/home/user/.gitignore_global'); + expect(second).toBe(first); + expect(mockExecSync).toHaveBeenCalledTimes(1); + }); + + it("a different fromPath is not served from another path's cache entry", () => { + mockExecSync.mockReturnValueOnce(Buffer.from('/repo-a/.git\n')); + mockExecSync.mockReturnValueOnce(Buffer.from('/repo-b/.git\n')); + expect(getGitInfoExcludePath('/repo-a')).toBe(path.join('/repo-a/.git', 'info', 'exclude')); + expect(getGitInfoExcludePath('/repo-b')).toBe(path.join('/repo-b/.git', 'info', 'exclude')); + expect(mockExecSync).toHaveBeenCalledTimes(2); + }); + }); });