Skip to content

stream: avoid calls to listenerCount - #50357

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
nxtedition:readable-data-listener
Oct 26, 2023
Merged

stream: avoid calls to listenerCount#50357
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
nxtedition:readable-data-listener

Conversation

@ronag

@ronagronag commented Oct 24, 2023

Copy link
Copy Markdown
Member
streams/pipe-object-mode.js n=5000000 ** 6.60 % ±4.18% ±5.73% ±7.81%
streams/pipe.js n=5000000 * 3.89 % ±3.24% ±4.59% ±6.59%

@nodejs-github-bot

nodejs-github-bot commented Oct 24, 2023•
edited by ronag
Loading

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 24, 2023
@ronag

ronag commented Oct 24, 2023

Copy link
Copy Markdown
MemberAuthor

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1455/

@ronag
ronagforce-pushed the readable-data-listener branch from bbba726 to 6ecfbccCompareOctober 24, 2023 06:31
@ronagronag changed the title streams: avoid calls to listenerCountstream: avoid calls to listenerCountOct 24, 2023
@ronag
ronagforce-pushed the readable-data-listener branch from 6ecfbcc to f9f02fdCompareOctober 24, 2023 06:59
@ronagronag added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 24, 2023

Copy link
Copy Markdown
Collaborator

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

Uzlopak
Uzlopak approved these changes Oct 24, 2023
View reviewed changes

@UzlopakUzlopak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@ronagronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 24, 2023
@rluvatonrluvaton added stream Issues and PRs related to the stream subsystem. performance Issues and PRs related to the performance of Node.js. labels Oct 24, 2023
@H4ad

H4ad commented Oct 24, 2023

Copy link
Copy Markdown
Member

Results:

05:10:29 confidence improvement accuracy (*) (**) (***)
05:10:29 streams/creation.js kind='duplex' n=50000000 -0.12 % ±1.06% ±1.42% ±1.86%
05:10:29 streams/creation.js kind='readable' n=50000000 0.44 % ±1.11% ±1.48% ±1.93%
05:10:29 streams/creation.js kind='transform' n=50000000 *** 9.73 % ±3.59% ±4.79% ±6.25%
05:10:29 streams/creation.js kind='writable' n=50000000 0.85 % ±1.32% ±1.75% ±2.28%
05:10:29 streams/destroy.js kind='duplex' n=1000000 * 2.25 % ±2.02% ±2.69% ±3.50%
05:10:29 streams/destroy.js kind='readable' n=1000000 -1.58 % ±2.03% ±2.70% ±3.52%
05:10:29 streams/destroy.js kind='transform' n=1000000 -0.20 % ±1.53% ±2.04% ±2.66%
05:10:29 streams/destroy.js kind='writable' n=1000000 -0.21 % ±1.61% ±2.14% ±2.79%
05:10:29 streams/pipe-object-mode.js n=5000000 -1.85 % ±3.71% ±4.93% ±6.43%
05:10:29 streams/pipe.js n=5000000 -2.06 % ±3.11% ±4.14% ±5.39%
05:10:29 streams/readable-async-iterator.js sync='no' n=100000 -2.98 % ±5.78% ±7.72% ±10.10%
05:10:29 streams/readable-async-iterator.js sync='yes' n=100000 -2.00 % ±3.70% ±4.99% ±6.61%
05:10:29 streams/readable-bigread.js n=1000 0.42 % ±1.88% ±2.50% ±3.26%
05:10:29 streams/readable-bigunevenread.js n=1000 1.00 % ±1.72% ±2.30% ±3.00%
05:10:29 streams/readable-boundaryread.js type='buffer' n=2000 * 0.76 % ±0.75% ±1.00% ±1.31%
05:10:29 streams/readable-boundaryread.js type='string' n=2000 -0.43 % ±0.54% ±0.72% ±0.94%
05:10:29 streams/readable-from.js type='array' n=10000000 -7.15 % ±7.48% ±10.07% ±13.34%
05:10:29 streams/readable-from.js type='async-generator' n=10000000 *** 6.43 % ±1.16% ±1.55% ±2.03%
05:10:29 streams/readable-from.js type='sync-generator-with-async-values' n=10000000 *** 9.24 % ±1.61% ±2.15% ±2.81%
05:10:29 streams/readable-from.js type='sync-generator-with-sync-values' n=10000000 -0.93 % ±1.55% ±2.07% ±2.70%
05:10:29 streams/readable-readall.js n=5000 -1.37 % ±2.93% ±3.90% ±5.08%
05:10:29 streams/readable-unevenread.js n=1000 -0.12 % ±0.41% ±0.54% ±0.71%
05:10:29 streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='no' n=100000 0.11 % ±0.96% ±1.27% ±1.66%
05:10:29 streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='yes' n=100000 0.22 % ±3.12% ±4.15% ±5.41%
05:10:29 streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='no' n=100000 0.35 % ±2.28% ±3.04% ±3.95%
05:10:29 streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='yes' n=100000 -1.60 % ±2.48% ±3.30% ±4.30%
05:10:29 streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='no' n=100000 0.18 % ±1.18% ±1.57% ±2.04%
05:10:29 streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='yes' n=100000 0.60 % ±2.40% ±3.19% ±4.15%
05:10:29 streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='no' n=100000 0.11 % ±1.85% ±2.46% ±3.21%
05:10:29 streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='yes' n=100000 -1.29 % ±2.55% ±3.40% ±4.44%
05:10:29 streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='no' n=100000 -0.47 % ±0.94% ±1.25% ±1.63%
05:10:29 streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='yes' n=100000 1.63 % ±2.50% ±3.33% ±4.34%
05:10:29 streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='no' n=100000 -1.18 % ±2.30% ±3.09% ±4.07%
05:10:29 streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='yes' n=100000 -0.25 % ±2.47% ±3.29% ±4.28%
05:10:29 streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='no' n=100000 -0.38 % ±0.97% ±1.29% ±1.68%
05:10:29 streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='yes' n=100000 0.03 % ±2.58% ±3.44% ±4.48%
05:10:29 streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='no' n=100000 0.17 % ±1.26% ±1.68% ±2.18%
05:10:29 streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='yes' n=100000 -1.21 % ±2.20% ±2.93% ±3.81%

