mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
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.
This commit is contained in:
parent
9ac87ae60f
commit
382801790c
2 changed files with 76 additions and 7 deletions
|
|
@ -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<string, string | null>();
|
||||
const coreExcludesFilePathCache = new Map<string, string>();
|
||||
|
||||
/**
|
||||
* 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;
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue