perf: bump benchmark iterations from 5 to 30 and wire up workflow input#1870
perf: bump benchmark iterations from 5 to 30 and wire up workflow input#1870
Conversation
With only 5 iterations, P95/P99/max are all the same value — no statistical significance in percentile calculations. Bumping to 30 gives meaningful tail-latency data (P95 = 2nd-worst of 30 runs). Changes: - Read iteration count from AWF_BENCHMARK_ITERATIONS env var (default 30) - Pass workflow_dispatch iterations input to the script via env var - Increase workflow timeout from 30 to 90 minutes for more iterations Closes #1864 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
Updates the performance benchmarking defaults and GitHub Actions wiring so scheduled/manual runs collect enough samples for meaningful percentile reporting, and so the workflow’s iterations input actually affects the benchmark script.
Changes:
- Change benchmark script default iterations from 5 to 30, read from
AWF_BENCHMARK_ITERATIONS. - Update workflow dispatch input default to 30 and pass it to the script via env var.
- Increase workflow timeout to accommodate the higher iteration count.
Show a summary per file
| File | Description |
|---|---|
| scripts/ci/benchmark-performance.ts | Reads benchmark iteration count from AWF_BENCHMARK_ITERATIONS (default 30). |
| .github/workflows/performance-monitor.yml | Sets workflow default iterations to 30, exports env var for the script, and increases timeout. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
| // ── Configuration ────────────────────────────────────────────────── | ||
|
|
||
| const ITERATIONS = 5; | ||
| const ITERATIONS = parseInt(process.env.AWF_BENCHMARK_ITERATIONS || '30', 10); |
There was a problem hiding this comment.
ITERATIONS is derived directly from AWF_BENCHMARK_ITERATIONS via parseInt(...) with no validation. If the env var is non-numeric (NaN) or <= 0, several benchmarks will produce an empty values array and then stats(values) will throw (stats() requires at least one value). Consider validating/clamping to a positive integer and falling back to the default (e.g., 30) with a clear stderr warning when the input is invalid.
| const ITERATIONS = parseInt(process.env.AWF_BENCHMARK_ITERATIONS || '30', 10); | |
| const DEFAULT_ITERATIONS = 30; | |
| function getIterations(): number { | |
| const raw = process.env.AWF_BENCHMARK_ITERATIONS; | |
| if (raw === undefined) { | |
| return DEFAULT_ITERATIONS; | |
| } | |
| const parsed = Number.parseInt(raw, 10); | |
| if (!Number.isInteger(parsed) || parsed <= 0) { | |
| console.error( | |
| `Invalid AWF_BENCHMARK_ITERATIONS=${JSON.stringify(raw)}; using default ${DEFAULT_ITERATIONS}.` | |
| ); | |
| return DEFAULT_ITERATIONS; | |
| } | |
| return parsed; | |
| } | |
| const ITERATIONS = getIterations(); |
|
Smoke Test Results ✅ GitHub MCP — PRs: "feat: enable cli-proxy for smoke-services and firewall-issue-dispatcher", "chore: upgrade gh-aw to v0.67.4 and disable secret-digger schedules" Overall: PASS
|
This comment has been minimized.
This comment has been minimized.
|
🔮 The ancient spirits stir in the circuit of this run.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
If AWF_BENCHMARK_ITERATIONS is non-numeric (NaN) or <= 0, the benchmark script would produce an empty values array causing stats() to throw. Validate and clamp to a positive integer, falling back to the default (30) with a clear stderr warning when the input is invalid. Follow-up to #1870 addressing Copilot review feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If AWF_BENCHMARK_ITERATIONS is non-numeric (NaN) or <= 0, the benchmark script would produce an empty values array causing stats() to throw. Validate and clamp to a positive integer, falling back to the default (30) with a clear stderr warning when the input is invalid. Follow-up to #1870 addressing Copilot review feedback. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
iterationsworkflow dispatch input viaAWF_BENCHMARK_ITERATIONSenv var — previously declared but ignoredTest plan
npm run buildpassesnpx jest benchmark-utilspasses (26 tests)iterations: 5to verify env var is readCloses #1864
🤖 Generated with Claude Code