bench: add realistic workload benchmarks#179
Open
productdevbook wants to merge 2 commits intoh3js:mainfrom
Open
bench: add realistic workload benchmarks#179productdevbook wants to merge 2 commits intoh3js:mainfrom
productdevbook wants to merge 2 commits intoh3js:mainfrom
Conversation
Add comprehensive benchmarks with real-world API route sets (e-commerce 108 routes, GitHub-like 119 routes), route scaling tests (50/200/500), findAllRoutes, removeRoute, compilation time, and memory usage measurements. Document results in README. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request adds comprehensive benchmarking infrastructure to the project, including realistic API workload datasets, a scalable benchmark suite supporting Node/Bun/Deno execution, documented benchmark results in the README, and helper utilities for measuring router performance across interpreter vs compiled modes, scaling characteristics, and build timings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/bench/realistic.ts (2)
256-279: Memory measurements report incremental deltas rather than isolated per-router memory.The current approach measures heap growth between successive router builds. Since
memRouter50remains reachable whenmemRouter200is built,after200.heapUsed - after50.heapUsedreports the memory added bymemRouter200alone, butmemRouter50is still in memory. This seems intentional for incremental reporting, but the output labels (50 routes,200 routes,500 routes) may mislead users into thinking these are the standalone memory costs of each router size.If isolated per-router measurements are desired, consider building and measuring each router independently:
♻️ Alternative: isolated per-router memory measurement
if (typeof globalThis.gc === "function") { console.log("\n--- Memory Usage ---"); - globalThis.gc(); - const baseline = process.memoryUsage(); - - const memRouter50 = buildRouter(scale50.routes); - globalThis.gc(); - const after50 = process.memoryUsage(); - - const memRouter200 = buildRouter(scale200.routes); - globalThis.gc(); - const after200 = process.memoryUsage(); - - const memRouter500 = buildRouter(scale500.routes); - globalThis.gc(); - const after500 = process.memoryUsage(); - - // Prevent GC from collecting routers before measurement - do_not_optimize(memRouter50); - do_not_optimize(memRouter200); - do_not_optimize(memRouter500); - - const fmt = (bytes: number) => `${(bytes / 1024).toFixed(1)} KB`; - console.log(` 50 routes: ~${fmt(after50.heapUsed - baseline.heapUsed)}`); - console.log(` 200 routes: ~${fmt(after200.heapUsed - after50.heapUsed)}`); - console.log(` 500 routes: ~${fmt(after500.heapUsed - after200.heapUsed)}`); - console.log(` Total heap: ${fmt(after500.heapUsed)}`); + const fmt = (bytes: number) => `${(bytes / 1024).toFixed(1)} KB`; + + const measure = (routes: Route[]) => { + globalThis.gc!(); + const before = process.memoryUsage().heapUsed; + const router = buildRouter(routes); + globalThis.gc!(); + const after = process.memoryUsage().heapUsed; + do_not_optimize(router); + return after - before; + }; + + console.log(` 50 routes: ~${fmt(measure(scale50.routes))}`); + console.log(` 200 routes: ~${fmt(measure(scale200.routes))}`); + console.log(` 500 routes: ~${fmt(measure(scale500.routes))}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/bench/realistic.ts` around lines 256 - 279, The measurement currently reports incremental heap deltas (after50, after200, after500) while keeping earlier routers reachable (memRouter50, memRouter200), which mislabels values as per-router costs; either change the printed labels to indicate "incremental" or measure routers in isolation by creating each via buildRouter (memRouter50, memRouter200, memRouter500), calling globalThis.gc(), capturing memory (after50/after200/after500), then immediately release the reference (set memRouterX = null), run globalThis.gc() again before the next build to ensure previous router is collected, or alternatively spawn isolated processes for each measurement—use the existing symbols (buildRouter, memRouter50/memRouter200/memRouter500, do_not_optimize, after50/after200/after500, baseline, fmt) to implement one of these fixes and update the console labels accordingly.
191-206: Consider clarifying that removeRoute benchmark includes addRoute setup.The
removeRoutebenchmark measures the cost of adding all routes and then removing them within each iteration. The benchmark name suggests it measures onlyremoveRoute, but the timing includes bothaddRouteandremoveRouteoperations.If you intend to measure only
removeRoutelatency, consider pre-building the router in the setup phase using mitata's setup mechanism.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/bench/realistic.ts` around lines 191 - 206, The benchmark currently builds the router and calls rou3.addRoute inside each iteration, so the measured time includes both addRoute and removeRoute; change the createCase so route addition happens in the setup phase and the benchmark body only calls rou3.removeRoute (or removes from a cloned/prepared router). Specifically, for the createCase calls in the "removeRoute" group, move the loop that calls rou3.addRoute into the setup function (using mitata's setup mechanism or by prebuilding and returning a router from setup), have the benchmark receive that prepared router and only call do_not_optimize(rou3.removeRoute(router, r.method, r.path)); keep the same route data (scale50.routes and scale200.routes) and use the existing symbols rou3.createRouter, rou3.addRoute, rou3.removeRoute, createCase and do_not_optimize to locate and modify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/bench/realistic.ts`:
- Around line 256-279: The measurement currently reports incremental heap deltas
(after50, after200, after500) while keeping earlier routers reachable
(memRouter50, memRouter200), which mislabels values as per-router costs; either
change the printed labels to indicate "incremental" or measure routers in
isolation by creating each via buildRouter (memRouter50, memRouter200,
memRouter500), calling globalThis.gc(), capturing memory
(after50/after200/after500), then immediately release the reference (set
memRouterX = null), run globalThis.gc() again before the next build to ensure
previous router is collected, or alternatively spawn isolated processes for each
measurement—use the existing symbols (buildRouter,
memRouter50/memRouter200/memRouter500, do_not_optimize,
after50/after200/after500, baseline, fmt) to implement one of these fixes and
update the console labels accordingly.
- Around line 191-206: The benchmark currently builds the router and calls
rou3.addRoute inside each iteration, so the measured time includes both addRoute
and removeRoute; change the createCase so route addition happens in the setup
phase and the benchmark body only calls rou3.removeRoute (or removes from a
cloned/prepared router). Specifically, for the createCase calls in the
"removeRoute" group, move the loop that calls rou3.addRoute into the setup
function (using mitata's setup mechanism or by prebuilding and returning a
router from setup), have the benchmark receive that prepared router and only
call do_not_optimize(rou3.removeRoute(router, r.method, r.path)); keep the same
route data (scale50.routes and scale200.routes) and use the existing symbols
rou3.createRouter, rou3.addRoute, rou3.removeRoute, createCase and
do_not_optimize to locate and modify the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a29fc83f-4647-46b6-a02b-8bc6390871cd
📒 Files selected for processing (4)
README.mdpackage.jsontest/bench/realistic-input.tstest/bench/realistic.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
findRoute,findAllRoutes,addRoute,removeRoute,compileRouter, and memory usageBenchmark Highlights (Apple M3 Max, Node 24)
New files
test/bench/realistic-input.ts— Route sets & request datatest/bench/realistic.ts— Benchmark runner (6 default + 4--fullgroups)Test plan
pnpm bench:realisticruns successfully and produces valid outputgenerateScaleRoutes(500)produces exactly 500 routes🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Tests