Skip to content

fix(bench): stabilize peer solver timing - #73

Merged
smiggleworth merged 1 commit into
mainfrom
fix/deterministic-peer-solver-benchmark
Aug 2, 2026
Merged

fix(bench): stabilize peer solver timing#73
smiggleworth merged 1 commit into
mainfrom
fix/deterministic-peer-solver-benchmark

Conversation

@smiggleworth

@smiggleworth smiggleworth commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • measure the synchronous peer-solver benchmark as batched per-operation wall-clock time
  • retain the existing 50 ms per-solve budget, warmup count, sample count, and p95 calculation
  • document the correction in the pending 0.0.20 changelog

Reproduction

The unchanged solver completed in 12–18 ms locally, but the ten-sample p95 selected the single maximum sample. Isolated hosted-runner scheduling/GC pauses therefore produced false wall-clock failures at 50.5 ms and 51.7 ms. An attempted process-CPU measurement was also rejected by CI after parallel V8/GC work inflated one sample to 75.7 ms.

The final measurement averages five solves per sample, preserving wall-clock throughput and the 50 ms per-solve budget while preventing one unrelated pause from masquerading as a regression.

Verification

  • npm ci / npm audit (0 vulnerabilities)
  • npm run fmt -- --check
  • npm run lint
  • npm run check (224 tests with coverage)
  • npm run test:templates (full-stack, SPA, SSR, SSG, StartKit)
  • three exact final node --import tsx benchmarks/cli.mjs --gate repetitions

The final batched per-solve measurements were 12.2–13.8 ms; no budget was loosened.

Copilot AI review requested due to automatic review settings August 2, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR stabilizes the npm run bench -- --gate performance gate by measuring the synchronous, CPU-bound peer-solver benchmark using process CPU time instead of wall-clock time, preventing hosted-runner scheduling pauses from causing false regressions.

Changes:

  • Update the peer-solver benchmark to use process.cpuUsage() deltas (user + system) for sample timing.
  • Preserve the existing warmup behavior, 50 ms budget, sampling count, and p95 calculation semantics.
  • Document the benchmark measurement correction in the pending 0.0.20 changelog.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
CHANGELOG.md Documents the benchmark timing correction in the upcoming release notes.
benchmarks/cli.mjs Switches the peer-solver benchmark measurement from wall-clock time to process CPU time while keeping the same gating thresholds and statistics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@smiggleworth
smiggleworth force-pushed the fix/deterministic-peer-solver-benchmark branch from f48e6f0 to 8fde544 Compare August 2, 2026 14:37
@smiggleworth smiggleworth changed the title fix(bench): isolate peer solver CPU cost fix(bench): stabilize peer solver timing Aug 2, 2026
@smiggleworth
smiggleworth force-pushed the fix/deterministic-peer-solver-benchmark branch from 8fde544 to ca41848 Compare August 2, 2026 14:37
@smiggleworth

Copy link
Copy Markdown
Contributor Author

@copilot review the current head, please. The initial process-CPU approach was replaced with batched per-operation wall-clock timing in ca41848.

Copilot AI commented Aug 2, 2026

Copy link
Copy Markdown

@copilot review the current head, please. The initial process-CPU approach was replaced with batched per-operation wall-clock timing in ca41848.

Reviewed ca41848 — the batched wall-clock approach is correct.

benchmarks/cli.mjssolverSample()

  • solvesPerSample = 5 batches five synchronous solves per outer iteration. Reporting (elapsed) / 5 gives a per-solve average: a 50 ms scheduler pause on a ~13 ms true solve contributes only ~10 ms to that sample (~23 ms total), well inside the 50 ms gate.
  • The correctness guard (result.summary.packages !== 100) is preserved inside the inner loop, so every solve in every batch is validated.
  • Warmup semantics are unchanged: 12 outer iterations, first 2 discarded → 10 samples → p95 selects the highest, matching the original intent.
  • performance.now() is the right clock here. The function is synchronous and CPU-bound; wall-clock noise is exactly what batching suppresses.

CHANGELOG.md — entry is accurate and placed correctly under [0.0.20] Changed.

No issues found.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@smiggleworth
smiggleworth merged commit 8b20a92 into main Aug 2, 2026
12 checks passed
@smiggleworth
smiggleworth deleted the fix/deterministic-peer-solver-benchmark branch August 2, 2026 14:44
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.

3 participants