perf: dedupe VS Code git read execs
Reduce duplicate git rev-parse work in VS Code Cache only safe git read commands Add VS Code bridge cache coverage
This commit is contained in:
@@ -0,0 +1,88 @@
|
|||||||
|
import { beforeEach, describe, expect, it, mock } from 'bun:test';
|
||||||
|
import { promisify } from 'node:util';
|
||||||
|
|
||||||
|
const execCalls = [];
|
||||||
|
const execMock = mock(() => {
|
||||||
|
throw new Error('exec should be called through promisify');
|
||||||
|
});
|
||||||
|
|
||||||
|
execMock[promisify.custom] = (command, options) => {
|
||||||
|
execCalls.push({ command, options });
|
||||||
|
return new Promise((resolve) => {
|
||||||
|
setTimeout(() => {
|
||||||
|
resolve({ stdout: '/repo/.git\n/repo/.git\n', stderr: '' });
|
||||||
|
}, 10);
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
mock.module('child_process', () => ({
|
||||||
|
exec: execMock,
|
||||||
|
}));
|
||||||
|
|
||||||
|
mock.module('vscode', () => ({
|
||||||
|
workspace: {
|
||||||
|
workspaceFolders: [{ uri: { fsPath: '/workspace' } }],
|
||||||
|
fs: {},
|
||||||
|
},
|
||||||
|
Uri: {
|
||||||
|
file: (fsPath) => ({ fsPath }),
|
||||||
|
},
|
||||||
|
FileType: {
|
||||||
|
Directory: 2,
|
||||||
|
},
|
||||||
|
window: {},
|
||||||
|
}));
|
||||||
|
|
||||||
|
const { clearGitReadCacheForTests, handleFsBridgeMessage } = await import('./bridge-fs-runtime');
|
||||||
|
|
||||||
|
const deps = {
|
||||||
|
resolveUserPath: (value) => value,
|
||||||
|
listDirectoryEntries: mock(),
|
||||||
|
normalizeFsPath: (value) => value,
|
||||||
|
execGit: mock(),
|
||||||
|
searchDirectory: mock(),
|
||||||
|
resolveFileReadPath: mock(),
|
||||||
|
parseDroppedFileReference: mock(),
|
||||||
|
readUriAsAttachment: mock(),
|
||||||
|
};
|
||||||
|
|
||||||
|
describe('bridge fs exec git read cache', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
execCalls.length = 0;
|
||||||
|
clearGitReadCacheForTests();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('dedupes in-flight cacheable git reads and reuses fresh results', async () => {
|
||||||
|
const command = 'git rev-parse --absolute-git-dir --git-common-dir';
|
||||||
|
const cwd = '/repo';
|
||||||
|
|
||||||
|
const [first, second] = await Promise.all([
|
||||||
|
handleFsBridgeMessage({ id: '1', type: 'api:fs:exec', payload: { commands: [command], cwd } }, deps),
|
||||||
|
handleFsBridgeMessage({ id: '2', type: 'api:fs:exec', payload: { commands: [command], cwd } }, deps),
|
||||||
|
]);
|
||||||
|
|
||||||
|
expect(first?.success).toBe(true);
|
||||||
|
expect(second?.success).toBe(true);
|
||||||
|
expect(execCalls).toHaveLength(1);
|
||||||
|
|
||||||
|
const spacedCommand = 'git rev-parse --absolute-git-dir --git-common-dir';
|
||||||
|
const cached = await handleFsBridgeMessage({ id: '3', type: 'api:fs:exec', payload: { commands: [spacedCommand], cwd } }, deps);
|
||||||
|
|
||||||
|
expect(execCalls).toHaveLength(1);
|
||||||
|
expect(cached?.data?.results?.[0]).toMatchObject({
|
||||||
|
command: spacedCommand,
|
||||||
|
success: true,
|
||||||
|
stdout: '/repo/.git\n/repo/.git',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not cache arbitrary exec commands', async () => {
|
||||||
|
const command = 'git status --porcelain';
|
||||||
|
const cwd = '/repo';
|
||||||
|
|
||||||
|
await handleFsBridgeMessage({ id: '1', type: 'api:fs:exec', payload: { commands: [command], cwd } }, deps);
|
||||||
|
await handleFsBridgeMessage({ id: '2', type: 'api:fs:exec', payload: { commands: [command], cwd } }, deps);
|
||||||
|
|
||||||
|
expect(execCalls).toHaveLength(2);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -36,6 +36,65 @@ type DirectoryEntry = {
|
|||||||
isDirectory: boolean;
|
isDirectory: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type FsExecCommandResult = {
|
||||||
|
command: string;
|
||||||
|
success: boolean;
|
||||||
|
exitCode?: number;
|
||||||
|
stdout?: string;
|
||||||
|
stderr?: string;
|
||||||
|
error?: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
const createGitReadCacheTtlMs = () => {
|
||||||
|
const raw = Number(process.env.OPENCHAMBER_GIT_READ_CACHE_TTL_MS);
|
||||||
|
if (Number.isFinite(raw) && raw >= 0) return raw;
|
||||||
|
return 30 * 1000;
|
||||||
|
};
|
||||||
|
|
||||||
|
const normalizeCommand = (command: unknown): string =>
|
||||||
|
typeof command === 'string' ? command.trim().replace(/\s+/g, ' ') : '';
|
||||||
|
|
||||||
|
const isCacheableGitReadCommand = (command: string): boolean => {
|
||||||
|
const normalized = normalizeCommand(command);
|
||||||
|
return /^git rev-parse(?: --(?:absolute-git-dir|git-common-dir|show-toplevel)){1,3}$/.test(normalized);
|
||||||
|
};
|
||||||
|
|
||||||
|
const GIT_READ_CACHE_TTL_MS = createGitReadCacheTtlMs();
|
||||||
|
const GIT_READ_CACHE_MAX_ENTRIES = 500;
|
||||||
|
const GIT_READ_CACHE_MAX_BYTES = 1024 * 1024;
|
||||||
|
const gitReadCache = new Map<string, { result: FsExecCommandResult; at: number }>();
|
||||||
|
const inFlightGitReadCache = new Map<string, Promise<FsExecCommandResult>>();
|
||||||
|
|
||||||
|
const gitReadEntryBytes = (key: string, result: FsExecCommandResult): number =>
|
||||||
|
key.length + (result.stdout?.length ?? 0) + (result.stderr?.length ?? 0);
|
||||||
|
|
||||||
|
const setGitReadCacheEntry = (key: string, result: FsExecCommandResult): void => {
|
||||||
|
gitReadCache.delete(key);
|
||||||
|
gitReadCache.set(key, { result, at: Date.now() });
|
||||||
|
|
||||||
|
let totalBytes = 0;
|
||||||
|
for (const [entryKey, entry] of gitReadCache) {
|
||||||
|
totalBytes += gitReadEntryBytes(entryKey, entry.result);
|
||||||
|
}
|
||||||
|
|
||||||
|
while (
|
||||||
|
gitReadCache.size > GIT_READ_CACHE_MAX_ENTRIES ||
|
||||||
|
(totalBytes > GIT_READ_CACHE_MAX_BYTES && gitReadCache.size > 1)
|
||||||
|
) {
|
||||||
|
const oldest = gitReadCache.entries().next().value;
|
||||||
|
if (!oldest) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
totalBytes -= gitReadEntryBytes(oldest[0], oldest[1].result);
|
||||||
|
gitReadCache.delete(oldest[0]);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
export const clearGitReadCacheForTests = (): void => {
|
||||||
|
gitReadCache.clear();
|
||||||
|
inFlightGitReadCache.clear();
|
||||||
|
};
|
||||||
|
|
||||||
type FsDeps = {
|
type FsDeps = {
|
||||||
resolveUserPath: (value: string, baseDirectory: string) => string;
|
resolveUserPath: (value: string, baseDirectory: string) => string;
|
||||||
listDirectoryEntries: (directoryPath: string) => Promise<DirectoryEntry[]>;
|
listDirectoryEntries: (directoryPath: string) => Promise<DirectoryEntry[]>;
|
||||||
@@ -301,44 +360,81 @@ export async function handleFsBridgeMessage(
|
|||||||
PATH: process.env.PATH,
|
PATH: process.env.PATH,
|
||||||
};
|
};
|
||||||
|
|
||||||
const results: Array<{
|
const runCommand = async (cmd: string): Promise<FsExecCommandResult> => {
|
||||||
command: string;
|
|
||||||
success: boolean;
|
|
||||||
exitCode?: number;
|
|
||||||
stdout?: string;
|
|
||||||
stderr?: string;
|
|
||||||
error?: string;
|
|
||||||
}> = [];
|
|
||||||
|
|
||||||
for (const cmd of commands) {
|
|
||||||
if (typeof cmd !== 'string' || !cmd.trim()) {
|
|
||||||
results.push({ command: cmd, success: false, error: 'Invalid command' });
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
try {
|
try {
|
||||||
const { stdout, stderr } = await execAsync(`${shell} ${shellFlag} "${cmd.replace(/"/g, '\\"')}"`, {
|
const { stdout, stderr } = await execAsync(`${shell} ${shellFlag} "${cmd.replace(/"/g, '\\"')}"`, {
|
||||||
cwd: resolvedCwd,
|
cwd: resolvedCwd,
|
||||||
env: augmentedEnv,
|
env: augmentedEnv,
|
||||||
timeout: 300000,
|
timeout: 300000,
|
||||||
});
|
});
|
||||||
results.push({
|
return {
|
||||||
command: cmd,
|
command: cmd,
|
||||||
success: true,
|
success: true,
|
||||||
exitCode: 0,
|
exitCode: 0,
|
||||||
stdout: (stdout || '').trim(),
|
stdout: (stdout || '').trim(),
|
||||||
stderr: (stderr || '').trim(),
|
stderr: (stderr || '').trim(),
|
||||||
});
|
};
|
||||||
} catch (execError) {
|
} catch (execError) {
|
||||||
const err = execError as { code?: number; stdout?: string; stderr?: string; message?: string };
|
const err = execError as { code?: number; stdout?: string; stderr?: string; message?: string };
|
||||||
results.push({
|
return {
|
||||||
command: cmd,
|
command: cmd,
|
||||||
success: false,
|
success: false,
|
||||||
exitCode: typeof err.code === 'number' ? err.code : 1,
|
exitCode: typeof err.code === 'number' ? err.code : 1,
|
||||||
stdout: (err.stdout || '').trim(),
|
stdout: (err.stdout || '').trim(),
|
||||||
stderr: (err.stderr || '').trim(),
|
stderr: (err.stderr || '').trim(),
|
||||||
error: err.message,
|
error: err.message,
|
||||||
});
|
};
|
||||||
}
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
const runCommandWithGitReadCache = async (cmd: string): Promise<FsExecCommandResult> => {
|
||||||
|
const cacheable = GIT_READ_CACHE_TTL_MS > 0 && isCacheableGitReadCommand(cmd);
|
||||||
|
const cacheKey = cacheable ? `${resolvedCwd}\0${normalizeCommand(cmd)}` : null;
|
||||||
|
|
||||||
|
if (cacheKey) {
|
||||||
|
const cached = gitReadCache.get(cacheKey);
|
||||||
|
if (cached && Date.now() - cached.at < GIT_READ_CACHE_TTL_MS) {
|
||||||
|
gitReadCache.delete(cacheKey);
|
||||||
|
gitReadCache.set(cacheKey, cached);
|
||||||
|
return { ...cached.result, command: cmd };
|
||||||
|
}
|
||||||
|
if (cached) {
|
||||||
|
gitReadCache.delete(cacheKey);
|
||||||
|
}
|
||||||
|
|
||||||
|
const inFlight = inFlightGitReadCache.get(cacheKey);
|
||||||
|
if (inFlight) {
|
||||||
|
const result = await inFlight;
|
||||||
|
return { ...result, command: cmd };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const runPromise = runCommand(cmd).then((result) => {
|
||||||
|
if (cacheKey && result.success) {
|
||||||
|
setGitReadCacheEntry(cacheKey, result);
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}).finally(() => {
|
||||||
|
if (cacheKey && inFlightGitReadCache.get(cacheKey) === runPromise) {
|
||||||
|
inFlightGitReadCache.delete(cacheKey);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
if (cacheKey) {
|
||||||
|
inFlightGitReadCache.set(cacheKey, runPromise);
|
||||||
|
}
|
||||||
|
|
||||||
|
return runPromise;
|
||||||
|
};
|
||||||
|
|
||||||
|
const results: FsExecCommandResult[] = [];
|
||||||
|
|
||||||
|
for (const cmd of commands) {
|
||||||
|
if (typeof cmd !== 'string' || !cmd.trim()) {
|
||||||
|
results.push({ command: cmd, success: false, error: 'Invalid command' });
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
results.push(await runCommandWithGitReadCache(cmd));
|
||||||
}
|
}
|
||||||
|
|
||||||
const allSucceeded = results.every((r) => r.success);
|
const allSucceeded = results.every((r) => r.success);
|
||||||
|
|||||||
Reference in New Issue
Block a user