feat: configurable high-iteration benchmarks with env var overrides#76
feat: configurable high-iteration benchmarks with env var overrides#76Copilot wants to merge 2 commits into
Conversation
…results Agent-Logs-Url: https://github.com/dev-dami/zario/sessions/35f9f8d7-fb14-400b-9bf4-ed6d6a514f97 Co-authored-by: dev-dami <141376183+dev-dami@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds runtime configurability to the logger benchmark suite so benchmark fidelity can be increased without editing source, and provides a high-iteration preset script for repeatable runs.
Changes:
- Introduces env-var driven benchmark configs (
ZARIO_BENCH_ITERATIONS,ZARIO_BENCH_SAMPLES) with a sharedmakeConfig()helper and warmup scaling. - Adds
benchandbench:highnpm scripts and includestsxto run the TypeScript benchmark file directly. - Adds a
benchmarks/RESULTS.mdsnapshot for the high-iteration preset run.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Adds benchmark scripts and tsx dev dependency to run TS benchmarks via Node. |
| package-lock.json | Updates lockfile to include tsx and its dependency tree. |
| benchmarks/logger.bench.ts | Replaces hardcoded configs with env-var override support and warmup scaling logic. |
| benchmarks/RESULTS.md | Documents a high-iteration benchmark run output and how to run benchmarks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const BENCH_ITERATIONS = Number(process.env["ZARIO_BENCH_ITERATIONS"] ?? 0); | ||
| const BENCH_SAMPLES = Number(process.env["ZARIO_BENCH_SAMPLES"] ?? 0); | ||
|
|
||
| function makeConfig(baseIterations: number, baseWarmup: number): BenchmarkConfig { | ||
| const iterations = BENCH_ITERATIONS > 0 ? BENCH_ITERATIONS : baseIterations; | ||
| const warmupIterations = BENCH_ITERATIONS > 0 ? Math.floor(iterations * 0.2) : baseWarmup; | ||
| const samples = BENCH_SAMPLES > 0 ? BENCH_SAMPLES : 6; |
There was a problem hiding this comment.
The benchmark config reads iteration/sample counts directly via Number(...) and only checks > 0. This allows non-integers (e.g. 0.5), Infinity (e.g. 1e309/Infinity), or very large values, which can lead to misleading metrics (division by non-integer) or effectively infinite loops/hangs. Consider parsing as an integer and validating with Number.isFinite + Number.isInteger, and either clamping or throwing a clear error when invalid/out-of-range values are provided.
| const BENCH_ITERATIONS = Number(process.env["ZARIO_BENCH_ITERATIONS"] ?? 0); | |
| const BENCH_SAMPLES = Number(process.env["ZARIO_BENCH_SAMPLES"] ?? 0); | |
| function makeConfig(baseIterations: number, baseWarmup: number): BenchmarkConfig { | |
| const iterations = BENCH_ITERATIONS > 0 ? BENCH_ITERATIONS : baseIterations; | |
| const warmupIterations = BENCH_ITERATIONS > 0 ? Math.floor(iterations * 0.2) : baseWarmup; | |
| const samples = BENCH_SAMPLES > 0 ? BENCH_SAMPLES : 6; | |
| function parsePositiveIntegerEnv(name: string): number | undefined { | |
| const rawValue = process.env[name]; | |
| if (rawValue == null || rawValue.trim() === "") { | |
| return undefined; | |
| } | |
| const parsedValue = Number(rawValue); | |
| if (!Number.isFinite(parsedValue) || !Number.isInteger(parsedValue) || parsedValue <= 0) { | |
| throw new Error( | |
| `Invalid ${name}: expected a finite positive integer, received "${rawValue}".`, | |
| ); | |
| } | |
| return parsedValue; | |
| } | |
| const BENCH_ITERATIONS = parsePositiveIntegerEnv("ZARIO_BENCH_ITERATIONS"); | |
| const BENCH_SAMPLES = parsePositiveIntegerEnv("ZARIO_BENCH_SAMPLES"); | |
| function makeConfig(baseIterations: number, baseWarmup: number): BenchmarkConfig { | |
| const iterations = BENCH_ITERATIONS ?? baseIterations; | |
| const warmupIterations = BENCH_ITERATIONS != null ? Math.floor(iterations * 0.2) : baseWarmup; | |
| const samples = BENCH_SAMPLES ?? 6; |
| "bench": "node --expose-gc --import tsx/esm benchmarks/logger.bench.ts", | ||
| "bench:high": "ZARIO_BENCH_ITERATIONS=500000 ZARIO_BENCH_SAMPLES=8 node --expose-gc --import tsx/esm benchmarks/logger.bench.ts", |
There was a problem hiding this comment.
The bench:high script sets env vars using POSIX inline assignment (ZARIO_BENCH_ITERATIONS=...). This won’t work in Windows cmd.exe (and differs in PowerShell), so the benchmark scripts become shell-dependent. If cross-platform support is desired, use a cross-platform env setter (e.g., cross-env) or move the preset logic into the benchmark script itself (e.g., via CLI args).
Benchmark iteration counts were hardcoded, making it impossible to run at higher fidelity without editing source. This adds runtime control via env vars and a dedicated high-iteration npm script.
Changes
benchmarks/logger.bench.ts— replaces hardcodedBenchmarkConfigconstants withmakeConfig()that readsZARIO_BENCH_ITERATIONSandZARIO_BENCH_SAMPLESat runtime; warmup auto-scales to 20% of iterationspackage.json— addsbenchandbench:highscripts; addstsxtodevDependenciesbenchmarks/RESULTS.md— captures results from the 500k-iteration run (8 samples)Usage
High-iteration results snapshot (500k/sample, 8 samples)