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

events: improve arrayClone performance #33774

Merged
merged 1 commit into from
Jun 8, 2020

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Jun 6, 2020

Benchmark results:

 events/ee-emit.js listeners=10 argc=0 n=6000000                            0.73 %       ±3.58% ±4.82% ±6.37%
 events/ee-emit.js listeners=100 argc=0 n=6000000                  ***      8.96 %       ±0.48% ±0.64% ±0.84%
 events/ee-emit.js listeners=2 argc=0 n=6000000                    ***     16.72 %       ±1.07% ±1.43% ±1.87%
 events/ee-emit.js listeners=3 argc=0 n=6000000                    ***     13.46 %       ±0.66% ±0.88% ±1.14%
 events/ee-emit.js listeners=30 argc=0 n=6000000                   ***      6.85 %       ±0.79% ±1.06% ±1.40%
 events/ee-emit.js listeners=4 argc=0 n=6000000                    ***     10.06 %       ±4.48% ±6.04% ±8.02%
 events/ee-emit.js listeners=5 argc=0 n=6000000                    ***     17.93 %       ±3.18% ±4.28% ±5.68%
 events/ee-emit.js listeners=6 argc=0 n=6000000                    ***     13.06 %       ±3.26% ±4.34% ±5.67%
 events/ee-emit.js listeners=7 argc=0 n=6000000                            -2.73 %       ±3.11% ±4.19% ±5.56%
 events/ee-listeners.js raw='true' listeners=10 n=40000000         ***     19.09 %       ±1.78% ±2.39% ±3.16%
 events/ee-listeners.js raw='true' listeners=2 n=40000000          ***     43.05 %       ±1.10% ±1.47% ±1.91%
 events/ee-listeners.js raw='true' listeners=3 n=40000000          ***     47.95 %       ±1.35% ±1.80% ±2.36%
 events/ee-listeners.js raw='true' listeners=4 n=40000000          ***     58.57 %       ±2.62% ±3.52% ±4.64%
 events/ee-listeners.js raw='true' listeners=5 n=40000000          ***     65.12 %       ±1.31% ±1.74% ±2.27%
 events/ee-listeners.js raw='true' listeners=6 n=40000000          ***     74.60 %       ±1.61% ±2.15% ±2.81%
 events/ee-listeners.js raw='true' listeners=7 n=40000000           **      2.31 %       ±1.37% ±1.84% ±2.41%

Note: array.slice() roughly equals the performance of the original for-loop at a certain point but gets slightly faster as the number of elements increases, which explains some of the results above.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@mscdex mscdex added events Issues and PRs related to the events subsystem / EventEmitter. performance Issues and PRs related to the performance of Node.js. labels Jun 6, 2020
@nodejs-github-bot
Copy link
Collaborator

lib/events.js Show resolved Hide resolved
lib/events.js Show resolved Hide resolved
PR-URL: nodejs#33774
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
@mscdex mscdex force-pushed the events-arrayclone-perf branch from 0765777 to 4ba9080 Compare June 8, 2020 19:22
@nodejs-github-bot
Copy link
Collaborator

@mscdex mscdex merged commit 4ba9080 into nodejs:master Jun 8, 2020
@mscdex mscdex deleted the events-arrayclone-perf branch June 8, 2020 20:21
codebytere pushed a commit that referenced this pull request Jun 18, 2020
PR-URL: #33774
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
@codebytere codebytere mentioned this pull request Jun 28, 2020
codebytere pushed a commit that referenced this pull request Jun 30, 2020
PR-URL: #33774
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #33774
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
events Issues and PRs related to the events subsystem / EventEmitter. performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants