From 52dc2c4d10d94b279589ca5417ed23ae7ef55468 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Sat, 4 Apr 2026 16:32:39 -0600 Subject: [PATCH 1/2] fix(bench): attribute unified walk time to per-phase timers The analysis engine runs AST/complexity/CFG/dataflow visitors in a single DFS walk, then delegates to buildXxx functions for DB writes. The walk time was stored in a hidden _unifiedWalkMs field while the reported phase timers only captured the DB-write tail. This made WASM 1-file incremental benchmarks show ~0ms for CFG/dataflow/complexity (the real work was done in the unified walk but never attributed). Now each file's walk time is distributed proportionally among active visitors, and delegateToBuildFunctions accumulates (+=) rather than overwrites (=) so both walk + DB-write time are captured. --- src/ast-analysis/engine.ts | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/ast-analysis/engine.ts b/src/ast-analysis/engine.ts index 94878c53..9b05480f 100644 --- a/src/ast-analysis/engine.ts +++ b/src/ast-analysis/engine.ts @@ -629,7 +629,7 @@ async function delegateToBuildFunctions( } catch (err: unknown) { debug(`buildAstNodes failed: ${toErrorMessage(err)}`); } - timing.astMs = performance.now() - t0; + timing.astMs += performance.now() - t0; } if (opts.complexity !== false) { @@ -640,7 +640,7 @@ async function delegateToBuildFunctions( } catch (err: unknown) { debug(`buildComplexityMetrics failed: ${toErrorMessage(err)}`); } - timing.complexityMs = performance.now() - t0; + timing.complexityMs += performance.now() - t0; } if (opts.cfg !== false) { @@ -651,7 +651,7 @@ async function delegateToBuildFunctions( } catch (err: unknown) { debug(`buildCFGData failed: ${toErrorMessage(err)}`); } - timing.cfgMs = performance.now() - t0; + timing.cfgMs += performance.now() - t0; } if (opts.dataflow !== false) { @@ -662,7 +662,7 @@ async function delegateToBuildFunctions( } catch (err: unknown) { debug(`buildDataflowEdges failed: ${toErrorMessage(err)}`); } - timing.dataflowMs = performance.now() - t0; + timing.dataflowMs += performance.now() - t0; } } @@ -699,7 +699,10 @@ export async function runAnalyses( // WASM pre-parse for files that still need it (AST store, or native gaps) await ensureWasmTreesIfNeeded(fileSymbols, opts, rootDir); - // Unified pre-walk: run all applicable visitors in a single DFS per file + // Unified pre-walk: run all applicable visitors in a single DFS per file. + // Time each file's walk and distribute proportionally among active visitors + // so that phase timers (astMs, complexityMs, etc.) reflect real work — not + // just the DB-write tail in delegateToBuildFunctions. const t0walk = performance.now(); for (const [relPath, symbols] of fileSymbols) { @@ -714,7 +717,22 @@ export async function runAnalyses( if (visitors.length === 0) continue; + const walkStart = performance.now(); const results = walkWithVisitors(symbols._tree.rootNode, visitors, langId, walkerOpts); + const walkMs = performance.now() - walkStart; + + // Distribute walk time equally among active visitors + const activeCount = [astVisitor, complexityVisitor, cfgVisitor, dataflowVisitor].filter( + Boolean, + ).length; + if (activeCount > 0) { + const share = walkMs / activeCount; + if (astVisitor) timing.astMs += share; + if (complexityVisitor) timing.complexityMs += share; + if (cfgVisitor) timing.cfgMs += share; + if (dataflowVisitor) timing.dataflowMs += share; + } + const defs = symbols.definitions || []; if (astVisitor) { From fa5e686f24200deb4a07aa998447858bc7e83b29 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Sat, 4 Apr 2026 21:28:32 -0600 Subject: [PATCH 2/2] fix(bench): clarify timing comments per review feedback - Change "proportionally" to "equally" in walk-distribution comment - Document that _unifiedWalkMs includes setupVisitors overhead and overlaps with per-phase timers (not an additive bucket) - Add JSDoc to _unifiedWalkMs field in AnalysisTiming interface --- src/ast-analysis/engine.ts | 6 +++++- src/types.ts | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/ast-analysis/engine.ts b/src/ast-analysis/engine.ts index 9b05480f..5c5c69a3 100644 --- a/src/ast-analysis/engine.ts +++ b/src/ast-analysis/engine.ts @@ -700,7 +700,7 @@ export async function runAnalyses( await ensureWasmTreesIfNeeded(fileSymbols, opts, rootDir); // Unified pre-walk: run all applicable visitors in a single DFS per file. - // Time each file's walk and distribute proportionally among active visitors + // Time each file's walk and distribute equally among active visitors // so that phase timers (astMs, complexityMs, etc.) reflect real work — not // just the DB-write tail in delegateToBuildFunctions. const t0walk = performance.now(); @@ -745,6 +745,10 @@ export async function runAnalyses( if (dataflowVisitor) symbols.dataflow = results.dataflow as DataflowResult; } + // Total wall-clock time for the unified walk loop, including per-file + // setupVisitors overhead. Walk time is already distributed into per-phase + // timers above, so this field overlaps with (astMs + complexityMs + ...). + // It is kept as a diagnostic cross-check, not an additive bucket. timing._unifiedWalkMs = performance.now() - t0walk; // Reconcile: apply CFG-derived cyclomatic override for any definitions that have diff --git a/src/types.ts b/src/types.ts index e4e63495..79aa04ee 100644 --- a/src/types.ts +++ b/src/types.ts @@ -666,6 +666,12 @@ export interface AnalysisTiming { complexityMs: number; cfgMs: number; dataflowMs: number; + /** + * Diagnostic: total wall-clock time for the unified walk loop (includes + * setupVisitors overhead). Walk time is already distributed equally into + * the per-phase timers above, so this overlaps — it is not an additive + * bucket. Useful for cross-checking that Σ phase timers ≈ this value. + */ _unifiedWalkMs?: number; }