@nodejs-github-bot

nodejs-github-bot commented Oct 24, 2023

Copy link
Copy Markdown
Collaborator

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

@github-actionsgithub-actionsBot mentioned this pull request Oct 25, 2023
Open
16 tasks
@nodejs-github-bot

nodejs-github-bot commented Oct 25, 2023

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Oct 25, 2023

Copy link
Copy Markdown
Collaborator

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

@anonriganonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 25, 2023
mcollina
mcollina approved these changes Oct 25, 2023
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@github-actionsgithub-actionsBot mentioned this pull request Oct 26, 2023
Open
20 tasks
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 26, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 79be4ea into nodejs:mainOct 26, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 26, 2023

Copy link
Copy Markdown
Collaborator

Landed in 79be4ea

This was referenced Oct 27, 2023
Open
Open
targos pushed a commit that referenced this pull request Nov 11, 2023
@ronag@targos
stream: avoid calls to listenerCount
893024c
PR-URL: #50357
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@targostargos mentioned this pull request Nov 12, 2023
Merged
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
@ronag@UlisesGascon
stream: avoid calls to listenerCount
59f7316
PR-URL: #50357
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@UlisesGasconUlisesGascon mentioned this pull request Dec 12, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@benjamingrbenjamingrbenjamingr approved these changes
@anonriganonriganonrig approved these changes
@H4adH4adH4ad approved these changes
@rluvatonrluvatonrluvaton approved these changes
+1 more reviewer
@UzlopakUzlopakUzlopak approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.performanceIssues and PRs related to the performance of Node.js.streamIssues and PRs related to the stream subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@ronag@nodejs-github-bot@H4ad@mcollina@benjamingr@anonrig@Uzlopak@rluvaton