Skip to content

chore(bench): one command compares a one-file sync before and after a change - #530

Merged
bompus merged 2 commits into
fork/consolidatedfrom
bench/sync-ab
Oct 19, 2026
Merged

bompus merged 2 commits into
fork/consolidatedfrom
bench/sync-ab

Conversation

@bompus

@bompus bompus commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

One command now measures whether a change moves a one-file sync's peak memory, wall time or CPU time, and stops early when the comparison would mislead. On n8n it reproduces #529's result (peak 961 to 915.5 MiB, 4.7% lower) in 5 minutes. The run that measured #529 took a hand-written driver and a manual index fix.

scripts/benchmarks/runtime/sync-ab.mjs takes a corpus, the file to edit, a saved index and two builds or runtimes. Each run restores the saved index and checks it byte for byte, then runs sync under /usr/bin/time -v and hashes the edges and near-duplicate tables. The two builds' order alternates within each session. Same-build control pairs give the noise floor. A metric qualifies when every session's median ratio is outside both the 3% band and that floor, in the same direction. The output is a Markdown table for a PR body, a chart spec and one JSON line per run.

It stops early in three cases:

  • The builds did different work. After the first pair it compares the graph hashes and the counts printed in the sync's timing lines, and checks whether either build rebuilt the synthesis skip list. This is the trap from perf(resolution): a one-file sync uses less memory looking up imports #529: the saved index keys its skip list to a fingerprint of the resolution code, so the changed build rescanned every file. Its first screen read +22% memory and +75% time. With --no-relabel the tool now stops after one pair and names the rebuilt list.
  • The answer is already clear. It stops after one session when every pair is outside the band and twice the control spread.
  • The noise floor is already known. --noise reuses the floor stored for the same host, kernel, runtime version, corpus revision and edit, and runs no controls; an upgrade changes the key.

To avoid the rescan, each build runs once untimed. When that run rebuilt the skip list, the build gets an index copy labelled with its own fingerprint, and the run prints a note. --proxies runs each build once under perf stat -e instructions:u, and on Node once with a preload that totals the bytes allocated, as a four-run screen.

Measured on n8n 7cc0450a, #528 (5ccc833) as the before build and #529 as the after build, Node 24.22.0:

Command Runs Wall-clock Result
Default, two sessions 22 5 min Peak 961 to 915.5 MiB, 4.7% lower, qualified (controls 2.6%); wall and CPU no clear change
--noise with the floor stored by the run above 14 3 min 30 s 953 to 910.5 MiB, 4.5% lower, qualified
--proxies 6 1 min 30 s Instructions 0.952, bytes allocated 0.930 (1,692 to 1,574 MiB)
--no-relabel 4 1 min 30 s Stopped after the first pair: the after build rebuilt the skip list

#529 measured the same change by hand at 960 to 915 MiB (40 runs over Node and Bun).

