Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

benchmark: add undici websocket benchmark #50586

Merged
merged 8 commits into from
Dec 6, 2023

Conversation

Ch3nYuY
Copy link
Contributor

@Ch3nYuY Ch3nYuY commented Nov 7, 2023

Refs: nodejs/performance#114

I added a simple benchmark for the undici websocket. I modified the client in https://github.com/websockets/ws/blob/master/bench/speed.js to use undici instead.

Platform
Linux benchmark-S2600WFT 5.15.0-41-generic #44-Ubuntu SMP Wed Jun 22 14:20:53 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

20.x vs 21.x

➜  websocket git: ✗ node-benchmark-compare compare-v20-v21.csv
                                                                   confidence improvement accuracy (*)    (**)   (***)
websocket/simple.js size=1048576 roundtrips=1 useBinary='false'           ***     -4.76 %       ±2.08%  ±3.06%  ±4.65%
websocket/simple.js size=1048576 roundtrips=1 useBinary='true'            ***     -3.67 %       ±0.89%  ±1.42%  ±2.50%
websocket/simple.js size=1048576 roundtrips=100 useBinary='false'         ***      3.80 %       ±1.05%  ±1.57%  ±2.47%
websocket/simple.js size=1048576 roundtrips=100 useBinary='true'                   2.15 %       ±2.42%  ±3.61%  ±5.65%
websocket/simple.js size=1048576 roundtrips=1000 useBinary='false'                 2.45 %       ±3.38%  ±5.38%  ±9.37%
websocket/simple.js size=1048576 roundtrips=1000 useBinary='true'         ***      2.92 %       ±0.96%  ±1.41%  ±2.15%
websocket/simple.js size=1048576 roundtrips=5000 useBinary='false'          *      3.18 %       ±3.04%  ±4.56%  ±7.20%
websocket/simple.js size=1048576 roundtrips=5000 useBinary='true'           *      2.19 %       ±2.03%  ±3.05%  ±4.82%
websocket/simple.js size=131072 roundtrips=1 useBinary='false'             **     -7.30 %       ±4.48%  ±6.73% ±10.66%
websocket/simple.js size=131072 roundtrips=1 useBinary='true'              **     -7.36 %       ±3.40%  ±4.99%  ±7.60%
websocket/simple.js size=131072 roundtrips=100 useBinary='false'          ***     10.01 %       ±3.04%  ±4.94%  ±8.92%
websocket/simple.js size=131072 roundtrips=100 useBinary='true'           ***     10.70 %       ±4.10%  ±6.30% ±10.37%
websocket/simple.js size=131072 roundtrips=1000 useBinary='false'                  1.93 %       ±1.97%  ±2.88%  ±4.35%
websocket/simple.js size=131072 roundtrips=1000 useBinary='true'            *      1.97 %       ±1.73%  ±2.58%  ±4.03%
websocket/simple.js size=131072 roundtrips=5000 useBinary='false'                  0.78 %       ±2.01%  ±2.94%  ±4.46%
websocket/simple.js size=131072 roundtrips=5000 useBinary='true'            *      1.79 %       ±1.67%  ±2.53%  ±4.09%
websocket/simple.js size=16384 roundtrips=1 useBinary='false'              **     -2.66 %       ±1.57%  ±2.29%  ±3.44%
websocket/simple.js size=16384 roundtrips=1 useBinary='true'                      -1.60 %       ±3.18%  ±5.06%  ±8.81%
websocket/simple.js size=16384 roundtrips=100 useBinary='false'            **     11.25 %       ±4.08%  ±6.69% ±12.23%
websocket/simple.js size=16384 roundtrips=100 useBinary='true'            ***     12.92 %       ±4.48%  ±6.53%  ±9.81%
websocket/simple.js size=16384 roundtrips=1000 useBinary='false'           **      6.52 %       ±3.31%  ±5.26%  ±9.16%
websocket/simple.js size=16384 roundtrips=1000 useBinary='true'           ***      6.92 %       ±0.66%  ±0.97%  ±1.47%
websocket/simple.js size=16384 roundtrips=5000 useBinary='false'                   2.79 %       ±4.68%  ±6.88% ±10.51%
websocket/simple.js size=16384 roundtrips=5000 useBinary='true'                    3.22 %       ±3.66%  ±6.05% ±11.24%
websocket/simple.js size=64 roundtrips=1 useBinary='false'                  *     -3.62 %       ±2.36%  ±3.78%  ±6.64%
websocket/simple.js size=64 roundtrips=1 useBinary='true'                   *     -3.05 %       ±2.39%  ±3.72%  ±6.28%
websocket/simple.js size=64 roundtrips=100 useBinary='false'              ***     12.20 %       ±5.13%  ±7.46% ±11.23%
websocket/simple.js size=64 roundtrips=100 useBinary='true'                **     10.84 %       ±4.84%  ±7.83% ±14.05%
websocket/simple.js size=64 roundtrips=1000 useBinary='false'             ***      9.51 %       ±0.92%  ±1.42%  ±2.32%
websocket/simple.js size=64 roundtrips=1000 useBinary='true'                *      7.52 %       ±6.22% ±10.26% ±19.04%
websocket/simple.js size=64 roundtrips=5000 useBinary='false'              **      1.62 %       ±0.90%  ±1.31%  ±1.97%
websocket/simple.js size=64 roundtrips=5000 useBinary='true'                *      1.33 %       ±1.06%  ±1.54%  ±2.34%
 
Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 32 comparisons, you can thus expect the following amount of false-positive results:
  1.60 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.32 false positives, when considering a   1% risk acceptance (**, ***),
  0.03 false positives, when considering a 0.1% risk acceptance (***)

