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.

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

stream: writable state bitmap #49899

Merged
merged 1 commit into from Sep 29, 2023
Merged

stream: writable state bitmap #49899

merged 1 commit into from Sep 29, 2023

Conversation

ronag
Copy link
Member

@ronag ronag commented Sep 27, 2023

Just some ideas. Not sure I will have time to finish this. Feel free to take over.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Sep 27, 2023
@ronag ronag force-pushed the writable-state-bimap branch 3 times, most recently from de32558 to 9529f21 Compare September 27, 2023 16:30
@benjamingr
Copy link
Member

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

@ronag ronag force-pushed the writable-state-bimap branch 13 times, most recently from 048fcdf to d7b1625 Compare September 28, 2023 08:14
@rluvaton
Copy link
Member

New benchmark CI

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

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

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

@ronag ronag force-pushed the writable-state-bimap branch 2 times, most recently from 38b4a3f to c39b474 Compare September 28, 2023 09:19
@nodejs-github-bot
Copy link
Collaborator

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

mcollina
mcollina approved these changes Sep 29, 2023
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

huge win. clearBuffer is one of the biggest bottlenecks in http!

@benjamingr
Copy link
Member

Wait until you see what we have planned for optimizing state.buffered :D

@ronag
Copy link
Member Author

ronag commented Sep 29, 2023

Wait until you see what we have planned for optimizing state.buffered :D

No spoilers!

rluvaton
rluvaton approved these changes Sep 29, 2023
View reviewed changes
Copy link
Member

@rluvaton rluvaton left a comment

Choose a reason for hiding this comment

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

well done!

@rluvaton rluvaton added stream Issues and PRs related to the stream subsystem. performance Issues and PRs related to the performance of Node.js. needs-benchmark-ci PR that need a benchmark CI run. labels Sep 29, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@ronag
Copy link
Member Author

ronag commented Sep 29, 2023

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

@ronag
Copy link
Member Author

ronag commented Sep 29, 2023 

