perf(rules-injector): cache match decisions
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -5,13 +5,18 @@ import * as os from "node:os";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { RULES_INJECTOR_STORAGE } from "./constants";
|
||||
import { clearParsedRuleCache, createRuleInjectionProcessor, getParsedRuleCacheStats } from "./injector";
|
||||
import {
|
||||
clearParsedRuleCache,
|
||||
createRuleInjectionProcessor,
|
||||
getParsedRuleCacheStats,
|
||||
} from "./injector";
|
||||
|
||||
type StatSnapshot = { mtimeMs: number; size: number };
|
||||
|
||||
let trackedRulePath = "";
|
||||
let statSnapshots: Array<StatSnapshot | Error> = [];
|
||||
let trackedReadFileCount = 0;
|
||||
let trackedShouldApplyRuleCount = 0;
|
||||
let mockedHomeDir = "";
|
||||
|
||||
const originalReadFileSync = fs.readFileSync.bind(fs);
|
||||
@@ -26,7 +31,7 @@ async function createProcessor(projectRoot: string): Promise<{
|
||||
processFilePathForInjection: (
|
||||
filePath: string,
|
||||
sessionID: string,
|
||||
output: { title: string; output: string; metadata: unknown }
|
||||
output: { title: string; output: string; metadata: unknown },
|
||||
) => Promise<void>;
|
||||
}> {
|
||||
const sessionCaches = new Map<
|
||||
@@ -78,10 +83,15 @@ async function createProcessor(projectRoot: string): Promise<{
|
||||
return originalStatSync(filePath);
|
||||
},
|
||||
homedir: () => mockedHomeDir || originalHomedir(),
|
||||
shouldApplyRule: () => ({ applies: true, reason: "matched" }),
|
||||
isDuplicateByRealPath: (realPath: string, cache: Set<string>) => cache.has(realPath),
|
||||
shouldApplyRule: () => {
|
||||
trackedShouldApplyRuleCount += 1;
|
||||
return { applies: true, reason: "matched" };
|
||||
},
|
||||
isDuplicateByRealPath: (realPath: string, cache: Set<string>) =>
|
||||
cache.has(realPath),
|
||||
createContentHash: (content: string) => `hash:${content}`,
|
||||
isDuplicateByContentHash: (hash: string, cache: Set<string>) => cache.has(hash),
|
||||
isDuplicateByContentHash: (hash: string, cache: Set<string>) =>
|
||||
cache.has(hash),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -107,12 +117,14 @@ describe("createRuleInjectionProcessor", () => {
|
||||
projectRoot,
|
||||
".github",
|
||||
"instructions",
|
||||
"typescript.instructions.md"
|
||||
"typescript.instructions.md",
|
||||
);
|
||||
|
||||
mkdirSync(join(projectRoot, ".git"), { recursive: true });
|
||||
mkdirSync(join(projectRoot, "src"), { recursive: true });
|
||||
mkdirSync(join(projectRoot, ".github", "instructions"), { recursive: true });
|
||||
mkdirSync(join(projectRoot, ".github", "instructions"), {
|
||||
recursive: true,
|
||||
});
|
||||
mkdirSync(homeRoot, { recursive: true });
|
||||
|
||||
writeFileSync(targetFile, "export const value = 1;\n");
|
||||
@@ -122,6 +134,7 @@ describe("createRuleInjectionProcessor", () => {
|
||||
trackedRulePath = ruleFile;
|
||||
statSnapshots = [];
|
||||
trackedReadFileCount = 0;
|
||||
trackedShouldApplyRuleCount = 0;
|
||||
mockedHomeDir = homeRoot;
|
||||
});
|
||||
|
||||
@@ -141,8 +154,16 @@ describe("createRuleInjectionProcessor", () => {
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(targetFile, "session-1", createOutput());
|
||||
await processor.processFilePathForInjection(targetFile, "session-2", createOutput());
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-1",
|
||||
createOutput(),
|
||||
);
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-2",
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(trackedReadFileCount).toBe(1);
|
||||
@@ -157,8 +178,16 @@ describe("createRuleInjectionProcessor", () => {
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(targetFile, "session-1", createOutput());
|
||||
await processor.processFilePathForInjection(targetFile, "session-2", createOutput());
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-1",
|
||||
createOutput(),
|
||||
);
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-2",
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(trackedReadFileCount).toBe(2);
|
||||
@@ -173,13 +202,95 @@ describe("createRuleInjectionProcessor", () => {
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(targetFile, "session-1", createOutput());
|
||||
await processor.processFilePathForInjection(targetFile, "session-2", createOutput());
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-1",
|
||||
createOutput(),
|
||||
);
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-2",
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(trackedReadFileCount).toBe(2);
|
||||
});
|
||||
|
||||
it("reuses match decision when stat fingerprint and target are unchanged", async () => {
|
||||
// given
|
||||
statSnapshots = [
|
||||
{ mtimeMs: 1000, size: 13 },
|
||||
{ mtimeMs: 1000, size: 13 },
|
||||
];
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-1",
|
||||
createOutput(),
|
||||
);
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-2",
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(trackedShouldApplyRuleCount).toBe(1);
|
||||
});
|
||||
|
||||
it("re-evaluates match decision when stat fingerprint changes", async () => {
|
||||
// given
|
||||
statSnapshots = [
|
||||
{ mtimeMs: 1000, size: 13 },
|
||||
{ mtimeMs: 2000, size: 13 },
|
||||
];
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-1",
|
||||
createOutput(),
|
||||
);
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-2",
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(trackedShouldApplyRuleCount).toBe(2);
|
||||
});
|
||||
|
||||
it("keeps match decisions separate for different target files", async () => {
|
||||
// given
|
||||
const secondTargetFile = join(projectRoot, "src", "other.ts");
|
||||
writeFileSync(secondTargetFile, "export const other = 2;\n");
|
||||
statSnapshots = [
|
||||
{ mtimeMs: 1000, size: 13 },
|
||||
{ mtimeMs: 1000, size: 13 },
|
||||
];
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-1",
|
||||
createOutput(),
|
||||
);
|
||||
await processor.processFilePathForInjection(
|
||||
secondTargetFile,
|
||||
"session-2",
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(trackedShouldApplyRuleCount).toBe(2);
|
||||
});
|
||||
|
||||
it("does not cache oversized parsed rule bodies", async () => {
|
||||
// given
|
||||
const largeBody = "x".repeat(70 * 1024);
|
||||
@@ -191,8 +302,16 @@ describe("createRuleInjectionProcessor", () => {
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(targetFile, "session-1", createOutput());
|
||||
await processor.processFilePathForInjection(targetFile, "session-2", createOutput());
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-1",
|
||||
createOutput(),
|
||||
);
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-2",
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(trackedReadFileCount).toBe(2);
|
||||
@@ -223,7 +342,11 @@ describe("createRuleInjectionProcessor", () => {
|
||||
});
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(targetFile, sessionID, createOutput());
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
sessionID,
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(fs.existsSync(injectedPath)).toBe(false);
|
||||
@@ -239,7 +362,11 @@ describe("createRuleInjectionProcessor", () => {
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(targetFile, sessionID, createOutput());
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
sessionID,
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(fs.existsSync(injectedPath)).toBe(true);
|
||||
@@ -255,10 +382,19 @@ describe("createRuleInjectionProcessor", () => {
|
||||
const processor = await createProcessor(projectRoot);
|
||||
|
||||
// when
|
||||
await processor.processFilePathForInjection(targetFile, "session-1", createOutput());
|
||||
await processor.processFilePathForInjection(targetFile, "session-2", createOutput());
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-1",
|
||||
createOutput(),
|
||||
);
|
||||
await processor.processFilePathForInjection(
|
||||
targetFile,
|
||||
"session-2",
|
||||
createOutput(),
|
||||
);
|
||||
|
||||
// then
|
||||
expect(trackedReadFileCount).toBe(2);
|
||||
expect(trackedShouldApplyRuleCount).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -31,7 +31,7 @@ type RuleToInject = {
|
||||
type DynamicTruncator = {
|
||||
truncate: (
|
||||
sessionID: string,
|
||||
content: string
|
||||
content: string,
|
||||
) => Promise<{ result: string; truncated: boolean }>;
|
||||
};
|
||||
|
||||
@@ -44,6 +44,14 @@ interface ParsedRuleEntry {
|
||||
body: string;
|
||||
}
|
||||
|
||||
type ParsedRule = {
|
||||
metadata: RuleMetadata;
|
||||
body: string;
|
||||
statFingerprint: string | null;
|
||||
};
|
||||
|
||||
type MatchDecisionCache = Map<string, string | null>;
|
||||
|
||||
export interface ParsedRuleCacheStats {
|
||||
entries: number;
|
||||
bodyBytes: number;
|
||||
@@ -51,6 +59,7 @@ export interface ParsedRuleCacheStats {
|
||||
|
||||
const MAX_PARSED_RULE_CACHE_ENTRIES = 256;
|
||||
const MAX_PARSED_RULE_CACHE_BODY_BYTES = 64 * 1024;
|
||||
const MAX_MATCH_DECISION_CACHE_ENTRIES = 4096;
|
||||
const parsedRuleCache = new Map<string, ParsedRuleEntry>();
|
||||
|
||||
export function clearParsedRuleCache(): void {
|
||||
@@ -65,8 +74,12 @@ export function getParsedRuleCacheStats(): ParsedRuleCacheStats {
|
||||
return { entries: parsedRuleCache.size, bodyBytes };
|
||||
}
|
||||
|
||||
function setParsedRuleCacheEntry(realPath: string, entry: ParsedRuleEntry): void {
|
||||
if (Buffer.byteLength(entry.body, "utf8") > MAX_PARSED_RULE_CACHE_BODY_BYTES) return;
|
||||
function setParsedRuleCacheEntry(
|
||||
realPath: string,
|
||||
entry: ParsedRuleEntry,
|
||||
): void {
|
||||
if (Buffer.byteLength(entry.body, "utf8") > MAX_PARSED_RULE_CACHE_BODY_BYTES)
|
||||
return;
|
||||
if (parsedRuleCache.size >= MAX_PARSED_RULE_CACHE_ENTRIES) {
|
||||
const oldestRealPath = parsedRuleCache.keys().next().value;
|
||||
if (oldestRealPath !== undefined) {
|
||||
@@ -78,7 +91,7 @@ function setParsedRuleCacheEntry(realPath: string, entry: ParsedRuleEntry): void
|
||||
|
||||
function resolveFilePath(
|
||||
workspaceDirectory: string,
|
||||
path: string
|
||||
path: string,
|
||||
): string | null {
|
||||
if (!path) return null;
|
||||
if (path.startsWith("/")) return path;
|
||||
@@ -103,7 +116,7 @@ export function createRuleInjectionProcessor(deps: {
|
||||
processFilePathForInjection: (
|
||||
filePath: string,
|
||||
sessionID: string,
|
||||
output: ToolExecuteOutput
|
||||
output: ToolExecuteOutput,
|
||||
) => Promise<void>;
|
||||
} {
|
||||
const {
|
||||
@@ -118,17 +131,29 @@ export function createRuleInjectionProcessor(deps: {
|
||||
shouldApplyRule: shouldApplyRuleImpl = shouldApplyRule,
|
||||
isDuplicateByRealPath: isDuplicateByRealPathImpl = isDuplicateByRealPath,
|
||||
createContentHash: createContentHashImpl = createContentHash,
|
||||
isDuplicateByContentHash: isDuplicateByContentHashImpl = isDuplicateByContentHash,
|
||||
isDuplicateByContentHash:
|
||||
isDuplicateByContentHashImpl = isDuplicateByContentHash,
|
||||
saveInjectedRules: saveInjectedRulesImpl = saveInjectedRules,
|
||||
} = deps;
|
||||
|
||||
function getParsedRule(filePath: string, realPath: string): { metadata: RuleMetadata; body: string } {
|
||||
const matchDecisionCache: MatchDecisionCache = new Map();
|
||||
|
||||
function getParsedRule(filePath: string, realPath: string): ParsedRule {
|
||||
try {
|
||||
const stat = statRuleSync(filePath);
|
||||
const statFingerprint = `${stat.mtimeMs}:${stat.size}`;
|
||||
const cached = parsedRuleCache.get(realPath);
|
||||
|
||||
if (cached && cached.mtimeMs === stat.mtimeMs && cached.size === stat.size) {
|
||||
return { metadata: cached.metadata, body: cached.body };
|
||||
if (
|
||||
cached &&
|
||||
cached.mtimeMs === stat.mtimeMs &&
|
||||
cached.size === stat.size
|
||||
) {
|
||||
return {
|
||||
metadata: cached.metadata,
|
||||
body: cached.body,
|
||||
statFingerprint,
|
||||
};
|
||||
}
|
||||
|
||||
const rawContent = readRuleFileSync(filePath, "utf-8");
|
||||
@@ -139,17 +164,18 @@ export function createRuleInjectionProcessor(deps: {
|
||||
metadata,
|
||||
body,
|
||||
});
|
||||
return { metadata, body };
|
||||
return { metadata, body, statFingerprint };
|
||||
} catch {
|
||||
const rawContent = readRuleFileSync(filePath, "utf-8");
|
||||
return parseRuleFrontmatter(rawContent);
|
||||
const { metadata, body } = parseRuleFrontmatter(rawContent);
|
||||
return { metadata, body, statFingerprint: null };
|
||||
}
|
||||
}
|
||||
|
||||
async function processFilePathForInjection(
|
||||
filePath: string,
|
||||
sessionID: string,
|
||||
output: ToolExecuteOutput
|
||||
output: ToolExecuteOutput,
|
||||
): Promise<void> {
|
||||
const resolved = resolveFilePath(workspaceDirectory, filePath);
|
||||
if (!resolved) return;
|
||||
@@ -170,25 +196,61 @@ export function createRuleInjectionProcessor(deps: {
|
||||
let dirty = false;
|
||||
|
||||
for (const candidate of ruleFileCandidates) {
|
||||
if (isDuplicateByRealPathImpl(candidate.realPath, cache.realPaths)) continue;
|
||||
if (isDuplicateByRealPathImpl(candidate.realPath, cache.realPaths))
|
||||
continue;
|
||||
|
||||
try {
|
||||
const { metadata, body } = getParsedRule(
|
||||
const { metadata, body, statFingerprint } = getParsedRule(
|
||||
candidate.path,
|
||||
candidate.realPath
|
||||
candidate.realPath,
|
||||
);
|
||||
|
||||
let matchReason: string;
|
||||
if (candidate.isSingleFile) {
|
||||
matchReason = "copilot-instructions (always apply)";
|
||||
} else {
|
||||
const matchResult = shouldApplyRuleImpl(metadata, resolved, projectRoot);
|
||||
if (!matchResult.applies) continue;
|
||||
const cachedMatchReason = getCachedMatchReason(
|
||||
matchDecisionCache,
|
||||
projectRoot,
|
||||
resolved,
|
||||
candidate.realPath,
|
||||
statFingerprint,
|
||||
);
|
||||
if (cachedMatchReason !== undefined) {
|
||||
if (cachedMatchReason === null) continue;
|
||||
matchReason = cachedMatchReason;
|
||||
} else {
|
||||
const matchResult = shouldApplyRuleImpl(
|
||||
metadata,
|
||||
resolved,
|
||||
projectRoot,
|
||||
);
|
||||
if (!matchResult.applies) {
|
||||
setCachedMatchReason(
|
||||
matchDecisionCache,
|
||||
projectRoot,
|
||||
resolved,
|
||||
candidate.realPath,
|
||||
statFingerprint,
|
||||
null,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
matchReason = matchResult.reason ?? "matched";
|
||||
setCachedMatchReason(
|
||||
matchDecisionCache,
|
||||
projectRoot,
|
||||
resolved,
|
||||
candidate.realPath,
|
||||
statFingerprint,
|
||||
matchReason,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const contentHash = createContentHashImpl(body);
|
||||
if (isDuplicateByContentHashImpl(contentHash, cache.contentHashes)) continue;
|
||||
if (isDuplicateByContentHashImpl(contentHash, cache.contentHashes))
|
||||
continue;
|
||||
|
||||
const relativePath = projectRoot
|
||||
? relative(projectRoot, candidate.path)
|
||||
@@ -214,7 +276,7 @@ export function createRuleInjectionProcessor(deps: {
|
||||
for (const rule of toInject) {
|
||||
const { result, truncated } = await truncator.truncate(
|
||||
sessionID,
|
||||
rule.content
|
||||
rule.content,
|
||||
);
|
||||
const truncationNotice = truncated
|
||||
? `\n\n[Note: Content was truncated to save context window space. For full context, please read the file directly: ${rule.relativePath}]`
|
||||
@@ -229,3 +291,61 @@ export function createRuleInjectionProcessor(deps: {
|
||||
|
||||
return { processFilePathForInjection };
|
||||
}
|
||||
|
||||
function getCachedMatchReason(
|
||||
cache: MatchDecisionCache,
|
||||
projectRoot: string | null,
|
||||
resolvedFilePath: string,
|
||||
realPath: string,
|
||||
statFingerprint: string | null,
|
||||
): string | null | undefined {
|
||||
const cacheKey = matchDecisionCacheKey(
|
||||
projectRoot,
|
||||
resolvedFilePath,
|
||||
realPath,
|
||||
statFingerprint,
|
||||
);
|
||||
if (cacheKey === null || !cache.has(cacheKey)) return undefined;
|
||||
|
||||
const cached = cache.get(cacheKey) ?? null;
|
||||
cache.delete(cacheKey);
|
||||
cache.set(cacheKey, cached);
|
||||
return cached;
|
||||
}
|
||||
|
||||
function setCachedMatchReason(
|
||||
cache: MatchDecisionCache,
|
||||
projectRoot: string | null,
|
||||
resolvedFilePath: string,
|
||||
realPath: string,
|
||||
statFingerprint: string | null,
|
||||
matchReason: string | null,
|
||||
): void {
|
||||
const cacheKey = matchDecisionCacheKey(
|
||||
projectRoot,
|
||||
resolvedFilePath,
|
||||
realPath,
|
||||
statFingerprint,
|
||||
);
|
||||
if (cacheKey === null) return;
|
||||
|
||||
if (cache.size >= MAX_MATCH_DECISION_CACHE_ENTRIES) {
|
||||
const oldestCacheKey = cache.keys().next().value;
|
||||
if (oldestCacheKey !== undefined) {
|
||||
cache.delete(oldestCacheKey);
|
||||
}
|
||||
}
|
||||
cache.set(cacheKey, matchReason);
|
||||
}
|
||||
|
||||
function matchDecisionCacheKey(
|
||||
projectRoot: string | null,
|
||||
resolvedFilePath: string,
|
||||
realPath: string,
|
||||
statFingerprint: string | null,
|
||||
): string | null {
|
||||
if (statFingerprint === null) return null;
|
||||
return [projectRoot ?? "", resolvedFilePath, realPath, statFingerprint].join(
|
||||
"\0",
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user