Add Compression Benchmark Scenario#59
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
uWestJS Benchmark Results
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@benchmarks/scenarios/compression.js`:
- Around line 29-32: The async route at app.get('/compress') calls
handler.compressBuffer without error handling; wrap the await
handler.compressBuffer(req, res, body) call in a local try/catch inside that
route handler (the function passed to app.get('/compress')), and on catch send a
controlled 500 response (e.g.,
res.status(500).setHeader('content-type','text/plain').send('Compression
failed') or similar) to avoid unhandled rejections and benchmark instability;
keep the existing success path (setHeader and res.send(compressed)) unchanged.
- Around line 11-33: The payload is precomputed as a Buffer only for uwestjs
which skews benchmarking; change to build a single shared Buffer (from payload)
once and use that Buffer for all frameworks: replace Express's string send in
the app.get('/compress') handler (used in the 'express' branch) to send the
prebuilt Buffer (keeping res.type('text/plain') and res.send(buffer)), and
replace Fastify's reply.send(payload) in the 'fastify' branch to
reply.send(buffer) (keep reply.type('text/plain') or equivalent), while leaving
the uwestjs CompressionHandler usage as-is (handler.compressBuffer should still
receive the buffer). Ensure the unique symbols payload, app.get('/compress'),
CompressionHandler, handler.compressBuffer, and reply.send are updated
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 12848f94-c12f-4f93-8a46-95278fff7e4a
📒 Files selected for processing (2)
benchmarks/scenarios/compression.jsbenchmarks/wrk-scripts/compression.lua
|
Compression Failure is expected as #57 fix isn't at place. Will open a PR soon |
30b209a to
0ec768b
Compare
Added compression benchmark scenario comparing Express, Fastify, and uWestJS
Closes #58
Summary by CodeRabbit