8000 event: improve performance of EventEmitter.emit by mcollina · Pull Request #29633 · nodejs/node · GitHub
[go: up one dir, main page]
Skip to content

event: improve performance of EventEmitter.emit #29633

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.

Sign up for GitHub

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

Jump to bottom
Closed
mcollina wants to merge 1 commit into from
Closed

event: improve performance of EventEmitter.emit #29633

mcollina wants to merge 1 commit into from

Conversation

@mcollina
Copy link
Member
@mcollina mcollina commented Sep 20, 2019

This restore some performance we lost when we introduced primordialias.
Improvements are up to +100%.

Details 
                                                     confidence improvement accuracy (*)   (**)  (***)
 events/ee-add-remove.js n=1000000                                   0.18 %       ±0.51% ±0.68% ±0.88%
 events/ee-emit.js listeners=1 argc=0 n=2000000             ***     43.53 %       ±1.04% ±1.40% ±1.84%
 events/ee-emit.js listeners=1 argc=10 n=2000000            ***     26.47 %       ±1.61% ±2.14% ±2.79%
 events/ee-emit.js listeners=1 argc=2 n=2000000             ***     22.94 %       ±1.02% ±1.38% ±1.82%
 events/ee-emit.js listeners=1 argc=4 n=2000000             ***     24.83 %       ±2.27% ±3.04% ±3.99%
 events/ee-emit.js listeners=10 argc=0 n=2000000            ***    111.71 %       ±1.43% ±1.91% ±2.50%
 events/ee-emit.js listeners=10 argc=10 n=2000000           ***     29.13 %       ±1.76% ±2.34% ±3.05%
 events/ee-emit.js listeners=10 argc=2 n=2000000            ***     27.52 %       ±1.60% ±2.13% ±2.78%
 events/ee-emit.js listeners=10 argc=4 n=2000000            ***     31.05 %       ±2.91% ±3.92% ±5.19%
 events/ee-emit.js listeners=5 argc=0 n=2000000             ***     73.72 %       ±0.98% ±1.31% ±1.72%
 events/ee-emit.js listeners=5 argc=10 n=2000000            ***     28.93 %       ±1.68% ±2.24% ±2.92%
 events/ee-emit.js listeners=5 argc=2 n=2000000             ***     28.53 %       ±1.71% ±2.30% ±3.03%
 events/ee-emit.js listeners=5 argc=4 n=2000000             ***     28.88 %       ±0.78% ±1.04% ±1.35%
 events/ee-listener-count-on-prototype.js n=50000000                 0.73 %       ±0.81% ±1.08% ±1.41%
 events/ee-listeners-many.js n=5000000                               0.52 %       ±1.13% ±1.51% ±1.96%
 events/ee-listeners.js n=5000000                                   -0.68 %       ±2.06% ±2.77% ±3.66%
 events/ee-once.js argc=0 n=20000000                        ***     14.97 %       ±3.18% ±4.23% ±5.51%
 events/ee-once.js argc=1 n=20000000                        ***      8.85 %       ±1.30% ±1.75% ±2.29%
 events/ee-once.js argc=4 n=20000000                        ***      8.55 %       ±1.10% ±1.47% ±1.91%
 events/ee-once.js argc=5 n=20000000                        ***      7.37 %       ±1.10% ±1.46% ±1.90
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@mcollina
event: improve performance of EventEmitter.emit
c0989f4 
This restore some performance we lost when we introduced primordialias.
Improvements are up to +100%.
@nodejs-github-bot nodejs-github-bot added the events Issues and PRs related to the events subsystem / EventEmitter. label Sep 20, 2019
targos
targos approved these changes Sep 20, 2019
View reviewed changes
Copy link
Member
@targos targos left a comment

Choose a reason for hiding this comment

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

LGTM but we should make the V8 team aware of this

@ZYSzys ZYSzys added the performance Issues and PRs related to the performance of Node.js. label Sep 22, 2019
@nodejs-github-bot
Copy link
Collaborator
nodejs-github-bot commented Sep 23, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/25643/

@BridgeAR
Copy link
Member
BridgeAR commented Sep 23, 2019

@nodejs/v8 PTAL

@nodejs-github-bot
Copy link
Collaborator
nodejs-github-bot commented Sep 24, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/25668/

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 24, 2019
@mcollina
Copy link
Member Author
mcollina commented Sep 24, 2019

Landed in c5f5f84

@mcollina mcollina closed this Sep 24, 2019
@mcollina mcollina deleted the restore-perf-emit branch September 24, 2019 07:19
mcollina added a commit that referenced this pull request Sep 24, 2019
@mcollina
events: improve performance of EventEmitter.emit
c5f5f84 
This restore some performance we lost when we introduced primordialias.
Improvements are up to +100%.

PR-URL: #29633
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR pushed a commit that referenced this pull request Sep 24, 2019
@mcollina @BridgeAR
events: improve performance of EventEmitter.emit
edd7c57 
This restore some performance we lost when we introduced primordialias.
Improvements are up to +100%.

PR-URL: #29633
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR BridgeAR mentioned this pull request Sep 24, 2019
Merged
BridgeAR pushed a commit that referenced this pull request Sep 25, 2019
@mcollina @BridgeAR
events: improve performance of EventEmitter.emit
0a47d06 
This restore some performance we lost when we introduced primordialias.
Improvements are up to +100%.

PR-URL: #29633
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This was referenced Sep 25, 2019
Closed
Closed
This was referenced Sep 26, 2019
Closed
Closed
Closed
Closed
@joyeecheung joyeecheung mentioned this pull request Sep 29, 2019
Closed
jizusun added a commit to jizusun/node that referenced this pull request Nov 3, 2019
@jizusun
buffer: improve performance caused by primordials
3a7c798 
This is my first PR, and it's based on the code-and-learn guidances
This restore some performance after introducing primordialias.

Fixes: nodejs#29766
Refs: nodejs/code-and-learn#97
Refs: nodejs#29633
@jizusun jizusun mentioned this pull request Nov 3, 2019
Closed
2 tasks
@jizusun
Copy link
Contributor
jizusun commented Nov 3, 2019

@mcollina Sir, I just made a similar pull request #30235 , and I also want to get the Details section. Could you help me know how to generate this section? Thanks so much ❤️

@mcollina
Copy link
Member Author
mcollina commented Nov 3, 2019

The content is based on the benchmark data: https://github.com/nodejs/node/blob/master/doc/guides/writing-and-running-benchmarks.md. The details block is markdown: https://gist.github.com/joyrexus/16041f2426450e73f5df9391f7f7ae5f.
0