Skip to content

Commit 8ed0b39

Browse files
authored
perf(wasm): scope ensureWasmTrees re-parse to files that need it (#1038)
* perf(wasm): scope ensureWasmTrees re-parse to files that actually need it Fixes #1036 — WASM full build regressed from 7.6s (3.9.5) to 14.0s (3.9.6) on the 744-file dogfooding corpus. Root cause: PR #1016 expanded AST_TYPE_MAPS from 3 to 23 languages, growing WALK_EXTENSIONS to cover .rs/.go/.py/etc. Files like crates/codegraph-core/ build.rs (5 lines, no strings/awaits/throws) produce zero ast_nodes, so the worker returned `astNodes: undefined`. On the main thread, `fileNeedsWasmTree` saw `!Array.isArray(symbols.astNodes) && WALK_EXTENSIONS.has('.rs')` and flagged the file as needing re-parse — at which point ensureWasmTrees ignored the per-file decision and re-parsed every WASM-parseable file in the build. Fix: 1. wasm-worker-entry.ts — always serialize astNodes as an array (even empty) when ast-store ran for the file. Empty != undefined: empty means "we walked it and found nothing", which is what fileNeedsWasmTree needs to see. 2. parser.ts::ensureWasmTrees — accept an optional `needsFn` filter so the caller can scope the re-parse to files that genuinely lack data instead of pulling in every WASM-parseable file in the map. 3. ast-analysis/engine.ts — pass `fileNeedsWasmTree` as that filter. Also rolled in two small ast-store-visitor optimizations found while profiling: hoist the `newTypes` Set into a per-astTypeMap WeakMap cache (was rebuilt per file), and skip the `findParentDef` linear scan when `nodeIdMap` is empty (worker context — main thread re-resolves anyway). The codepoint check uses an `s.length`-based fast path so we only spread when length 2 or 3 needs the surrogate-pair disambiguation. Bench (744 files, dogfooding): WASM full build: 14014ms → 7847ms (-44%, restores 3.9.5 baseline) Native full build: 1693ms (unchanged) WASM incremental: 51ms (unchanged) AST node parity: 39702 nodes stored, matches native engine * perf(wasm): fold redundant len===3 codepoint check into fast path (#1038) A 3-unit UTF-16 string must contain at least 2 code points (worst case: one surrogate pair + one BMP char = 2 code points), so the spread is always redundant for len===3. Only len===2 is genuinely ambiguous — short-circuit lengths >=3 with the fast path. * test(bench): mark known 3.9.6 regressions as fixed/tracked (#1038) The benchmark regression guard was failing on three pre-existing regressions recorded in 3.9.6 BUILD-BENCHMARKS: - WASM Build ms/file (16.3 → 28.3) and No-op rebuild (21 → 134) — fixed in this PR (#1036 root cause: ensureWasmTrees re-parse scope). - Native Query time (29.4 → 47ms) — sample-noise blip on a small target set; not reproducible locally. - Haskell resolution precision/recall (100%/33% → 0%/0%) — separate resolver regression unrelated to #1036, tracked in #1039. Adding these to KNOWN_REGRESSIONS unblocks CI; entries will be removed once the corrected v3.9.7+ benchmark data lands.
1 parent 1877d40 commit 8ed0b39

5 files changed

Lines changed: 79 additions & 22 deletions

File tree

src/ast-analysis/engine.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,9 @@ async function ensureWasmTreesIfNeeded(
421421
if (needsWasmTrees) {
422422
try {
423423
const { ensureWasmTrees } = await getParserModule();
424-
await ensureWasmTrees(fileSymbols, rootDir);
424+
await ensureWasmTrees(fileSymbols, rootDir, (relPath, symbols) =>
425+
fileNeedsWasmTree(relPath, symbols, flags),
426+
);
425427
} catch (err: unknown) {
426428
debug(`ensureWasmTrees failed: ${toErrorMessage(err)}`);
427429
}

src/ast-analysis/visitors/ast-store-visitor.ts

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,21 @@ function extractChildExpressionText(node: TreeSitterNode): string | null {
131131
return truncate(node.text);
132132
}
133133

134+
/**
135+
* Count code points cheaply: skip the `[...s]` spread when `s.length` already
136+
* decides the answer. Each code point is 1 or 2 UTF-16 units, so `.length < 2`
137+
* implies `< 2` code points and `.length >= 3` already guarantees `>= 2` code
138+
* points (worst case: one surrogate pair + one BMP char = 2 code points).
139+
* Only `.length === 2` is genuinely ambiguous (could be a single surrogate
140+
* pair = 1 code point, or two BMP chars = 2 code points) and needs the spread.
141+
*/
142+
function codePointCountAtLeast2(s: string): boolean {
143+
const len = s.length;
144+
if (len < 2) return false;
145+
if (len >= 3) return true;
146+
return [...s].length >= 2;
147+
}
148+
134149
/**
135150
* Extract string content from a string-literal node, mirroring the native
136151
* engine's `build_string_node` (`helpers.rs`). Returns `null` when the
@@ -142,15 +157,27 @@ function extractStringContent(node: TreeSitterNode, cfg: AstStringConfig): strin
142157

143158
let s = raw;
144159
s = trimLeadingChars(s, '@');
145-
s = trimLeadingChars(s, cfg.stringPrefixes);
160+
if (cfg.stringPrefixes) s = trimLeadingChars(s, cfg.stringPrefixes);
146161
if (isRawString) s = trimLeadingChars(s, 'r#');
147162
s = trimLeadingChars(s, cfg.quoteChars);
148163
if (isRawString) s = trimTrailingChars(s, '#');
149164
s = trimTrailingChars(s, cfg.quoteChars);
150165

151-
// Count code points, not UTF-16 code units — matches Rust `chars().count()`.
152-
const codePointCount = [...s].length;
153-
if (codePointCount < 2) return null;
166+
return codePointCountAtLeast2(s) ? s : null;
167+
}
168+
169+
// Per-astTypeMap cache for the set of node-types that map to kind 'new'.
170+
// Computed once per unique astTypeMap reference (one per language) instead
171+
// of once per file.
172+
const _newTypesCache = new WeakMap<Record<string, string>, Set<string>>();
173+
function newTypesFor(astTypeMap: Record<string, string>): Set<string> {
174+
let s = _newTypesCache.get(astTypeMap);
175+
if (s) return s;
176+
s = new Set<string>();
177+
for (const type in astTypeMap) {
178+
if (astTypeMap[type] === 'new') s.add(type);
179+
}
180+
_newTypesCache.set(astTypeMap, s);
154181
return s;
155182
}
156183

@@ -164,11 +191,12 @@ export function createAstStoreVisitor(
164191
): Visitor {
165192
const rows: AstStoreRow[] = [];
166193
const matched = new Set<number>();
167-
const newTypes = new Set<string>(
168-
Object.entries(astTypeMap)
169-
.filter(([, kind]) => kind === 'new')
170-
.map(([type]) => type),
171-
);
194+
const newTypes = newTypesFor(astTypeMap);
195+
// When nodeIdMap is empty, parentNodeId resolution is wasted work — the
196+
// worker passes an empty map and the main thread re-resolves against its
197+
// own DB-populated map in features/ast.ts::collectFileAstRows. Skip the
198+
// findParentDef linear scan in that case.
199+
const skipParentLookup = nodeIdMap.size === 0;
172200

173201
function findParentDef(line: number): Definition | null {
174202
let best: Definition | null = null;
@@ -183,6 +211,7 @@ export function createAstStoreVisitor(
183211
}
184212

185213
function resolveParentNodeId(line: number): number | null {
214+
if (skipParentLookup) return null;
186215
const parentDef = findParentDef(line);
187216
if (!parentDef) return null;
188217
return nodeIdMap.get(`${parentDef.name}|${parentDef.kind}|${parentDef.line}`) || null;

src/domain/parser.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,16 +316,23 @@ export function getParser(parsers: Map<string, Parser | null>, filePath: string)
316316
*
317317
* Name is preserved for caller compatibility; the function now ensures
318318
* *analysis data* rather than *trees*.
319+
*
320+
* `needsFn` (optional): when provided, only files for which it returns true are
321+
* re-parsed. Without it the function falls back to "any WASM-parseable file
322+
* without _tree", which was the source of #1036 — a single file missing one
323+
* analysis triggered a full-build re-parse of every WASM-parseable file.
319324
*/
320325
export async function ensureWasmTrees(
321326
fileSymbols: Map<string, any>,
322327
rootDir: string,
328+
needsFn?: (relPath: string, symbols: any) => boolean,
323329
): Promise<void> {
324330
// Collect files that still need analysis data and are parseable by WASM.
325331
const pending: Array<{ relPath: string; absPath: string; symbols: any }> = [];
326332
for (const [relPath, symbols] of fileSymbols) {
327333
if (symbols._tree) continue; // legacy path — leave existing trees alone
328334
if (!_extToLang.has(path.extname(relPath).toLowerCase())) continue;
335+
if (needsFn && !needsFn(relPath, symbols)) continue;
329336
pending.push({ relPath, absPath: path.join(rootDir, relPath), symbols });
330337
}
331338
if (pending.length === 0) return;

src/domain/wasm-worker-entry.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -708,18 +708,18 @@ async function handleParse(msg: WorkerParseRequest): Promise<SerializedExtractor
708708
file?: string;
709709
parentNodeId?: number | null;
710710
}>;
711-
if (astRows.length > 0) {
712-
// Strip `file` and `parentNodeId` — main thread re-resolves parent IDs
713-
// against its DB in features/ast.ts::collectFileAstRows, and `file` is
714-
// known from the map key.
715-
serializedAstNodes = astRows.map((n) => ({
716-
line: n.line,
717-
kind: n.kind,
718-
name: n.name ?? '',
719-
text: n.text ?? undefined,
720-
receiver: n.receiver ?? undefined,
721-
}));
722-
}
711+
// Always set an array (even empty) — leaving astNodes undefined makes
712+
// engine.ts::fileNeedsWasmTree treat the file as un-walked and trigger
713+
// a full ensureWasmTrees re-parse of every WASM-parseable file (#1036).
714+
// Strip `file` and `parentNodeId` — main thread re-resolves both in
715+
// features/ast.ts::collectFileAstRows.
716+
serializedAstNodes = astRows.map((n) => ({
717+
line: n.line,
718+
kind: n.kind,
719+
name: n.name ?? '',
720+
text: n.text ?? undefined,
721+
receiver: n.receiver ?? undefined,
722+
}));
723723
}
724724

725725
if (complexityVisitor) storeComplexityResults(results, defs, entry.id);

tests/benchmarks/regression-guard.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,20 @@ const SKIP_VERSIONS = new Set(['3.8.0']);
7979
*
8080
* - 3.9.2:Full build — NativeDbProxy overhead causes native full build to
8181
* regress from 5206ms to 9403ms (+81%). Fix tracked in PR #906.
82+
*
83+
* - 3.9.6:Build ms/file / 3.9.6:No-op rebuild — WASM full build regressed
84+
* (#1036) when PR #1016 expanded AST_TYPE_MAPS from 3 to 23 languages,
85+
* causing zero-AST-row files to return `astNodes: undefined` and trigger
86+
* a full-corpus re-parse. Fixed by PR #1038. Benchmarks captured before
87+
* the fix landed; will reclear in v3.9.7+ data.
88+
*
89+
* - 3.9.6:Query time — native query benchmark sample-noise blip (29.4 → 47ms)
90+
* above the natural variance of the small target set. Not reproducible
91+
* locally (~30ms steady-state); will be re-validated on v3.9.7+ data.
92+
*
93+
* - 3.9.6:resolution haskell precision/recall — separate Haskell resolver
94+
* regression introduced in 3.9.6, unrelated to #1036 / PR #1038. Tracked
95+
* in #1039.
8296
*/
8397
const KNOWN_REGRESSIONS = new Set([
8498
'3.9.0:1-file rebuild',
@@ -87,6 +101,11 @@ const KNOWN_REGRESSIONS = new Set([
87101
'3.9.0:fnDeps depth 5',
88102
'3.9.1:1-file rebuild',
89103
'3.9.2:Full build',
104+
'3.9.6:Build ms/file',
105+
'3.9.6:No-op rebuild',
106+
'3.9.6:Query time',
107+
'3.9.6:resolution haskell precision',
108+
'3.9.6:resolution haskell recall',
90109
]);
91110

92111
/**

0 commit comments

Comments
 (0)