Skip to content

perf(bench): exclude resolution fixtures from incremental-benchmark sweep#1131

Merged
carlos-alm merged 3 commits into
mainfrom
perf/1112-exclude-benchmark-fixtures
May 15, 2026
Merged

perf(bench): exclude resolution fixtures from incremental-benchmark sweep#1131
carlos-alm merged 3 commits into
mainfrom
perf/1112-exclude-benchmark-fixtures

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • Add an optional exclude glob array to BuildGraphOpts, merged into config.exclude in setupPipeline so both the JS pipeline and the native orchestrator (which receives ctx.config as JSON) honor it.
  • Pass tests/benchmarks/resolution/fixtures/** from scripts/incremental-benchmark.ts so future language PRs don't silently inflate fullBuildMs purely from fixture parsing (e.g. tree-sitter-verilog added ~850ms / +43% on 1959 → 2809ms in feat(native): port Verilog extractor to Rust #1107).
  • Remove the 3.10.0:Full build entry (and its docstring paragraph) from KNOWN_REGRESSIONS now that the root cause is addressed.

Smoke-tested locally against this worktree (WASM engine):

without exclude — verilog nodes: 25    (19061 total nodes)
with exclude    — verilog nodes:  0    (17758 total nodes, all fixtures filtered)

Closes #1112

Test plan

  • npm run typecheck clean
  • npm run lint clean on changed files
  • RUN_REGRESSION_GUARD=1 npm test -- tests/benchmarks/regression-guard.test.ts → 17/17 pass after removing the exemption (committed 3.10.0 entry already had a clean 1959ms baseline; only the rolling dev comparison needed the exemption while fixtures inflated the number)
  • End-to-end verified: with exclude: ['tests/benchmarks/resolution/fixtures/**'], no fixture nodes land in the DB; without it, 25 verilog nodes are present
  • CI confirms the per-PR dev → 3.10.0 Full build comparison no longer hits the threshold

…weep

The incremental benchmark walks the repo root, which pulls hand-annotated
resolution-benchmark fixtures into the corpus. A single heavy grammar
(tree-sitter-verilog #1107) added ~850ms to fullBuildMs — the cost is
real but unrelated to shared code paths the benchmark is meant to track.

Add an optional exclude glob array to BuildGraphOpts, merged into
config.exclude in setupPipeline so both the JS pipeline and the native
orchestrator (which receives ctx.config as JSON) honor it. Pass
'tests/benchmarks/resolution/fixtures/**' from the benchmark script so
future language PRs don't silently inflate timings, and remove the
3.10.0:Full build entry from KNOWN_REGRESSIONS now that the cause is
addressed.

Closes #1112
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 15, 2026

Greptile Summary

This PR fixes a benchmark inflation issue where resolution fixture files (e.g. Verilog) were being parsed during the incremental-benchmark sweep, silently inflating fullBuildMs by up to 43% whenever new language support was added. The fix adds an optional exclude field to BuildGraphOpts that gets merged into ctx.config in setupPipeline, then passed as JSON to both the JS pipeline and the native Rust orchestrator.

  • src/types.ts + pipeline.ts: New exclude?: string[] on BuildGraphOpts is merged into a freshly-spread ctx.config (not mutating the loadConfig cache), and the native orchestrator receives the updated config via JSON.stringify(ctx.config).
  • scripts/incremental-benchmark.ts: Centralizes all buildGraph call options into BUILD_OPTS with exclude: ['tests/benchmarks/resolution/fixtures/**']; applied to all six benchmark calls.
  • tests/benchmarks/regression-guard.test.ts: Removes the 3.10.0:Full build exemption now that fixture exclusion makes the measurements consistent; confirmed passing at 17/17.

Confidence Score: 5/5

Safe to merge; the change is narrowly scoped to benchmark infrastructure with no impact on normal user builds.

The loadConfig cache is never mutated — setupPipeline creates a new object so subsequent calls get the original config. The merged config is correctly forwarded to the native orchestrator via JSON.stringify(ctx.config). BUILD_OPTS is applied consistently across all six buildGraph calls. Removing the 3.10.0:Full build exemption is safe given the stored baseline was already at 1959ms.

No files require special attention.

Important Files Changed

Filename Overview
src/types.ts Adds optional exclude?: string[] to BuildGraphOpts with clear JSDoc; no issues.
src/domain/graph/builder/pipeline.ts Merges opts.exclude into a freshly-cloned ctx.config after loadConfig; cache is not mutated, and the updated config is correctly serialized to the native orchestrator via JSON.stringify(ctx.config).
scripts/incremental-benchmark.ts Centralizes benchmark options into BUILD_OPTS and excludes tests/benchmarks/resolution/fixtures/** from all buildGraph calls; applied consistently across full, noop, and one-file benchmark sweeps.
tests/benchmarks/regression-guard.test.ts Removes 3.10.0:Full build exemption and its docstring; 17/17 tests pass with the stored 3.10.0 baseline already at the pre-inflation value.

Reviews (3): Last reviewed commit: "Merge branch 'main' into perf/1112-exclu..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

Codegraph Impact Analysis

2 functions changed6 callers affected across 5 files

  • setupPipeline in src/domain/graph/builder/pipeline.ts:168 (6 transitive callers)
  • BuildGraphOpts.exclude in src/types.ts:1072 (0 transitive callers)

@carlos-alm carlos-alm merged commit 0138915 into main May 15, 2026
21 checks passed
@carlos-alm carlos-alm deleted the perf/1112-exclude-benchmark-fixtures branch May 15, 2026 17:06
@github-actions github-actions Bot locked and limited conversation to collaborators May 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: exclude verilog benchmark fixtures from incremental benchmark sweep

1 participant