feat: add reusePort benchmark #22
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a GitHub Actions CI workflow and removes Travis config, introduces an install script for wrk and an npm script, updates package.json dependency aliases and scripts, starts additional egg-cluster instances with reusePort/worker_threads, and expands the test runner to probe ports 7002–7009 with latency measurement. Changes
Sequence Diagram(s)sequenceDiagram
participant Actions as GitHub Actions
participant Installer as install-wrk.sh / npm (install-wrk)
participant CI_Script as npm ci script
participant Tester as simple/run.sh (wrk)
participant Dispatcher as simple/dispatch.js
participant Cluster as egg-cluster manager
participant Worker as worker process/thread
Actions->>Installer: run `install-wrk` (clone & build wrk)
Actions->>CI_Script: invoke CI script (PATH includes wrk/bin)
CI_Script->>Tester: execute `simple/run.sh` (wrk tests)
Tester->>Dispatcher: ensure servers started / trigger dispatch
Dispatcher->>Cluster: start clusters on ports 7002..7009 (reusePort / worker_threads)
Cluster->>Worker: spawn workers (processes/threads)
Tester->>Worker: run wrk probes against ports 7002..7009
Worker-->>Tester: return responses (latency/throughput)
Tester-->>Actions: surface results/logs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds a benchmark for the egg3 framework with reusePort=true feature to measure performance impact. It also migrates CI from Travis to GitHub Actions and updates dependency management.
- Adds new benchmark endpoint on port 7008 for egg3 with reusePort enabled
- Migrates from Travis CI to GitHub Actions with Node.js 20, 22, and 24
- Updates egg dependency declarations to use npm alias syntax and adds egg-cluster beta dependency
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| simple/run.sh | Adds reusePort benchmark, reorders tests to prioritize egg3 variants, adds --latency flag to all wrk commands |
| simple/dispatch.js | Adds new egg-cluster instance with reusePort enabled on port 7008, adds version logging |
| package.json | Adds egg-cluster 2.4.0-beta.0 dependency, updates egg aliases to npm: syntax, adds install-wrk script |
| install-wrk.sh | New script to install wrk benchmark tool |
| README.md | Converts list formatting from dashes to asterisks, adds contributors section |
| .travis.yml | Removes Travis CI configuration (file deleted) |
| .github/workflows/nodejs.yml | Adds GitHub Actions workflow for CI with Node.js versions 20, 22, 24 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
simple/dispatch.js
Outdated
| require('egg-cluster').startCluster(({ | ||
| workers, | ||
| baseDir: __dirname, | ||
| port: 7008, | ||
| framework: 'egg3', | ||
| reusePort: true, | ||
| })); |
There was a problem hiding this comment.
There's an extra opening parenthesis in the startCluster call. The code has require('egg-cluster').startCluster(({ but it should be require('egg-cluster').startCluster({ - remove one opening parenthesis.
| require('egg-cluster').startCluster(({ | |
| workers, | |
| baseDir: __dirname, | |
| port: 7008, | |
| framework: 'egg3', | |
| reusePort: true, | |
| })); | |
| require('egg-cluster').startCluster({ | |
| workers, | |
| baseDir: __dirname, | |
| port: 7008, | |
| framework: 'egg3', | |
| reusePort: true, | |
| }); |
| @@ -0,0 +1,3 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| [ ! -f wrk/bin/wrk ] && rm -rf wrk && git clone --depth 1 https://github.com/wg/wrk.git && make -j10 -C wrk && mkdir wrk/bin && mv wrk/wrk wrk/bin || true | |||
There was a problem hiding this comment.
The install-wrk.sh script clones and builds wrk from the mutable default branch of https://github.com/wg/wrk.git and immediately runs make on the fetched repository. This means any compromise of that repository (or a malicious change on its default branch) would execute attacker-controlled build steps with your local or CI privileges, potentially exfiltrating secrets or modifying artifacts. To reduce this supply-chain risk, fetch and build from a pinned immutable reference (specific commit or signed release) and consider verifying integrity (e.g., via checksum or signature) before running make.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
simple/dispatch.js (2)
7-7: Use imported os module consistently.The imported
osmodule at line 5 should be used here instead of callingrequire('os')again.Apply this diff:
-let workers = Number(process.argv[2] || require('os').cpus().length); +let workers = Number(process.argv[2] || os.cpus().length);
37-37: Fix syntax error: remove extra opening parenthesis.The call to
require('egg-cluster').startClusterhas an extra opening parenthesis:startCluster(({should bestartCluster({.Apply this diff to fix the syntax error:
- require('egg-cluster').startCluster(({ + require('egg-cluster').startCluster({README.md (1)
31-31: Add documentation for the new reusePort benchmark.The README is missing an entry for the new egg3 with reusePort benchmark on port 7008 that was added in this PR.
Apply this diff to add the missing documentation:
* egg3 with worker_threads: `wrk http://127.0.0.1:7006/ -d 10 -c 50 -t 8` +* egg3 with reusePort: `wrk http://127.0.0.1:7008/ -d 10 -c 50 -t 8`install-wrk.sh (1)
3-3: Supply-chain security risk: pin to immutable reference.The script clones and builds
wrkfrom the mutable default branch ofhttps://github.com/wg/wrk.git. A compromise or malicious change on that branch could execute attacker-controlled build steps. Pin to a specific commit or signed release tag and consider verifying integrity before runningmake.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/nodejs.yml(1 hunks).travis.yml(0 hunks)README.md(1 hunks)install-wrk.sh(1 hunks)package.json(2 hunks)simple/dispatch.js(2 hunks)simple/run.sh(2 hunks)
💤 Files with no reviewable changes (1)
- .travis.yml
🧰 Additional context used
🧬 Code graph analysis (1)
simple/run.sh (2)
simple_passport/run.sh (1)
print_head(11-17)simple_view/run.sh (1)
print_head(25-31)
🪛 LanguageTool
README.md
[grammar] ~16-~16: Ensure spelling is correct
Context: ...led, because in most situation we won't caculate csrf token and set to cookie ## Scenes...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Node.js / Test (ubuntu-latest, 22)
- GitHub Check: Node.js / Test (ubuntu-latest, 24)
- GitHub Check: Node.js / Test (ubuntu-latest, 20)
🔇 Additional comments (3)
simple/run.sh (2)
17-17: LGTM - Port 7008 added for reusePort testing.The new curl command correctly probes port 7008, which corresponds to the egg3 cluster with reusePort enabled in dispatch.js.
29-98: LGTM - Test sequence properly reorganized.The test sequence has been reorganized to prioritize egg3 variants (standard, reusePort, worker_threads) before testing other frameworks. The addition of the
--latencyflag to all wrk commands provides consistent performance measurement across all benchmarks..github/workflows/nodejs.yml (1)
1-16: LGTM - GitHub Actions workflow properly configured.The workflow correctly:
- Triggers on push and pull requests to master
- Reuses a standard node-test workflow from node-modules/github-actions
- Tests across Node.js 20, 22, and 24
- Installs wrk before running tests via the install-wrk script
The
cat /proc/cpuinfocommand at the end of the install step provides useful CPU information for benchmark context.
package.json
Outdated
| "description": "egg benchmark", | ||
| "private": true, | ||
| "dependencies": { | ||
| "egg-cluster": "2.4.0-beta.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest version of egg-cluster npm package
💡 Result:
Latest version: egg-cluster 2.4.0 — as of December 12, 2025. [1][2]
Sources:
[1] npm package page for egg-cluster.
[2] Snyk / package info for egg-cluster.
Update egg-cluster to the latest stable version 2.4.0.
The PR currently specifies egg-cluster@2.4.0-beta.0, but the stable release 2.4.0 is available. For benchmarking purposes, use the stable version instead of the beta.
🤖 Prompt for AI Agents
In package.json around line 7, the dependency currently pins "egg-cluster" to
the beta release "2.4.0-beta.0"; update it to the stable release "2.4.0" by
replacing the version string so the project uses the stable package for
benchmarking and CI.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
simple/dispatch.js (2)
5-7: Use the importedosmodule instead of re-requiring it.
Line 7 redundantly doesrequire('os')...despiteconst os = require('os');at Line 5.-let workers = Number(process.argv[2] || require('os').cpus().length); +let workers = Number(process.argv[2] || os.cpus().length);
37-43: Fix syntax: extra parentheses inegg-cluster.startClustercall.
Line 37’sstartCluster(( { ... } ))looks like a syntax error; should bestartCluster({ ... }).- require('egg-cluster').startCluster(({ + require('egg-cluster').startCluster({ workers, baseDir: __dirname, port: 7008, framework: 'egg3', reusePort: true, - })); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
simple/dispatch.js(3 hunks)simple/run.sh(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- simple/run.sh
🧰 Additional context used
🧬 Code graph analysis (1)
simple/dispatch.js (2)
simple_passport/dispatch.js (2)
workers(5-5)egg3(1-1)simple_view/dispatch.js (3)
workers(6-6)cluster(4-4)egg3(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Node.js / Test (ubuntu-latest, 22)
- GitHub Check: Node.js / Test (ubuntu-latest, 24)
- GitHub Check: Node.js / Test (ubuntu-latest, 20)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
simple/dispatch.js (3)
37-43: ConfirmreusePort(andstartMode: 'worker_threads'on egg-cluster) is actually supported; otherwise results are misleading.
You’re passingreusePort: truetoegg-cluster.startClustertwice; if unsupported/ignored, the “reusePort” benchmark labels won’t reflect reality.egg-cluster startCluster options reusePort egg-cluster startCluster startMode worker_threads ports optionAlso applies to: 61-68
37-43: Fix syntax: extra parentheses breakstartClustercall.
startCluster(( { ... } ))should bestartCluster({ ... }).- require('egg-cluster').startCluster(({ + require('egg-cluster').startCluster({ workers, baseDir: __dirname, port: 7008, framework: 'egg3', reusePort: true, - })); + });
5-10: Use the importedosmodule instead ofrequire('os')again.
Line 7 should useos.cpus().lengthfor consistency and to avoid double-require.-let workers = Number(process.argv[2] || require('os').cpus().length); +let workers = Number(process.argv[2] || os.cpus().length);
🧹 Nitpick comments (2)
simple/run.sh (2)
11-19: Add basic readiness checks (curl retries / fail-fast) so benchmarks don’t run against half-started servers.
Right now a slow boot just prints curl errors and continues towrk, skewing results.
7-10: Ensure server process is always cleaned up (trap on EXIT).
If the script exits early,dispatch.jsmay be left running and contaminate later runs.EGG_SERVER_ENV=prod node $NODE_FLAGS `dirname $0`/dispatch.js $1 & pid=$! +trap 'kill $pid 2>/dev/null || true' EXITAlso applies to: 127-129
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(2 hunks)simple/dispatch.js(3 hunks)simple/run.sh(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
simple/dispatch.js (2)
simple_passport/dispatch.js (2)
workers(5-5)egg3(1-1)simple_view/dispatch.js (3)
workers(6-6)cluster(4-4)egg3(1-1)
simple/run.sh (2)
simple_passport/run.sh (1)
print_head(11-17)simple_view/run.sh (1)
print_head(25-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Node.js / Test (ubuntu-latest, 24)
- GitHub Check: Node.js / Test (ubuntu-latest, 22)
- GitHub Check: Node.js / Test (ubuntu-latest, 20)
🔇 Additional comments (2)
simple/dispatch.js (1)
13-15: Startup logging looks fine; consider hardening theegg-cluster/package.jsonrequire if optional.
Ifegg-clusteris always installed for this benchmark, OK; otherwise this will crash master at boot.simple/run.sh (1)
31-90: Benchmark titles assume reusePort works—ensure the runtime actually enabled it.
Ifsimple/dispatch.jscan’t enable reusePort (or it’s ignored), the “reusePort=true” blocks (7008/7009) will be mislabeled.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
simple/dispatch.js (2)
7-7: Use the importedosmodule instead of requiring it again.The
osmodule is imported at line 5, but line 7 callsrequire('os').cpus()directly. Use the imported module for consistency.Apply this diff:
-let workers = Number(process.argv[2] || require('os').cpus().length); +let workers = Number(process.argv[2] || os.cpus().length);
37-43: Fix syntax error and verifyreusePortsupport in egg-cluster 2.5.0-beta.3.Two issues:
Syntax error: Line 37 has an extra opening parenthesis
(({— should be({.Verify
reusePortoption: Previous review noted thatreusePortis not supported in egg-cluster 2.4.0-beta.0. Since package.json now specifies 2.5.0-beta.3, confirm whether this version supports thereusePortoption instartCluster.Apply this diff to fix the syntax error:
- require('egg-cluster').startCluster(({ + require('egg-cluster').startCluster({ workers, baseDir: __dirname, port: 7008, framework: 'egg3', reusePort: true, - })); + });Run the following script to verify
reusePortsupport:#!/bin/bash # Check if egg-cluster 2.5.0-beta.3 supports reusePort option in startCluster # Look for reusePort in egg-cluster source code rg -nP --type=js -C3 'reusePort' node_modules/egg-cluster/ # Check egg-cluster changelog or documentation for reusePort fd -t f "CHANGELOG|HISTORY|README" node_modules/egg-cluster/ --exec rg -nP -i "reusePort|reuse.?port" {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(2 hunks)simple/dispatch.js(3 hunks)simple/run.sh(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
simple/run.sh (2)
simple_passport/run.sh (1)
print_head(11-17)simple_view/run.sh (1)
print_head(25-31)
simple/dispatch.js (2)
simple_passport/dispatch.js (1)
workers(5-5)simple_view/dispatch.js (2)
workers(6-6)cluster(4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Node.js / Test (ubuntu-latest, 24)
- GitHub Check: Node.js / Test (ubuntu-latest, 22)
- GitHub Check: Node.js / Test (ubuntu-latest, 20)
🔇 Additional comments (8)
simple/dispatch.js (1)
13-14: LGTM!The logging statements correctly display OS and egg-cluster versions for benchmarking context.
simple/run.sh (7)
17-18: LGTM!The curl commands for ports 7008 and 7009 correctly verify the new egg-cluster instances before running benchmarks.
32-40: Verify test labels ifreusePortis unsupported.The test block correctly targets port 7008 with appropriate wrk configuration. However, if
reusePortis not supported in egg-cluster 2.5.0-beta.3 (as flagged in dispatch.js review), the test label "egg3 hello with reusePort=true" may be misleading since the feature wouldn't actually be enabled.Ensure dispatch.js lines 37-43 are verified first — if
reusePortis unsupported, consider updating the test labels to reflect the actual configuration.
44-52: LGTM!The test block correctly benchmarks the standard egg3 configuration on port 7005 with appropriate duration and latency measurement.
56-64: LGTM!The test block accurately benchmarks egg3 with worker_threads mode (1 worker) on port 7006.
68-76: Verify test labels ifreusePortis unsupported.The test block correctly targets port 7009 with worker_threads configuration. However, if
reusePortis not supported in egg-cluster 2.5.0-beta.3 (as flagged in dispatch.js review), consider updating the test label accordingly.Depends on verification of dispatch.js lines 53-60.
80-88: LGTM!The koa benchmark on port 7002 is correctly configured with consistent duration and latency measurement.
92-112: LGTM!The egg1 and egg2 test blocks provide valuable comparison benchmarks across framework versions. Port configurations and wrk parameters are correct.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
eggjs/egg#5365
Summary by CodeRabbit
Chores
Documentation
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.