fix(resolution): yield per ref and cache hot per-ref work so the watchdog can't kill a valid index (#1122) (#1137)
The #850 liveness watchdog was killing valid `codegraph init`/`index` runs at "Resolving refs 0-2%" on large collision-heavy repos (18-25K-file Java monorepos on slower hardware). #1105's cooperative yielding assumed a 500-ref sub-chunk is always cheap, but per-ref cost is unbounded: a colliding method name (`execute`, `process`, ...) whose candidate set misses the 5,000-entry name LRU re-fetches every same-named row (unbounded SELECT + materialization, measured 8.8ms at just 4K collisions on an M4 — linear in collision count), and receiver-type inference re-split the whole source file per ref (~20% of total index CPU). A dense pocket multiplied that past the 60s window and the heartbeat starved. Three guards, no behavior change: - resolveBatchYielding checkpoints after EVERY ref (maybeYield is a ~ns time check when under budget), so a slow pocket can never run more than one ref past the yield budget. - resolveMethodOnType's ref-independent candidate filter is memoized per (language, Type::method) on the resolver context; per-ref disambiguation (import FQN #314, call-site file #1079) stays outside the memo. - Receiver inference reads lines through a per-file LRU (shared and C++ inferrers), and skips generated/minified lines >10K chars instead of regex-scanning them per ref. Measured on a 4,028-file synthetic Java bank repo (392K refs): mid-loop max event-loop stall 1528ms -> 546ms under cache thrash, total init 250.9s -> 96.8s at default config. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
e699ee9686
commit
81cb59a86e
+96
-37
@@ -6,7 +6,7 @@
|
||||
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { Node, UnresolvedReference, Edge } from '../types';
|
||||
import { Language, Node, UnresolvedReference, Edge } from '../types';
|
||||
import { QueryBuilder } from '../db/queries';
|
||||
import {
|
||||
UnresolvedRef,
|
||||
@@ -227,6 +227,8 @@ export class ReferenceResolver {
|
||||
private nameCache: LRUCache<string, Node[]>; // name → nodes cache
|
||||
private lowerNameCache: LRUCache<string, Node[]>; // lower(name) → nodes cache
|
||||
private qualifiedNameCache: LRUCache<string, Node[]>; // qualified_name → nodes cache
|
||||
private fileLinesCache: LRUCache<string, string[] | null>; // file → split lines cache
|
||||
private methodMatchCache: LRUCache<string, Node[]>; // lang\0Type::method → matching method nodes
|
||||
private knownNames: Set<string> | null = null; // all known symbol names for fast pre-filtering
|
||||
private knownFiles: Set<string> | null = null;
|
||||
private cachesWarmed = false;
|
||||
@@ -254,6 +256,10 @@ export class ReferenceResolver {
|
||||
this.nameCache = new LRUCache(limit);
|
||||
this.lowerNameCache = new LRUCache(limit);
|
||||
this.qualifiedNameCache = new LRUCache(limit);
|
||||
// Split-lines arrays are heavier than content strings; refs arrive
|
||||
// file-ordered, so a small cache still hits nearly always.
|
||||
this.fileLinesCache = new LRUCache(contentLimit);
|
||||
this.methodMatchCache = new LRUCache(limit);
|
||||
|
||||
this.context = this.createContext();
|
||||
}
|
||||
@@ -324,11 +330,30 @@ export class ReferenceResolver {
|
||||
this.nameCache.clear();
|
||||
this.lowerNameCache.clear();
|
||||
this.qualifiedNameCache.clear();
|
||||
this.fileLinesCache.clear();
|
||||
this.methodMatchCache.clear();
|
||||
this.knownNames = null;
|
||||
this.knownFiles = null;
|
||||
this.cachesWarmed = false;
|
||||
}
|
||||
|
||||
/** `readFile` through the LRU content cache (null = read failed, also cached). */
|
||||
private readFileCached(filePath: string): string | null {
|
||||
if (this.fileCache.has(filePath)) {
|
||||
return this.fileCache.get(filePath)!;
|
||||
}
|
||||
const fullPath = path.join(this.projectRoot, filePath);
|
||||
try {
|
||||
const content = fs.readFileSync(fullPath, 'utf-8');
|
||||
this.fileCache.set(filePath, content);
|
||||
return content;
|
||||
} catch (error) {
|
||||
logDebug('Failed to read file for resolution', { filePath, error: String(error) });
|
||||
this.fileCache.set(filePath, null);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create the resolution context
|
||||
*/
|
||||
@@ -349,6 +374,27 @@ export class ReferenceResolver {
|
||||
return result;
|
||||
},
|
||||
|
||||
getMethodMatches: (typeName: string, methodName: string, language: Language) => {
|
||||
const key = `${language} ${typeName}::${methodName}`;
|
||||
const cached = this.methodMatchCache.get(key);
|
||||
if (cached !== undefined) return cached;
|
||||
let candidates = this.nameCache.get(methodName);
|
||||
if (candidates === undefined) {
|
||||
candidates = this.queries.getNodesByName(methodName);
|
||||
this.nameCache.set(methodName, candidates);
|
||||
}
|
||||
const want = `${typeName}::${methodName}`;
|
||||
const matches: Node[] = [];
|
||||
for (const m of candidates) {
|
||||
if (m.kind !== 'method') continue;
|
||||
if (m.language !== language) continue;
|
||||
const qn = m.qualifiedName;
|
||||
if (qn === want || qn.endsWith(`::${want}`)) matches.push(m);
|
||||
}
|
||||
this.methodMatchCache.set(key, matches);
|
||||
return matches;
|
||||
},
|
||||
|
||||
getNodesByQualifiedName: (qualifiedName: string) => {
|
||||
const cached = this.qualifiedNameCache.get(qualifiedName);
|
||||
if (cached !== undefined) return cached;
|
||||
@@ -379,21 +425,15 @@ export class ReferenceResolver {
|
||||
}
|
||||
},
|
||||
|
||||
readFile: (filePath: string) => {
|
||||
if (this.fileCache.has(filePath)) {
|
||||
return this.fileCache.get(filePath)!;
|
||||
}
|
||||
readFile: (filePath: string) => this.readFileCached(filePath),
|
||||
|
||||
const fullPath = path.join(this.projectRoot, filePath);
|
||||
try {
|
||||
const content = fs.readFileSync(fullPath, 'utf-8');
|
||||
this.fileCache.set(filePath, content);
|
||||
return content;
|
||||
} catch (error) {
|
||||
logDebug('Failed to read file for resolution', { filePath, error: String(error) });
|
||||
this.fileCache.set(filePath, null);
|
||||
return null;
|
||||
}
|
||||
getFileLines: (filePath: string) => {
|
||||
const cached = this.fileLinesCache.get(filePath);
|
||||
if (cached !== undefined) return cached;
|
||||
const source = this.readFileCached(filePath);
|
||||
const lines = source === null ? null : source.split(/\r?\n/);
|
||||
this.fileLinesCache.set(filePath, lines);
|
||||
return lines;
|
||||
},
|
||||
|
||||
getProjectRoot: () => this.projectRoot,
|
||||
@@ -926,39 +966,58 @@ export class ReferenceResolver {
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve one batch in smaller sub-chunks, yielding to the event loop between
|
||||
* them so the #850 liveness heartbeat can fire on a slow/dense batch (#1091).
|
||||
* Behaviourally identical to a single `resolveAll(batch)`: `warmCaches()` is
|
||||
* idempotent (guarded) and `resolveOne` is independent per ref, so splitting
|
||||
* and re-merging changes only timing, never which edges get created. Falls
|
||||
* through to a plain `resolveAll` when the batch is already small.
|
||||
* Resolve one batch with a yield checkpoint between EVERY ref so the #850
|
||||
* liveness heartbeat can fire on a slow/dense batch (#1091). The checkpoint
|
||||
* granularity is per-ref — not per-N-refs — because per-ref cost is unbounded
|
||||
* in the worst case (a collision-heavy method name whose candidate set misses
|
||||
* the LRU re-fetches tens of thousands of rows): any fixed N multiplies that
|
||||
* worst case into the watchdog window, which is how v1.2.0 still got killed
|
||||
* at "Resolving refs" on large Java monorepos (#1122). `maybeYield()` is a
|
||||
* ~ns time check when under budget, so per-ref checkpoints cost nothing.
|
||||
* Behaviourally identical to `resolveAll(batch)`: `warmCaches()` is
|
||||
* idempotent (guarded) and `resolveOne` is independent per ref, so yielding
|
||||
* between refs changes only timing, never which edges get created.
|
||||
*/
|
||||
private async resolveBatchYielding(
|
||||
batch: UnresolvedReference[],
|
||||
maybeYield: MaybeYield,
|
||||
subChunkSize: number = 500
|
||||
maybeYield: MaybeYield
|
||||
): Promise<ResolutionResult> {
|
||||
if (batch.length <= subChunkSize) return this.resolveAll(batch);
|
||||
this.warmCaches();
|
||||
|
||||
const resolved: ResolvedRef[] = [];
|
||||
const unresolved: UnresolvedRef[] = [];
|
||||
const byMethod: Record<string, number> = {};
|
||||
let total = 0;
|
||||
let resolvedCount = 0;
|
||||
let unresolvedCount = 0;
|
||||
for (let i = 0; i < batch.length; i += subChunkSize) {
|
||||
const chunk = this.resolveAll(batch.slice(i, i + subChunkSize));
|
||||
for (const r of chunk.resolved) resolved.push(r);
|
||||
for (const u of chunk.unresolved) unresolved.push(u);
|
||||
total += chunk.stats.total;
|
||||
resolvedCount += chunk.stats.resolved;
|
||||
unresolvedCount += chunk.stats.unresolved;
|
||||
for (const [m, c] of Object.entries(chunk.stats.byMethod)) {
|
||||
byMethod[m] = (byMethod[m] || 0) + c;
|
||||
|
||||
for (const raw of batch) {
|
||||
const ref: UnresolvedRef = {
|
||||
fromNodeId: raw.fromNodeId,
|
||||
referenceName: raw.referenceName,
|
||||
referenceKind: raw.referenceKind,
|
||||
line: raw.line,
|
||||
column: raw.column,
|
||||
filePath: raw.filePath || this.getFilePathFromNodeId(raw.fromNodeId),
|
||||
language: raw.language || this.getLanguageFromNodeId(raw.fromNodeId),
|
||||
};
|
||||
const result = this.resolveOne(ref);
|
||||
if (result) {
|
||||
resolved.push(result);
|
||||
byMethod[result.resolvedBy] = (byMethod[result.resolvedBy] || 0) + 1;
|
||||
} else {
|
||||
unresolved.push(ref);
|
||||
}
|
||||
await maybeYield();
|
||||
}
|
||||
return { resolved, unresolved, stats: { total, resolved: resolvedCount, unresolved: unresolvedCount, byMethod } };
|
||||
|
||||
return {
|
||||
resolved,
|
||||
unresolved,
|
||||
stats: {
|
||||
total: batch.length,
|
||||
resolved: resolved.length,
|
||||
unresolved: unresolved.length,
|
||||
byMethod,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -503,15 +503,25 @@ export function resolveMethodOnType(
|
||||
// in-class (`class Foo { int bar() { ... } }`) or out-of-line in a separate
|
||||
// file (`int Foo::bar() { ... }` in foo.cpp while class Foo is in foo.hpp).
|
||||
// The previous same-file approach missed the latter — the typical C++ layout.
|
||||
const methodCandidates = context.getNodesByName(methodName);
|
||||
const want = `${typeName}::${methodName}`;
|
||||
const matches: Node[] = [];
|
||||
for (const m of methodCandidates) {
|
||||
if (m.kind !== 'method') continue;
|
||||
if (m.language !== ref.language) continue;
|
||||
const qn = m.qualifiedName;
|
||||
if (qn === want || qn.endsWith(`::${want}`)) {
|
||||
matches.push(m);
|
||||
// Prefer the context's per-(type, method) memo: the raw name lookup fetches
|
||||
// EVERY node sharing the method name — tens of thousands of rows for a
|
||||
// collision-heavy Java name like `execute` — and re-filtering that per ref
|
||||
// was a dominant term in the #1122 watchdog kill on large repos. Only the
|
||||
// ref-independent filter is memoized; per-ref disambiguation stays below.
|
||||
let matches: Node[];
|
||||
if (context.getMethodMatches) {
|
||||
matches = context.getMethodMatches(typeName, methodName, ref.language);
|
||||
} else {
|
||||
const methodCandidates = context.getNodesByName(methodName);
|
||||
const want = `${typeName}::${methodName}`;
|
||||
matches = [];
|
||||
for (const m of methodCandidates) {
|
||||
if (m.kind !== 'method') continue;
|
||||
if (m.language !== ref.language) continue;
|
||||
const qn = m.qualifiedName;
|
||||
if (qn === want || qn.endsWith(`::${want}`)) {
|
||||
matches.push(m);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (matches.length === 0) {
|
||||
@@ -610,10 +620,14 @@ function inferCppReceiverType(
|
||||
context: ResolutionContext,
|
||||
depth = 0,
|
||||
): string | null {
|
||||
const source = context.readFile(ref.filePath);
|
||||
if (!source) return null;
|
||||
// Per-file lines cache when available — this runs per `receiver->method()`
|
||||
// ref and re-splitting the file each time is the same quadratic as the
|
||||
// shared inferrer's (#1122).
|
||||
const lines = context.getFileLines
|
||||
? context.getFileLines(ref.filePath)
|
||||
: (context.readFile(ref.filePath)?.split(/\r?\n/) ?? null);
|
||||
if (!lines || lines.length === 0) return null;
|
||||
|
||||
const lines = source.split(/\r?\n/);
|
||||
const callLineIndex = Math.max(0, Math.min(lines.length - 1, ref.line - 1));
|
||||
const escapedReceiver = receiverName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
const receiverPattern = new RegExp(`\\b${escapedReceiver}\\b`);
|
||||
@@ -646,10 +660,12 @@ function inferCppReceiverType(
|
||||
|
||||
for (const headerPath of headerCandidates) {
|
||||
if (!context.fileExists(headerPath)) continue;
|
||||
const headerSource = context.readFile(headerPath);
|
||||
if (!headerSource) continue;
|
||||
const headerLines = context.getFileLines
|
||||
? context.getFileLines(headerPath)
|
||||
: (context.readFile(headerPath)?.split(/\r?\n/) ?? null);
|
||||
if (!headerLines) continue;
|
||||
|
||||
for (const line of headerSource.split(/\r?\n/)) {
|
||||
for (const line of headerLines) {
|
||||
if (!receiverPattern.test(line)) continue;
|
||||
const declaratorMatch = line.match(declaratorRegex);
|
||||
if (!declaratorMatch) continue;
|
||||
@@ -1205,10 +1221,14 @@ function inferLocalReceiverType(
|
||||
);
|
||||
if (patterns.length === 0) return null;
|
||||
|
||||
const source = context.readFile(ref.filePath);
|
||||
if (!source) return null;
|
||||
// Split through the context's per-file lines cache when available: this runs
|
||||
// for EVERY `receiver.method()` ref, and re-splitting the whole file per ref
|
||||
// was ~20% of total index CPU on Java-heavy repos (#1122).
|
||||
const lines = context.getFileLines
|
||||
? context.getFileLines(ref.filePath)
|
||||
: (context.readFile(ref.filePath)?.split(/\r?\n/) ?? null);
|
||||
if (!lines || lines.length === 0) return null;
|
||||
|
||||
const lines = source.split(/\r?\n/);
|
||||
const callIdx = Math.max(0, Math.min(lines.length - 1, ref.line - 1));
|
||||
const startIdx = Math.max(0, enclosingScopeStartLine(ref, context) - 1);
|
||||
|
||||
@@ -1216,6 +1236,10 @@ function inferLocalReceiverType(
|
||||
for (let i = callIdx; i >= startIdx; i--) {
|
||||
const line = lines[i];
|
||||
if (!line) continue;
|
||||
// A generated/minified line (one multi-KB statement) is not something a
|
||||
// human-written local declaration lives on, and regexing it per ref is
|
||||
// pure waste — skip it rather than scan it.
|
||||
if (line.length > 10_000) continue;
|
||||
for (const re of patterns) {
|
||||
const m = line.match(re);
|
||||
if (m && m[1]) {
|
||||
|
||||
@@ -75,6 +75,26 @@ export interface ResolutionContext {
|
||||
fileExists(filePath: string): boolean;
|
||||
/** Read file content */
|
||||
readFile(filePath: string): string | null;
|
||||
/**
|
||||
* `readFile(filePath)` split into lines, LRU-cached per file. Receiver-type
|
||||
* inference scans source lines for EVERY `receiver.method()` ref; splitting
|
||||
* the whole file per ref made that O(refs-in-file × file-size) — ~20% of
|
||||
* total index CPU on a Java-heavy repo and a driver of the #1122 watchdog
|
||||
* kill on large ones. Optional so external/test contexts compile without it;
|
||||
* callers fall back to splitting `readFile` themselves.
|
||||
*/
|
||||
getFileLines?(filePath: string): string[] | null;
|
||||
/**
|
||||
* The method-definition nodes matching `typeName::methodName` in `language` —
|
||||
* exactly `resolveMethodOnType`'s kind/language/qualifiedName-suffix filter,
|
||||
* LRU-cached per (language, type, method). The uncached path re-fetches every
|
||||
* node sharing the METHOD name (unbounded — tens of thousands on a collision-
|
||||
* heavy Java repo) and re-scans it per ref, the dominant term in the #1122
|
||||
* watchdog kill. Cached entries hold only the small filtered result; per-ref
|
||||
* disambiguation (import FQN, call-site file) stays in the caller so a cached
|
||||
* entry is valid from any call site. Optional for external/test contexts.
|
||||
*/
|
||||
getMethodMatches?(typeName: string, methodName: string, language: Language): Node[];
|
||||
/** Get project root */
|
||||
getProjectRoot(): string;
|
||||
/** Get all files */
|
||||
|
||||
Reference in New Issue
Block a user