@nodejs-github-bot nodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Nov 7, 2023
benchmark/websocket/simple.js Outdated Show resolved Hide resolved
benchmark/websocket/simple.js Outdated Show resolved Hide resolved
benchmark/websocket/simple.js Show resolved Hide resolved
benchmark/websocket/simple.js Outdated Show resolved Hide resolved
benchmark/websocket/simple.js Show resolved Hide resolved
benchmark/websocket/simple.js Outdated Show resolved Hide resolved
benchmark/websocket/simple.js Outdated Show resolved Hide resolved
benchmark/websocket/simple.js Outdated Show resolved Hide resolved
benchmark/websocket/simple.js Outdated Show resolved Hide resolved
test/benchmark/test-benchmark-websocket.js Outdated Show resolved Hide resolved
@lpinca
Copy link
Member

lpinca commented Nov 14, 2023

There is a typo in a commit message ("benchamrk" -> "benchmark") but I think we will squash commits so it does not matter.

@lpinca lpinca added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Nov 14, 2023
@Ch3nYuY
Copy link
Contributor Author

Ch3nYuY commented Dec 5, 2023

@lpinca I noticed that you have approved this PR for three weeks now, but it has not been merged yet. Is it still waiting for approval from other reviewers? Thank you.

Copy link
Member

@KhafraDev KhafraDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I've been very busy the last few weeks.

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 5, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 5, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/50586
✔  Done loading data for nodejs/node/pull/50586
----------------------------------- PR info ------------------------------------
Title      benchmark: add undici websocket benchmark (#50586)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     chenyuyang2022:ws-bench -> nodejs:main
Labels     benchmark, commit-queue-squash
Commits    8
 - benchmark: add undici websocket benchmark
 - benchmark: update websocket server
 - benchmark: update undici benchmark
 - benchamrk: update undici websocket benchmark
 - test: add test/benchmark/test-benchmark-websocket.js
 - update undici websocket benchmark & test
 - fix lint error
 - update undici websocket benchmark
Committers 1
 - Chenyu Yang 
PR-URL: https://github.com/nodejs/node/pull/50586
Refs: https://github.com/nodejs/performance/issues/114
Reviewed-By: Luigi Pinca 
Reviewed-By: Matthew Aitken 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50586
Refs: https://github.com/nodejs/performance/issues/114
Reviewed-By: Luigi Pinca 
Reviewed-By: Matthew Aitken 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 07 Nov 2023 03:49:19 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/50586#pullrequestreview-1729891063
   ✔  - Matthew Aitken (@KhafraDev): https://github.com/nodejs/node/pull/50586#pullrequestreview-1765869891
   ✔  Last GitHub CI successful
   ✘  No Jenkins CI runs detected
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/7105461887

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 5, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 5, 2023
@nodejs-github-bot
Copy link
Collaborator

@lpinca lpinca added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 6, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 6, 2023
@nodejs-github-bot nodejs-github-bot merged commit 212a972 into nodejs:main Dec 6, 2023
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 212a972

RafaelGSS pushed a commit that referenced this pull request Dec 15, 2023
Refs: nodejs/performance#114
PR-URL: #50586
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Dec 15, 2023
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Refs: nodejs/performance#114
PR-URL: #50586
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Issues and PRs related to the benchmark subsystem. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants