refactor(benchmarks): replace hand-rolled harness with tinybench + autocannon#397
refactor(benchmarks): replace hand-rolled harness with tinybench + autocannon#397avoidwork wants to merge 1 commit into
Conversation
…tocannon - Replace custom perf_hooks orchestrator with benchmarks/run.js - Add 4 micro-benchmark suites (utility, middleware, routing, serving) - Add 2 macro-benchmark suites (comparison with Express/Fastify, http endpoints) - Use npx to run autocannon for HTTP performance measurements - Temporarily install Express/Fastify for comparison benchmarks - Add tinybench and autocannon as devDependencies
🤖 Augment PR SummarySummary: Refactors Woodland’s benchmark tooling by replacing the custom perf_hooks harness with a new benchmarks runner plus dedicated micro/macro suites. Changes:
Technical Notes: Macro comparison uses autocannon runs against multiple servers (raw/Woodland/Express/Fastify) and the runner temporarily installs Express/Fastify for the comparison suite. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| const SUITES = { | ||
| micro: { dir: MICRO_DIR, type: "tinybench" }, | ||
| macro: { dir: MACRO_DIR, type: "autocannon" }, |
There was a problem hiding this comment.
benchmarks/run.js:L14: The macro suite has no file, so runAutocannonSuite() will attempt to execute benchmarks/macro/macro.js, which doesn’t appear to exist in this PR. This makes the default “run all suites” path likely to fail immediately.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| async function runTinybenchSuite(suiteName, dir) { | ||
| console.log(`\n=== ${suiteName.toUpperCase()} BENCHMARKS ===`); | ||
| try { | ||
| execFileSync("node", ["--test", `${dir}/*.bench.js`], { |
There was a problem hiding this comment.
benchmarks/run.js:L37: execFileSync("node", ["--test", ${dir}/*.bench.js]) passes a literal * pattern without shell expansion; if Node’s --test CLI doesn’t interpret globs itself, the micro suite won’t discover any files (or will error on a non-existent path). Consider double-checking this behaves as intended across the supported Node versions.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| }); | ||
| console.log(`\n✓ ${suiteName} benchmark passed`); | ||
| } catch (error) { | ||
| console.error(`\n✗ ${suiteName} benchmark failed:`, error.message); |
There was a problem hiding this comment.
benchmarks/run.js:L59: runAutocannonSuite() catches failures and only logs them, so npm run benchmark can exit successfully even when one or more benchmark suites failed to run. That can mask broken benchmark scripts in CI or local usage.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| testServer = createServer(app.route); | ||
|
|
||
| await new Promise((resolve) => { |
There was a problem hiding this comment.
benchmarks/macro/http.js:L81: This script starts a server and then waits for SIGINT, so when benchmarks/run.js executes the http suite it won’t terminate on its own and will hang the overall benchmark run. As written it behaves like a manual benchmark target, not an automated suite.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| console.log(` npx --yes autocannon ${testServerUrl}`); | ||
| console.log(`\nOr benchmark specific endpoints:`); | ||
| console.log(` npx --yes autocannon ${testServerUrl}/users/123`); | ||
| console.log(` npx --yes autocannon ${testServerUrl}/health -m POST`); |
There was a problem hiding this comment.
benchmarks/macro/http.js:L96: The printed example uses -m POST against /health, but /health is registered as a GET route above, so the example will benchmark error-path behavior rather than the intended handler.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| function bench(name, fn) { | ||
| if (typeof fn === "function" && fn.constructor.name === "AsyncFunction") { | ||
| performance.now(); | ||
| for (let i = 0; i < WARM; i++) fn(); |
There was a problem hiding this comment.
benchmarks/micro/serving.bench.js:L95: In the async branch, warmup/iterations call fn() without awaiting it, so timings measure promise creation rather than app.serve() work and may leave many in-flight operations. This can also make the benchmark results unstable or cause resource buildup during the loop.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| for (const { name, url } of benchmarks) { | ||
| console.log(`Running: ${name} (${url})`); | ||
| try { | ||
| const output = execSync(`npx --yes autocannon ${url} --duration 3 --connections 10`, { |
There was a problem hiding this comment.
benchmarks/macro/comparison.js:L63: This uses execSync() to run npx autocannon ..., which is an “exec-style” pattern and may conflict with the project’s forbidden patterns (Rule: AGENTS.md). It also means benchmark runs will execute external commands from within repo code.
Severity: medium
Other Locations
benchmarks/run.js:3benchmarks/run.js:99
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
1 participant
Description
Replace the hand-rolled
node:perf_hooksbenchmark harness with standardized tools:tinybenchfor micro-benchmarks andautocannonfor macro-benchmarks. The previous custom orchestrator (~254 lines) is replaced withbenchmarks/run.jswhich provides consistent CLI flags (--suite,--help) and cleaner output.Type of Change
Testing
npm run benchmark -- --suite microruns all micro benchmarksnpm run benchmark -- --suite comparisoninstalls Express/Fastify temporarily and benchmark all 4 frameworksCoverage
npm test)Checklist
npm run lintpassesFiles Changed
Added
benchmarks/run.js- orchestrator supporting--suiteand--helpbenchmarks/micro/{utility,middleware,routing,serving}.bench.js- 49 micro-benchmarksbenchmarks/macro/{comparison,http}.js- 2 macro-benchmark suitesopenspec/changes/replace-benchmarks-tinybench-autocannon/- change proposal/design/tasksDeleted
benchmark.js- old hand-rolled orchestratorbenchmarks/{comparison,http,middleware,routing,serving,utility}.js- old benchmark suitesModified
package.json- updated benchmark script, added tinybench + autocannon depsNote on Fastify/Express
Express and Fastify are NOT installed as project dependencies. The comparison suite installs them temporarily via
npm install --no-savebefore benchmarking, removing them afterward. This keeps the project lean and reflects the original benchmark approach.Performance Benchmark (micro)