From 271878bcea5569250c4863557e5d695bbaf59d48 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 17 May 2026 01:53:24 +0900 Subject: [PATCH] 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. --- src/hooks/rules-injector/rule-file-finder.ts | 119 +++++++----------- .../rules-injector/rule-scan-cache.test.ts | 78 +++++++++++- src/hooks/rules-injector/rule-scan-cache.ts | 33 +++-- 3 files changed, 147 insertions(+), 83 deletions(-) diff --git a/src/hooks/rules-injector/rule-file-finder.ts b/src/hooks/rules-injector/rule-file-finder.ts index 7059804d4..c2e60e68d 100644 --- a/src/hooks/rules-injector/rule-file-finder.ts +++ b/src/hooks/rules-injector/rule-file-finder.ts @@ -1,12 +1,12 @@ import { existsSync, statSync } from "node:fs"; -import { dirname, join, sep } from "node:path"; +import { dirname, join } from "node:path"; import { OPENCODE_USER_RULE_DIRS, PROJECT_RULE_FILES, PROJECT_RULE_SUBDIRS, USER_RULE_DIR, } 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 type { RuleFileCandidate } from "./types"; @@ -14,6 +14,26 @@ export interface FindRuleFilesOptions { 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[] { const userRuleDirs = OPENCODE_USER_RULE_DIRS.map((dir) => join(homeDir, dir)); if (!skipClaudeUserRules) { @@ -30,54 +50,6 @@ function createCacheKey( 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( projectRoot: string | null, homeDir: string, @@ -89,12 +61,10 @@ export function findRuleFiles( const skipClaudeUserRules = options?.skipClaudeUserRules ?? false; const userRuleDirs = getUserRuleDirs(homeDir, skipClaudeUserRules); const cacheKey = createCacheKey(projectRoot, startDir, skipClaudeUserRules); - const cachedPaths = cache?.get(cacheKey); + const cachedCandidates = cache?.get(cacheKey); - if (cachedPaths) { - return cachedPaths - .map((filePath) => createCachedCandidate(filePath, projectRoot, startDir, userRuleDirs)) - .filter((candidate): candidate is RuleFileCandidate => candidate !== undefined); + if (cachedCandidates) { + return cachedCandidates; } const candidates: RuleFileCandidate[] = []; @@ -105,14 +75,17 @@ export function findRuleFiles( while (true) { for (const [parent, subdir] of PROJECT_RULE_SUBDIRS) { const ruleDir = join(currentDir, parent, subdir); - const files: string[] = []; - findRuleFilesRecursive(ruleDir, files); + const entries = scanDirectoryWithCache(ruleDir, cache); - for (const filePath of files) { - const realPath = safeRealpathSync(filePath); - if (seenRealPaths.has(realPath)) continue; - seenRealPaths.add(realPath); - candidates.push({ path: filePath, realPath, isGlobal: false, distance }); + for (const entry of entries) { + if (seenRealPaths.has(entry.realPath)) continue; + seenRealPaths.add(entry.realPath); + candidates.push({ + path: entry.path, + realPath: entry.realPath, + isGlobal: false, + distance, + }); } } @@ -148,14 +121,17 @@ export function findRuleFiles( } for (const userRuleDir of userRuleDirs) { - const userFiles: string[] = []; - findRuleFilesRecursive(userRuleDir, userFiles); + const entries = scanDirectoryWithCache(userRuleDir, cache); - for (const filePath of userFiles) { - const realPath = safeRealpathSync(filePath); - if (seenRealPaths.has(realPath)) continue; - seenRealPaths.add(realPath); - candidates.push({ path: filePath, realPath, isGlobal: true, distance: 9999 }); + for (const entry of entries) { + if (seenRealPaths.has(entry.realPath)) continue; + seenRealPaths.add(entry.realPath); + candidates.push({ + path: entry.path, + realPath: entry.realPath, + isGlobal: true, + distance: 9999, + }); } } @@ -166,10 +142,7 @@ export function findRuleFiles( return left.distance - right.distance; }); - cache?.set( - cacheKey, - candidates.map((candidate) => candidate.path), - ); + cache?.set(cacheKey, candidates); return candidates; } diff --git a/src/hooks/rules-injector/rule-scan-cache.test.ts b/src/hooks/rules-injector/rule-scan-cache.test.ts index f6a037a4b..733bf98cf 100644 --- a/src/hooks/rules-injector/rule-scan-cache.test.ts +++ b/src/hooks/rules-injector/rule-scan-cache.test.ts @@ -1,5 +1,13 @@ 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 { join } from "node:path"; @@ -12,7 +20,10 @@ describe("createRuleScanCache", () => { // given const { createRuleScanCache } = await import(`./rule-scan-cache${createImportSuffix()}`); 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 const initialValue = cache.get("k1"); @@ -80,4 +91,67 @@ describe("findRuleFiles with scan cache", () => { secondRuleFile, ].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, + ]); + }); }); diff --git a/src/hooks/rules-injector/rule-scan-cache.ts b/src/hooks/rules-injector/rule-scan-cache.ts index fc8ff1a20..fb69e46d6 100644 --- a/src/hooks/rules-injector/rule-scan-cache.ts +++ b/src/hooks/rules-injector/rule-scan-cache.ts @@ -1,21 +1,38 @@ +import type { RuleFileCandidate } from "./types"; + +export type DirectoryScanEntry = { + path: string; + realPath: string; +}; + export type RuleScanCache = { - get: (key: string) => string[] | undefined; - set: (key: string, value: string[]) => void; + get: (key: string) => RuleFileCandidate[] | undefined; + set: (key: string, value: RuleFileCandidate[]) => void; + getDirScan: (dir: string) => DirectoryScanEntry[] | undefined; + setDirScan: (dir: string, entries: DirectoryScanEntry[]) => void; clear: () => void; }; export function createRuleScanCache(): RuleScanCache { - const cache = new Map(); + const finalResultCache = new Map(); + const directoryScanCache = new Map(); return { - get(key: string): string[] | undefined { - return cache.get(key); + get(key: string): RuleFileCandidate[] | undefined { + return finalResultCache.get(key); }, - set(key: string, value: string[]): void { - cache.set(key, value); + set(key: string, value: RuleFileCandidate[]): void { + 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 { - cache.clear(); + finalResultCache.clear(); + directoryScanCache.clear(); }, }; }