17:14:01                                                                                          confidence improvement accuracy (*)   (**)   (***)
17:14:01 streams/creation.js kind='duplex' n=50000000                                                    ***     -8.37 %       ±2.83% ±3.81%  ±5.02%
17:14:01 streams/creation.js kind='readable' n=50000000                                                           0.43 %       ±0.82% ±1.09%  ±1.42%
17:14:01 streams/creation.js kind='transform' n=50000000                                                 ***      5.53 %       ±2.22% ±2.97%  ±3.91%
17:14:01 streams/creation.js kind='writable' n=50000000                                                  ***      8.52 %       ±1.75% ±2.34%  ±3.06%
17:14:01 streams/destroy.js kind='duplex' n=1000000                                                      ***     18.11 %       ±7.45% ±9.97% ±13.10%
17:14:01 streams/destroy.js kind='readable' n=1000000                                                            -0.95 %       ±2.66% ±3.54%  ±4.61%
17:14:01 streams/destroy.js kind='transform' n=1000000                                                   ***     12.84 %       ±6.31% ±8.40% ±10.93%
17:14:01 streams/destroy.js kind='writable' n=1000000                                                    ***      6.03 %       ±2.95% ±3.92%  ±5.11%
17:14:01 streams/pipe-object-mode.js n=5000000                                                                    4.20 %       ±4.66% ±6.21%  ±8.09%
17:14:01 streams/pipe.js n=5000000                                                                         *      3.85 %       ±3.16% ±4.21%  ±5.48%
17:14:01 streams/readable-async-iterator.js sync='no' n=100000                                                    0.03 %       ±3.28% ±4.37%  ±5.69%
17:14:01 streams/readable-async-iterator.js sync='yes' n=100000                                            *      6.32 %       ±5.55% ±7.42%  ±9.72%
17:14:01 streams/readable-bigread.js n=1000                                                                      -0.77 %       ±2.72% ±3.62%  ±4.72%
17:14:01 streams/readable-bigunevenread.js n=1000                                                                -5.28 %       ±5.59% ±7.44%  ±9.69%
17:14:01 streams/readable-boundaryread.js type='buffer' n=2000                                                    1.21 %       ±2.04% ±2.72%  ±3.55%
17:14:01 streams/readable-boundaryread.js type='string' n=2000                                                    0.33 %       ±0.59% ±0.79%  ±1.03%
17:14:01 streams/readable-from.js n=10000000                                                                      0.36 %       ±2.37% ±3.16%  ±4.12%
17:14:01 streams/readable-readall.js n=5000                                                                       0.73 %       ±1.85% ±2.47%  ±3.24%
17:14:01 streams/readable-unevenread.js n=1000                                                                    0.15 %       ±2.02% ±2.68%  ±3.49%
17:14:01 streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='no' n=100000              *      2.17 %       ±1.88% ±2.52%  ±3.32%
17:14:01 streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='yes' n=100000           ***      8.46 %       ±3.42% ±4.56%  ±5.94%
17:14:01 streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='no' n=100000                   -0.55 %       ±3.00% ±3.99%  ±5.19%
17:14:01 streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='yes' n=100000          ***      7.85 %       ±2.48% ±3.30%  ±4.30%
17:14:01 streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='no' n=100000                   -1.87 %       ±2.39% ±3.18%  ±4.14%
17:14:01 streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='yes' n=100000          ***     -5.02 %       ±2.68% ±3.57%  ±4.66%
17:14:01 streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='no' n=100000                   0.50 %       ±2.31% ±3.07%  ±4.01%
17:14:01 streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='yes' n=100000          **      4.24 %       ±2.61% ±3.48%  ±4.53%
17:14:01 streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='no' n=100000                    1.17 %       ±3.40% ±4.53%  ±5.91%
17:14:01 streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='yes' n=100000          ***     12.55 %       ±2.83% ±3.80%  ±5.01%
17:14:01 streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='no' n=100000                   0.45 %       ±1.42% ±1.89%  ±2.46%
17:14:01 streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='yes' n=100000         ***      8.27 %       ±3.76% ±5.02%  ±6.56%
17:14:01 streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='no' n=100000                  -1.06 %       ±2.82% ±3.75%  ±4.88%
17:14:01 streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='yes' n=100000          **      3.53 %       ±2.24% ±2.99%  ±3.94%
17:14:01 streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='no' n=100000           *     -3.02 %       ±2.39% ±3.19%  ±4.18%
17:14:01 streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='yes' n=100000                 1.87 %       ±2.48% ±3.32%  ±4.36%

@nodejs-github-bot
Copy link
Collaborator

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

@ronag ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 29, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 29, 2023
@nodejs-github-bot nodejs-github-bot merged commit 53b5545 into nodejs:main Sep 29, 2023
61 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 53b5545

This was referenced Sep 30, 2023
Open
Open
GeoffreyBooth pushed a commit to GeoffreyBooth/node that referenced this pull request Oct 1, 2023
@ronag @GeoffreyBooth
stream: writable state bitmap
1943620 
PR-URL: nodejs#49899
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@github-actions github-actions bot mentioned this pull request Oct 2, 2023
Open
12 tasks
@RafaelGSS RafaelGSS mentioned this pull request Oct 9, 2023
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@ronag @alexfernandez
stream: writable state bitmap
fa9d333 
PR-URL: nodejs#49899
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
targos pushed a commit that referenced this pull request Nov 11, 2023
@ronag @targos
stream: writable state bitmap
35ec931 
PR-URL: #49899
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@targos targos mentioned this pull request Nov 12, 2023
Merged
@juanarbol juanarbol mentioned this pull request Dec 6, 2023
Merged
@dead-claudia dead-claudia mentioned this pull request Feb 12, 2024
Closed
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
@ronag @debadree25
stream: writable state bitmap
24e646b 
PR-URL: nodejs#49899
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamingr benjamingr benjamingr approved these changes

@mcollina mcollina mcollina approved these changes

@anonrig anonrig anonrig approved these changes

@rluvaton rluvaton rluvaton approved these changes

Assignees
No one assigned
Labels
needs-benchmark-ci PR that need a benchmark CI run. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ronag @nodejs-github-bot @benjamingr @rluvaton @mcollina @anonrig