Skip to content

event: improve performance of EventEmitter.emit#29633

Closed
mcollina wants to merge 1 commit intonodejs:masterfrom
mcollina:restore-perf-emit
Closed

event: improve performance of EventEmitter.emit#29633
mcollina wants to merge 1 commit intonodejs:masterfrom
mcollina:restore-perf-emit

Conversation

@mcollina
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

nodejs-github-bot commented Sep 23, 2019

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

@BridgeAR
Copy link
Copy Markdown
Member

BridgeAR commented Sep 23, 2019

@nodejs/v8 PTAL

@nodejs-github-bot
Copy link
Copy Markdown
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
Copy Markdown
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
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
Copy Markdown
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
Copy Markdown
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.

@jizusun
Copy link
Copy Markdown
Contributor

jizusun 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.

Oh thank you!
Sorry, I should have looked thoroughly into this writing-and-running-benchmarks.md file.

ZYSzys pushed a commit that referenced this pull request Nov 4, 2019
@jizusun @ZYSzys
buffer: improve performance caused by primordials
22799be 
This is my first PR, and it's based on the code-and-learn guidances
This restore some performance after introducing primordialias.

Refs: #29766
Refs: nodejs/code-and-learn#97
Refs: #29633

PR-URL: #30235
Refs: #29766
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Nov 5, 2019
@jizusun @targos
buffer: improve performance caused by primordials
8860bd6 
This is my first PR, and it's based on the code-and-learn guidances
This restore some performance after introducing primordialias.

Refs: #29766
Refs: nodejs/code-and-learn#97
Refs: #29633

PR-URL: #30235
Refs: #29766
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Nov 8, 2019
@jizusun @targos
buffer: improve performance caused by primordials
b8f9a39 
This is my first PR, and it's based on the code-and-learn guidances
This restore some performance after introducing primordialias.

Refs: #29766
Refs: nodejs/code-and-learn#97
Refs: #29633

PR-URL: #30235
Refs: #29766
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Nov 10, 2019
@jizusun @targos
buffer: improve performance caused by primordials
4d8cd7b 
This is my first PR, and it's based on the code-and-learn guidances
This restore some performance after introducing primordialias.

Refs: #29766
Refs: nodejs/code-and-learn#97
Refs: #29633

PR-URL: #30235
Refs: #29766
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Nov 10, 2019
@jizusun @targos
buffer: improve performance caused by primordials
1f29c01 
This is my first PR, and it's based on the code-and-learn guidances
This restore some performance after introducing primordialias.

Refs: #29766
Refs: nodejs/code-and-learn#97
Refs: #29633

PR-URL: #30235
Refs: #29766
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Nov 11, 2019
@jizusun @targos
buffer: improve performance caused by primordials
680e9cc 
This is my first PR, and it's based on the code-and-learn guidances
This restore some performance after introducing primordialias.

Refs: #29766
Refs: nodejs/code-and-learn#97
Refs: #29633

PR-URL: #30235
Refs: #29766
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@antimonyGu antimonyGu mentioned this pull request Nov 21, 2019
Closed
4 tasks
Trott pushed a commit that referenced this pull request Nov 23, 2019
@antimonyGu @Trott
events: improve performance caused by primordials
e5cbbe0 
PR-URL: #30577
Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
addaleax pushed a commit that referenced this pull request Nov 30, 2019
@antimonyGu @addaleax
events: improve performance caused by primordials
0285aa0 
PR-URL: #30577
Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
targos pushed a commit that referenced this pull request Dec 1, 2019
@antimonyGu @targos
events: improve performance caused by primordials
447c7a1 
PR-URL: #30577
Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
@antimonyGu @MylesBorins
events: improve performance caused by primordials
217cc13 
PR-URL: #30577
Refs: nodejs/code-and-learn#97
Refs: #29766
Refs: #29633
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes

@lpinca lpinca lpinca approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@trivikr trivikr trivikr approved these changes

@ZYSzys ZYSzys ZYSzys approved these changes

@apapirovski apapirovski Awaiting requested review from apapirovski

@mscdex mscdex Awaiting requested review from mscdex

@MylesBorins MylesBorins Awaiting requested review from MylesBorins

+1 more reviewer

@devnexen devnexen devnexen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. events Issues and PRs related to the events subsystem / EventEmitter. performance Issues and PRs related to the performance of Node.js.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.