What the proxies predict. Run on four landed changes (#493, #494, #528, #529, Node and Bun), the instruction ratio matched large time changes: #493's 0.676 and 0.658 against a sync 29% and 35% faster. A drop of about 5% in instructions did not reliably show in wall time, whose same-build variation on n8n is up to 7%. Bytes allocated did not predict peak memory: #528 allocated 5% less on Node with no peak change, and its 12% Bun drop had no proxy. The README records the table and says to use the proxies to tell whether a change does less work, and the full comparison for memory claims. Repeating two of the cases moved each ratio by 0.003 at most.

Also changed: scripts/benchmarks/runtime/README.md documents the command, and the codegraph-build-validation skill points to it and records its run times.

Checks: node --check on the script, npm run check:agent-docs, and the four runs above at e2fecbe. At 3ef4c8b (review fixes): --cgdir . and an index inside the restore directory are refused before anything is deleted; the planted mismatch stops both the comparison and --proxies with exit 3; normal proxy runs on #529 and #493 pass. Not covered: Bun arms of the proxy mode (allocation is Node-only; instructions work on both), and corpora other than n8n.

README: no user-visible feature, command or number changes; the "About this fork" section is unaffected.

@bompus

bompus commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@bompus

bompus commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

cubic has no review quota until 2026-11-11, so it skips this pull request; Greptile and CodeRabbit review it.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 20 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository YAML (base), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e966a318-f229-411a-ab57-cbf6aecf3ea9
📥 Commits

Reviewing files that changed from the base of the PR and between 5ccc833 and 3ef4c8b.

📒 Files selected for processing: 4
  • .claude/skills/codegraph-build-validation/SKILL.md
  • scripts/benchmarks/runtime/README.md
  • scripts/benchmarks/runtime/alloc-total.cjs
  • scripts/benchmarks/runtime/sync-ab.mjs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 31 minutes.

@bompus

bompus commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

CodeRabbit skipped this pull request: "Review limit reached" (free OSS review quota used up). Per the no-quota rule it proceeds on Greptile; cubic also has no quota until 2026-11-11.

@greptile-apps

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 2/5

[Medium impact] Adds benchmark tooling for one-file sync performance.

Fix the unsafe directory deletion and workload checks before merging.

Findings

  1. P1 Directory options can delete sources ▶
  2. P1 Timing logs reject valid comparisons ▶
  3. P1 Proxy mode skips workload checks ▶
  4. P2 Noise reuse misses changed conditions ▶
Fix with agent prompt
### Issue 1
scripts/benchmarks/runtime/sync-ab.mjs:119-120
`restore()` deletes `cgIndex` before checking whether it is safe to replace. Passing `--cgdir .` deletes the corpus itself. Pointing `--index` at the restore directory deletes the saved index before copying it. Validate `--cgdir` as a plain directory name and reject overlapping source and destination paths before deleting anything.

### Issue 2
scripts/benchmarks/runtime/sync-ab.mjs:110
`workCounters()` treats timing-only lines as work counters, but synthesis passes print those lines only when they exceed 250 ms with `CODEGRAPH_SYNTH_TIMINGS=1`. A pass taking 240 ms before and 260 ms after therefore looks like different work. The script stops with status 3 even when both builds did the same work. Compare only lines containing actual work counts, or exclude the threshold-gated pass timings.

### Issue 3
scripts/benchmarks/runtime/sync-ab.mjs:207-215
`--proxies` exits before the graph and work checks in the session loop. For example, `--proxies --no-relabel` reports instruction and allocation ratios even when one build rebuilds the skip list. The regular comparison rejects that mismatch. Keep the run records and apply the same graph, counter, and rescan checks before publishing these ratios.

### Issue 4
scripts/benchmarks/runtime/sync-ab.mjs:191
`noiseKey()` omits `BENCH_WRAP` and the saved index's contents. Reusing `--noise` after changing a memory cap or replacing the index skips all controls and judges the new run against variation measured under different conditions. Include the wrapper and a saved-index fingerprint in the key so those changes require fresh controls.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds a one-command benchmark for comparing one-file sync memory and time.

  • One command compares one-file syncs across two builds.
  • Quick proxy runs compare instructions and Node allocations.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Restore saved index and warm both builds] --> B{Proxy mode?}
  B -->|Yes| C[Measure instructions and allocations]
  C --> D[Publish ratios without workload checks]
  B -->|No| E[Measure first pair]
  E --> F{Graph or work differs?}
  F -->|Yes| G[Stop comparison]
  F -->|No| H[Run pairs and controls]
  H --> I[Write verdicts and charts]
Loading

Reviews (1) · Last reviewed commit: "chore(bench): one command compares a one..." · Reviewed by Greptile

Comment thread scripts/benchmarks/runtime/sync-ab.mjs
Comment thread scripts/benchmarks/runtime/sync-ab.mjs Outdated
Comment thread scripts/benchmarks/runtime/sync-ab.mjs
Comment thread scripts/benchmarks/runtime/sync-ab.mjs Outdated
…heck proxies and key the noise floor on the wrapper and index
@bompus

bompus commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 26 seconds.

@bompus
bompus merged commit 2a4af7c into fork/consolidated Oct 19, 2026
2 checks passed
@bompus
bompus deleted the bench/sync-ab branch October 19, 2026 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant