From 625b4fe921dffcfe10c7643968851e8125c75b5f Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 7 Jul 2026 14:17:16 -0500 Subject: [PATCH] fix(resolution): stop per-call config-key scan that made large Java/Kotlin (Spring) indexes take ~1h (#1180) (#1210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a large Java/Kotlin Spring monorepo, reference resolution — not extraction — dominated a full index (Spring Boot's ~9,650-file tree: extraction 62s, resolution ~26min). The Spring framework resolver ran an uncached getNodesByKind('constant') full scan + canonicalConfigKey() filter for EVERY dotted `calls` ref (every list.add(), builder.build(), receiver.method()), because the config-key branch gated only on "dotted java/kotlin", not on ref kind. With ~1,100 constant nodes × ~200k dotted calls that is ~200M wasted row-fetches/allocations. Fixes, one theme — config-key constants bind config `references`, never `calls`: - frameworks/java.ts: gate the Spring config-key branch on referenceKind === 'references' (what @Value/@ConfigurationProperties emit) so the `calls` flood skips the scan. - name-matcher.ts: a `calls` ref no longer resolves to a yaml/properties config node via matchByQualifiedName (service.process() vs the yaml key service.process) — a wrong edge that also hid the real callee; it now falls through to method resolution. - resolution/index.ts: cache getNodesByKind in the resolver context (same lifetime as nameCache). Fixes the same uncached-per-ref scan in the Drupal hook_ resolver and is defense-in-depth for the Spring :prefix branch. Measured (Spring Boot): resolution 269s→16.5s on a 4.3k-file module (16×) and ~26min→44.7s on the full 9.6k-file tree (~35×); graph byte-identical, full suite passes. Adds a regression test (same key, two ref kinds, opposite outcomes). Co-authored-by: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + __tests__/frameworks-integration.test.ts | 58 ++++++++++++++++++++++++ src/resolution/frameworks/java.ts | 18 ++++++-- src/resolution/index.ts | 16 ++++++- src/resolution/name-matcher.ts | 19 ++++++-- 5 files changed, 103 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9a4c80..d36d2dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- Indexing a large Java or Kotlin **Spring** monorepo is dramatically faster. The reference-resolution phase — the bulk of a first-time `codegraph index` — could run for the better part of an hour on a big multi-module project and now finishes in a few minutes, producing the same graph. The cause: every `receiver.method()` call in the codebase was repeatedly scanning the project's entire set of configuration keys looking for a match — work that only ever applies to Spring `@Value` / `@ConfigurationProperties` bindings, never to ordinary method calls. As part of the same fix, a method call whose name coincides with a configuration key (say a `service.process()` call alongside a `service.process` entry in `application.yml`) is no longer mislinked to that config key — it now resolves to the actual method. Thanks @bayernjava for the report. (#1180) - `codegraph init` at a parent repository whose `.gitignore` excludes its child repositories no longer silently indexes nothing and reports success. The "super-repo of gitignored child repos" layout — a top-level Git repo that `.gitignore`s each `service-*/` or `packages/*` child so `git status` stays quiet — used to index only the parent's few top-level files and print "Done" with 0 nodes, even though running `codegraph init` inside any child worked fine (CodeGraph respects `.gitignore` by default, so the excluded children were skipped). Now, when an index comes up empty, CodeGraph detects the gitignored child repositories that were skipped, names them, and — in an interactive terminal — offers to index them (writing an `includeIgnored` entry to `codegraph.json` and re-indexing on the spot); non-interactive runs print the exact `codegraph.json` snippet to add. Projects that legitimately keep gitignored reference clones out of a working index are never nagged: the offer only appears when the index would otherwise be empty. Thanks @small-thanks for the report. (#1156) - The MyBatis mapper reader is sturdier on real-world XML. Single-quoted attribute values (`id='getById'`, legal XML and common in older mappers) are no longer skipped, so those statements make it into the graph. Statements and ``s that were commented out with `` no longer produce phantom symbols. And two vendor-split statements — the same `id` with `databaseId="oracle"` / `databaseId="mysql"` — written on a single line no longer silently drop one of the pair. Thanks @ESPINS for the report, the reproductions, and the fixes. (#1182) - `codegraph init` and `codegraph index` no longer get killed by the safety watchdog at the "Resolving refs" step on large method-name-heavy codebases (big Java/enterprise monorepos were the main victims, especially on slower machines). Resolution used to come up for air only every 500 references, so a dense stretch of expensive ones could starve the watchdog long enough for it to assume the process was stuck and kill a perfectly healthy index. Resolution now checkpoints after every reference, and two of the expensive steps got much cheaper: repeated method lookups on the same type are now cached, and source files are no longer re-split line-by-line for every call being resolved — indexing such repos is several times faster as a result. Generated or minified single-line files are also skipped during receiver-type inference instead of being scanned per call. Thanks @UchihaYong and @wangmeng-95 for the reports. (#1122) diff --git a/__tests__/frameworks-integration.test.ts b/__tests__/frameworks-integration.test.ts index f2abb50..3df4f2d 100644 --- a/__tests__/frameworks-integration.test.ts +++ b/__tests__/frameworks-integration.test.ts @@ -624,6 +624,64 @@ describe('Java end-to-end — field-injected bean trace (issue #389)', () => { cg.close(); }); + it('binds a config key only for `references` refs, never a same-named method call (#1180)', async () => { + // `service.process` is BOTH a yaml key and a `service.process()` method call. + // canonicalConfigKey collapses them to the same token, so before #1180 the + // method call (kind `calls`) fell into the Spring config-key branch and + // mis-resolved to the YAML constant at 0.9 confidence — a wrong edge, and the + // uncached constant scan that made large Java/Kotlin indexes take ~1h. The + // branch is now gated to `references` (only @Value/@ConfigurationProperties). + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-spring-kindgate-')); + const javaDir = path.join(tmpDir, 'src/main/java/com/example'); + const resDir = path.join(tmpDir, 'src/main/resources'); + fs.mkdirSync(javaDir, { recursive: true }); + fs.mkdirSync(resDir, { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, 'pom.xml'), + 'org.springframework.bootspring-boot-starter\n' + ); + fs.writeFileSync(path.join(resDir, 'application.yml'), 'service:\n process: "enabled"\n'); + fs.writeFileSync( + path.join(javaDir, 'Worker.java'), + 'package com.example;\n' + + 'import org.springframework.beans.factory.annotation.Value;\n' + + 'class Processor { void process() {} }\n' + + 'public class Worker {\n' + + ' private Processor service;\n' + + ' @Value("${service.process}") private String sp;\n' + + ' void run() { service.process(); }\n' + + '}\n' + ); + + const cg = CodeGraph.initSync(tmpDir); + await cg.indexAll(); + + const yamlKey = cg + .getNodesByKind('constant') + .find((n) => n.language === 'yaml' && n.qualifiedName === 'service.process'); + expect(yamlKey, 'yaml key service.process should be indexed').toBeDefined(); + + // `references` ref (@Value) DOES bind to the config key. + const valueBind = cg + .getNodesByKind('constant') + .find((n) => n.id.startsWith('spring-value:') && n.name === 'service.process'); + expect(valueBind).toBeDefined(); + expect( + cg.getOutgoingEdges(valueBind!.id).some((e) => e.target === yamlKey!.id), + '@Value should still bind to the yaml key', + ).toBe(true); + + // `calls` ref (service.process()) must NOT bind to the config key. + const run = cg.getNodesByKind('method').find((n) => n.name === 'run'); + expect(run).toBeDefined(); + expect( + cg.getOutgoingEdges(run!.id).some((e) => e.target === yamlKey!.id), + 'a method call must never resolve to a config-key constant', + ).toBe(false); + + cg.close(); + }); + it('emits only a file node for non-MyBatis XML (pom.xml, beans.xml, log4j.xml)', async () => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-xml-non-mybatis-')); fs.writeFileSync( diff --git a/src/resolution/frameworks/java.ts b/src/resolution/frameworks/java.ts index 157f4e4..916bfbc 100644 --- a/src/resolution/frameworks/java.ts +++ b/src/resolution/frameworks/java.ts @@ -84,14 +84,22 @@ export const springResolver: FrameworkResolver = { // (the bindings come from `@Value`). Skip non-Spring refs that happen to // have dots in them. } + // Spring config-key resolution: `@Value("${a.b.c}")` and + // `@ConfigurationProperties`. Gate on the `references` kind — those bindings + // are emitted as `references` by extractSpringValueBindings, whereas the far + // more numerous method-call refs (`list.add()`, `builder.build()`, every + // `receiver.method()`) are `calls`. A config key is NEVER a `calls` ref, so + // this loses no resolution. Without the gate, every dotted `calls` ref fell + // into the uncached getNodesByKind('constant') scan below — an + // O(dotted-calls × constant-nodes) cost that dominated resolution and made a + // full index take ~1h on large Java/Kotlin (Spring) monorepos (#1180). The + // old `split('.').length >= 2` heuristic couldn't separate keys from calls; + // the kind check does it exactly. if ( + ref.referenceKind === 'references' && (ref.language === 'java' || ref.language === 'kotlin') && ref.referenceName.includes('.') && - !ref.referenceName.includes('::') && - // Exclude method-call style (single-dot, both sides lower-camel). Spring - // config keys are typically 3+ segments and contain kebabs/dashes; we - // can't filter perfectly but skipping single-dot keeps the lookup tight. - ref.referenceName.split('.').length >= 2 + !ref.referenceName.includes('::') ) { const canonRef = canonicalConfigKey(ref.referenceName); const candidates = context.getNodesByKind('constant').filter( diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 1edae51..dbf1e00 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -229,6 +229,15 @@ export class ReferenceResolver { private qualifiedNameCache: LRUCache; // qualified_name → nodes cache private fileLinesCache: LRUCache; // file → split lines cache private methodMatchCache: LRUCache; // lang\0Type::method → matching method nodes + // Node kinds are a small fixed set (~24), so this is a plain Map, not an LRU. + // getNodesByKind returns the FULL node list for a kind; it was previously + // uncached — a per-ref `SELECT * FROM nodes WHERE kind=?` + row-mapping. Called + // for every dotted call ref by the Spring resolver (constants) and every + // `hook_` ref by the Drupal resolver (functions), that scan dominated + // resolution on large repos (#1180). The node set is stable within a + // resolution pass (same lifetime assumption as nameCache); clearCaches() resets + // it between passes. Callers must treat the returned array as read-only. + private nodesByKindCache = new Map(); private knownNames: Set | null = null; // all known symbol names for fast pre-filtering private knownFiles: Set | null = null; private cachesWarmed = false; @@ -332,6 +341,7 @@ export class ReferenceResolver { this.qualifiedNameCache.clear(); this.fileLinesCache.clear(); this.methodMatchCache.clear(); + this.nodesByKindCache.clear(); this.knownNames = null; this.knownFiles = null; this.cachesWarmed = false; @@ -404,7 +414,11 @@ export class ReferenceResolver { }, getNodesByKind: (kind: Node['kind']) => { - return this.queries.getNodesByKind(kind); + const cached = this.nodesByKindCache.get(kind); + if (cached !== undefined) return cached; + const result = this.queries.getNodesByKind(kind); + this.nodesByKindCache.set(kind, result); + return result; }, fileExists: (filePath: string) => { diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 5ebe62f..141d213 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -412,7 +412,21 @@ export function matchByQualifiedName( return null; } - const candidates = context.getNodesByQualifiedName(ref.referenceName); + // A method call `receiver.method()` can share an exact qualified name with a + // config-file key: `service.process()` (a `calls` ref named `service.process`) + // vs the yaml key `service.process`. Config keys are bound to their code refs + // upstream by the framework resolvers (`@Value` → `references`); a `calls` ref + // must never resolve to a yaml/properties config node — that's a wrong edge + // AND it hides the real callee. Drop those from both the exact and the partial + // candidate sets so resolution falls through to method resolution below (#1180). + const keepForRef = (nodes: Node[]): Node[] => + ref.referenceKind === 'calls' + ? nodes.filter( + (n) => !(n.kind === 'constant' && (n.language === 'yaml' || n.language === 'properties')), + ) + : nodes; + + const candidates = keepForRef(context.getNodesByQualifiedName(ref.referenceName)); if (candidates.length === 1) { return { @@ -444,8 +458,7 @@ export function matchByQualifiedName( const parts = ref.referenceName.split(/[:.]/); const lastName = parts[parts.length - 1]; if (lastName) { - const partialCandidates = context - .getNodesByName(lastName) + const partialCandidates = keepForRef(context.getNodesByName(lastName)) .filter((candidate) => candidate.qualifiedName.endsWith(ref.referenceName)); const chosen = preferCallSiteFile(partialCandidates, ref.filePath)[0]; if (chosen) {