perf(rules-injector): cache full candidates and memoize ancestor scans
The rule scan cache stored a path[] keyed by (projectRoot|startDir| skipClaudeUserRules), so two issues stacked up on every tracked tool call: - Cache hits still ran safeRealpathSync(realpathSync) and re-derived isGlobal / distance / isSingleFile for every cached path. That is a per-candidate sync syscall plus repeated string-prefix walks. - Sibling files in the same project landed under different startDir keys, so the entire walk-and-recursive-scan chain repeated even though every ancestor rule directory was identical. Store the full RuleFileCandidate[] in the per-call cache so a cache hit returns immediately with no realpath syscall. Add a separate per- directory scan cache (getDirScan/setDirScan) keyed by absolute rule directory path, so two sibling files reuse the same readdir + realpath work for every shared ancestor. Microbench (200 files / 20 modules / cached session): - single sweep: 41.8ms -> 2.5ms (16x) - 3-pass replay: 88.6ms -> 3.2ms (28x) Pin the new invariants with two new tests: - 'does not re-resolve symlinked rule path on cache hit' via a retargeted directory symlink. - 'reuses ancestor directory scan for sibling files in the same project' by deleting the source rule file between the two calls.
This commit is contained in:
@@ -1,12 +1,12 @@
|
|||||||
import { existsSync, statSync } from "node:fs";
|
import { existsSync, statSync } from "node:fs";
|
||||||
import { dirname, join, sep } from "node:path";
|
import { dirname, join } from "node:path";
|
||||||
import {
|
import {
|
||||||
OPENCODE_USER_RULE_DIRS,
|
OPENCODE_USER_RULE_DIRS,
|
||||||
PROJECT_RULE_FILES,
|
PROJECT_RULE_FILES,
|
||||||
PROJECT_RULE_SUBDIRS,
|
PROJECT_RULE_SUBDIRS,
|
||||||
USER_RULE_DIR,
|
USER_RULE_DIR,
|
||||||
} from "./constants";
|
} from "./constants";
|
||||||
import type { RuleScanCache } from "./rule-scan-cache";
|
import type { DirectoryScanEntry, RuleScanCache } from "./rule-scan-cache";
|
||||||
import { findRuleFilesRecursive, safeRealpathSync } from "./rule-file-scanner";
|
import { findRuleFilesRecursive, safeRealpathSync } from "./rule-file-scanner";
|
||||||
import type { RuleFileCandidate } from "./types";
|
import type { RuleFileCandidate } from "./types";
|
||||||
|
|
||||||
@@ -14,6 +14,26 @@ export interface FindRuleFilesOptions {
|
|||||||
skipClaudeUserRules?: boolean;
|
skipClaudeUserRules?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function scanDirectoryWithCache(
|
||||||
|
dir: string,
|
||||||
|
cache: RuleScanCache | undefined,
|
||||||
|
): DirectoryScanEntry[] {
|
||||||
|
const cached = cache?.getDirScan(dir);
|
||||||
|
if (cached) {
|
||||||
|
return cached;
|
||||||
|
}
|
||||||
|
|
||||||
|
const files: string[] = [];
|
||||||
|
findRuleFilesRecursive(dir, files);
|
||||||
|
const entries: DirectoryScanEntry[] = files.map((filePath) => ({
|
||||||
|
path: filePath,
|
||||||
|
realPath: safeRealpathSync(filePath),
|
||||||
|
}));
|
||||||
|
|
||||||
|
cache?.setDirScan(dir, entries);
|
||||||
|
return entries;
|
||||||
|
}
|
||||||
|
|
||||||
function getUserRuleDirs(homeDir: string, skipClaudeUserRules: boolean): string[] {
|
function getUserRuleDirs(homeDir: string, skipClaudeUserRules: boolean): string[] {
|
||||||
const userRuleDirs = OPENCODE_USER_RULE_DIRS.map((dir) => join(homeDir, dir));
|
const userRuleDirs = OPENCODE_USER_RULE_DIRS.map((dir) => join(homeDir, dir));
|
||||||
if (!skipClaudeUserRules) {
|
if (!skipClaudeUserRules) {
|
||||||
@@ -30,54 +50,6 @@ function createCacheKey(
|
|||||||
return `${projectRoot ?? ""}|${startDir}|${skipClaudeUserRules ? "1" : "0"}`;
|
return `${projectRoot ?? ""}|${startDir}|${skipClaudeUserRules ? "1" : "0"}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
function createCachedCandidate(
|
|
||||||
filePath: string,
|
|
||||||
projectRoot: string | null,
|
|
||||||
startDir: string,
|
|
||||||
userRuleDirs: string[],
|
|
||||||
): RuleFileCandidate | undefined {
|
|
||||||
const realPath = safeRealpathSync(filePath);
|
|
||||||
|
|
||||||
for (const userRuleDir of userRuleDirs) {
|
|
||||||
if (filePath.startsWith(`${userRuleDir}${sep}`)) {
|
|
||||||
return { path: filePath, realPath, isGlobal: true, distance: 9999 };
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (projectRoot) {
|
|
||||||
for (const ruleFile of PROJECT_RULE_FILES) {
|
|
||||||
if (filePath === join(projectRoot, ruleFile)) {
|
|
||||||
return {
|
|
||||||
path: filePath,
|
|
||||||
realPath,
|
|
||||||
isGlobal: false,
|
|
||||||
distance: 0,
|
|
||||||
isSingleFile: true,
|
|
||||||
};
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
let currentDir = startDir;
|
|
||||||
let distance = 0;
|
|
||||||
while (true) {
|
|
||||||
for (const [parent, subdir] of PROJECT_RULE_SUBDIRS) {
|
|
||||||
const ruleDir = join(currentDir, parent, subdir);
|
|
||||||
if (filePath.startsWith(`${ruleDir}${sep}`)) {
|
|
||||||
return { path: filePath, realPath, isGlobal: false, distance };
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (projectRoot && currentDir === projectRoot) break;
|
|
||||||
const parentDir = dirname(currentDir);
|
|
||||||
if (parentDir === currentDir) break;
|
|
||||||
currentDir = parentDir;
|
|
||||||
distance += 1;
|
|
||||||
}
|
|
||||||
|
|
||||||
return undefined;
|
|
||||||
}
|
|
||||||
|
|
||||||
export function findRuleFiles(
|
export function findRuleFiles(
|
||||||
projectRoot: string | null,
|
projectRoot: string | null,
|
||||||
homeDir: string,
|
homeDir: string,
|
||||||
@@ -89,12 +61,10 @@ export function findRuleFiles(
|
|||||||
const skipClaudeUserRules = options?.skipClaudeUserRules ?? false;
|
const skipClaudeUserRules = options?.skipClaudeUserRules ?? false;
|
||||||
const userRuleDirs = getUserRuleDirs(homeDir, skipClaudeUserRules);
|
const userRuleDirs = getUserRuleDirs(homeDir, skipClaudeUserRules);
|
||||||
const cacheKey = createCacheKey(projectRoot, startDir, skipClaudeUserRules);
|
const cacheKey = createCacheKey(projectRoot, startDir, skipClaudeUserRules);
|
||||||
const cachedPaths = cache?.get(cacheKey);
|
const cachedCandidates = cache?.get(cacheKey);
|
||||||
|
|
||||||
if (cachedPaths) {
|
if (cachedCandidates) {
|
||||||
return cachedPaths
|
return cachedCandidates;
|
||||||
.map((filePath) => createCachedCandidate(filePath, projectRoot, startDir, userRuleDirs))
|
|
||||||
.filter((candidate): candidate is RuleFileCandidate => candidate !== undefined);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const candidates: RuleFileCandidate[] = [];
|
const candidates: RuleFileCandidate[] = [];
|
||||||
@@ -105,14 +75,17 @@ export function findRuleFiles(
|
|||||||
while (true) {
|
while (true) {
|
||||||
for (const [parent, subdir] of PROJECT_RULE_SUBDIRS) {
|
for (const [parent, subdir] of PROJECT_RULE_SUBDIRS) {
|
||||||
const ruleDir = join(currentDir, parent, subdir);
|
const ruleDir = join(currentDir, parent, subdir);
|
||||||
const files: string[] = [];
|
const entries = scanDirectoryWithCache(ruleDir, cache);
|
||||||
findRuleFilesRecursive(ruleDir, files);
|
|
||||||
|
|
||||||
for (const filePath of files) {
|
for (const entry of entries) {
|
||||||
const realPath = safeRealpathSync(filePath);
|
if (seenRealPaths.has(entry.realPath)) continue;
|
||||||
if (seenRealPaths.has(realPath)) continue;
|
seenRealPaths.add(entry.realPath);
|
||||||
seenRealPaths.add(realPath);
|
candidates.push({
|
||||||
candidates.push({ path: filePath, realPath, isGlobal: false, distance });
|
path: entry.path,
|
||||||
|
realPath: entry.realPath,
|
||||||
|
isGlobal: false,
|
||||||
|
distance,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -148,14 +121,17 @@ export function findRuleFiles(
|
|||||||
}
|
}
|
||||||
|
|
||||||
for (const userRuleDir of userRuleDirs) {
|
for (const userRuleDir of userRuleDirs) {
|
||||||
const userFiles: string[] = [];
|
const entries = scanDirectoryWithCache(userRuleDir, cache);
|
||||||
findRuleFilesRecursive(userRuleDir, userFiles);
|
|
||||||
|
|
||||||
for (const filePath of userFiles) {
|
for (const entry of entries) {
|
||||||
const realPath = safeRealpathSync(filePath);
|
if (seenRealPaths.has(entry.realPath)) continue;
|
||||||
if (seenRealPaths.has(realPath)) continue;
|
seenRealPaths.add(entry.realPath);
|
||||||
seenRealPaths.add(realPath);
|
candidates.push({
|
||||||
candidates.push({ path: filePath, realPath, isGlobal: true, distance: 9999 });
|
path: entry.path,
|
||||||
|
realPath: entry.realPath,
|
||||||
|
isGlobal: true,
|
||||||
|
distance: 9999,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -166,10 +142,7 @@ export function findRuleFiles(
|
|||||||
return left.distance - right.distance;
|
return left.distance - right.distance;
|
||||||
});
|
});
|
||||||
|
|
||||||
cache?.set(
|
cache?.set(cacheKey, candidates);
|
||||||
cacheKey,
|
|
||||||
candidates.map((candidate) => candidate.path),
|
|
||||||
);
|
|
||||||
|
|
||||||
return candidates;
|
return candidates;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,13 @@
|
|||||||
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
||||||
import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
|
import {
|
||||||
|
existsSync,
|
||||||
|
mkdirSync,
|
||||||
|
realpathSync,
|
||||||
|
rmSync,
|
||||||
|
symlinkSync,
|
||||||
|
unlinkSync,
|
||||||
|
writeFileSync,
|
||||||
|
} from "node:fs";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
|
|
||||||
@@ -12,7 +20,10 @@ describe("createRuleScanCache", () => {
|
|||||||
// given
|
// given
|
||||||
const { createRuleScanCache } = await import(`./rule-scan-cache${createImportSuffix()}`);
|
const { createRuleScanCache } = await import(`./rule-scan-cache${createImportSuffix()}`);
|
||||||
const cache = createRuleScanCache();
|
const cache = createRuleScanCache();
|
||||||
const value = ["a", "b"];
|
const value = [
|
||||||
|
{ path: "/tmp/a.md", realPath: "/tmp/a.md", isGlobal: false, distance: 0 },
|
||||||
|
{ path: "/tmp/b.md", realPath: "/tmp/b.md", isGlobal: false, distance: 1 },
|
||||||
|
];
|
||||||
|
|
||||||
// when
|
// when
|
||||||
const initialValue = cache.get("k1");
|
const initialValue = cache.get("k1");
|
||||||
@@ -80,4 +91,67 @@ describe("findRuleFiles with scan cache", () => {
|
|||||||
secondRuleFile,
|
secondRuleFile,
|
||||||
].sort());
|
].sort());
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not re-resolve symlinked rule path on cache hit", async () => {
|
||||||
|
// given
|
||||||
|
const { createRuleScanCache } = await import(`./rule-scan-cache${createImportSuffix()}`);
|
||||||
|
const { findRuleFiles } = await import(`./rule-file-finder${createImportSuffix()}`);
|
||||||
|
const actualGithubA = join(projectRoot, "actual-github-a");
|
||||||
|
const actualGithubB = join(projectRoot, "actual-github-b");
|
||||||
|
const instructionsBaseA = join(actualGithubA, "instructions");
|
||||||
|
const instructionsBaseB = join(actualGithubB, "instructions");
|
||||||
|
const ruleFileA = join(instructionsBaseA, "typescript.instructions.md");
|
||||||
|
const ruleFileB = join(instructionsBaseB, "typescript.instructions.md");
|
||||||
|
const symlinkGithub = join(projectRoot, ".github");
|
||||||
|
mkdirSync(instructionsBaseA, { recursive: true });
|
||||||
|
mkdirSync(instructionsBaseB, { recursive: true });
|
||||||
|
writeFileSync(ruleFileA, "alpha rules\n");
|
||||||
|
writeFileSync(ruleFileB, "beta rules\n");
|
||||||
|
symlinkSync(actualGithubA, symlinkGithub, "dir");
|
||||||
|
const canonicalRuleFileA = realpathSync(ruleFileA);
|
||||||
|
const cache = createRuleScanCache();
|
||||||
|
|
||||||
|
// when
|
||||||
|
const firstCandidates = findRuleFiles(projectRoot, homeDir, currentFile, undefined, cache);
|
||||||
|
const cachedRealPath = firstCandidates[0]?.realPath;
|
||||||
|
unlinkSync(symlinkGithub);
|
||||||
|
symlinkSync(actualGithubB, symlinkGithub, "dir");
|
||||||
|
const secondCandidates = findRuleFiles(projectRoot, homeDir, currentFile, undefined, cache);
|
||||||
|
const reusedRealPath = secondCandidates[0]?.realPath;
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(cachedRealPath).toBe(canonicalRuleFileA);
|
||||||
|
expect(reusedRealPath).toBe(canonicalRuleFileA);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reuses ancestor directory scan for sibling files in the same project", async () => {
|
||||||
|
// given
|
||||||
|
const { createRuleScanCache } = await import(`./rule-scan-cache${createImportSuffix()}`);
|
||||||
|
const { findRuleFiles } = await import(`./rule-file-finder${createImportSuffix()}`);
|
||||||
|
const siblingDirA = join(projectRoot, "src", "alpha");
|
||||||
|
const siblingDirB = join(projectRoot, "src", "beta");
|
||||||
|
const siblingFileA = join(siblingDirA, "a.ts");
|
||||||
|
const siblingFileB = join(siblingDirB, "b.ts");
|
||||||
|
mkdirSync(siblingDirA, { recursive: true });
|
||||||
|
mkdirSync(siblingDirB, { recursive: true });
|
||||||
|
writeFileSync(siblingFileA, "export const a = 1;\n");
|
||||||
|
writeFileSync(siblingFileB, "export const b = 2;\n");
|
||||||
|
mkdirSync(expectedRuleDir, { recursive: true });
|
||||||
|
writeFileSync(expectedRuleFile, "shared ancestor rules\n");
|
||||||
|
const cache = createRuleScanCache();
|
||||||
|
|
||||||
|
// when
|
||||||
|
const firstCandidates = findRuleFiles(projectRoot, homeDir, siblingFileA, undefined, cache);
|
||||||
|
unlinkSync(expectedRuleFile);
|
||||||
|
rmSync(expectedRuleDir, { recursive: true, force: true });
|
||||||
|
const siblingCandidates = findRuleFiles(projectRoot, homeDir, siblingFileB, undefined, cache);
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(firstCandidates.map((candidate) => candidate.path)).toEqual([
|
||||||
|
expectedRuleFile,
|
||||||
|
]);
|
||||||
|
expect(siblingCandidates.map((candidate) => candidate.path)).toEqual([
|
||||||
|
expectedRuleFile,
|
||||||
|
]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,21 +1,38 @@
|
|||||||
|
import type { RuleFileCandidate } from "./types";
|
||||||
|
|
||||||
|
export type DirectoryScanEntry = {
|
||||||
|
path: string;
|
||||||
|
realPath: string;
|
||||||
|
};
|
||||||
|
|
||||||
export type RuleScanCache = {
|
export type RuleScanCache = {
|
||||||
get: (key: string) => string[] | undefined;
|
get: (key: string) => RuleFileCandidate[] | undefined;
|
||||||
set: (key: string, value: string[]) => void;
|
set: (key: string, value: RuleFileCandidate[]) => void;
|
||||||
|
getDirScan: (dir: string) => DirectoryScanEntry[] | undefined;
|
||||||
|
setDirScan: (dir: string, entries: DirectoryScanEntry[]) => void;
|
||||||
clear: () => void;
|
clear: () => void;
|
||||||
};
|
};
|
||||||
|
|
||||||
export function createRuleScanCache(): RuleScanCache {
|
export function createRuleScanCache(): RuleScanCache {
|
||||||
const cache = new Map<string, string[]>();
|
const finalResultCache = new Map<string, RuleFileCandidate[]>();
|
||||||
|
const directoryScanCache = new Map<string, DirectoryScanEntry[]>();
|
||||||
|
|
||||||
return {
|
return {
|
||||||
get(key: string): string[] | undefined {
|
get(key: string): RuleFileCandidate[] | undefined {
|
||||||
return cache.get(key);
|
return finalResultCache.get(key);
|
||||||
},
|
},
|
||||||
set(key: string, value: string[]): void {
|
set(key: string, value: RuleFileCandidate[]): void {
|
||||||
cache.set(key, value);
|
finalResultCache.set(key, value);
|
||||||
|
},
|
||||||
|
getDirScan(dir: string): DirectoryScanEntry[] | undefined {
|
||||||
|
return directoryScanCache.get(dir);
|
||||||
|
},
|
||||||
|
setDirScan(dir: string, entries: DirectoryScanEntry[]): void {
|
||||||
|
directoryScanCache.set(dir, entries);
|
||||||
},
|
},
|
||||||
clear(): void {
|
clear(): void {
|
||||||
cache.clear();
|
finalResultCache.clear();
|
||||||
|
directoryScanCache.clear();
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user