Skip to content

stream: avoid tick in writable hot path#49966

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
nxtedition:writable-avoid-tick
Oct 1, 2023
Merged

stream: avoid tick in writable hot path#49966
nodejs-github-bot merged 1 commit intonodejs:mainfrom
nxtedition:writable-avoid-tick

Conversation

@ronag
Copy link
Member

@ronag ronag commented Sep 29, 2023 

streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='no' n=100000            ***      4.11 %       ±1.32% ±1.76% ±2.31%
streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='yes' n=100000           ***     -2.82 %       ±1.32% ±1.77% ±2.34%
streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='no' n=100000           ***      4.15 %       ±1.63% ±2.18% ±2.85%
streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='yes' n=100000          ***      2.74 %       ±1.17% ±1.57% ±2.06%
streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='no' n=100000           ***      4.59 %       ±1.55% ±2.06% ±2.68%
streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='yes' n=100000          ***      5.49 %       ±2.10% ±2.80% ±3.65%
streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='no' n=100000          ***      6.62 %       ±2.87% ±3.82% ±4.98%
streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='yes' n=100000           *      5.87 %       ±4.72% ±6.34% ±8.37%
streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='no' n=100000           ***      5.45 %       ±1.31% ±1.74% ±2.27%
streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='yes' n=100000          ***      7.98 %       ±1.71% ±2.28% ±2.99%
streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='no' n=100000          ***      5.86 %       ±1.37% ±1.83% ±2.38%
streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='yes' n=100000         ***      8.15 %       ±1.79% ±2.41% ±3.19%
streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='no' n=100000          ***      4.93 %       ±0.67% ±0.90% ±1.17%
streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='yes' n=100000         ***      9.06 %       ±2.03% ±2.71% ±3.54%
streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='no' n=100000         ***      6.20 %       ±1.85% ±2.47% ±3.23%
streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='yes' n=100000        ***      8.49 %       ±0.85% ±1.13% ±1.47%

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 29, 2023

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Sep 29, 2023
ronag
ronag commented Sep 29, 2023
View reviewed changes
@ronag ronag force-pushed the writable-avoid-tick branch from 3813f33 to dc8621c Compare September 29, 2023 20:15
benjamingr
benjamingr reviewed Sep 29, 2023
View reviewed changes
@ronag ronag added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 30, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 30, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 30, 2023

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

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

@mcollina mcollina left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Lgtm

@lpinca
Copy link
Member

lpinca commented Sep 30, 2023

Test failures seem related.

@ronag ronag removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 30, 2023
@ronag ronag force-pushed the writable-avoid-tick branch 5 times, most recently from e85a819 to 9d69bf9 Compare September 30, 2023 09:17
ronag added a commit to nxtedition/node that referenced this pull request Sep 30, 2023
@ronag
stream: avoid tick in writable hot path
4b293ec 
PR-URL: nodejs#49966
@ronag ronag force-pushed the writable-avoid-tick branch from 9d69bf9 to 4b293ec Compare September 30, 2023 09:18
@ronag ronag added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Sep 30, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 30, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 30, 2023

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

@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 30, 2023
@ronag ronag force-pushed the writable-avoid-tick branch from c17b6c7 to f164932 Compare September 30, 2023 13:50
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 30, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 30, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 30, 2023

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

@ronag ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 30, 2023
@github-actions github-actions bot mentioned this pull request Oct 1, 2023
Open
16 tasks
@ronag

This comment was marked as resolved.

Sign in to view
@MoLow

This comment was marked as resolved.

Sign in to view
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 1, 2023
@nodejs-github-bot nodejs-github-bot merged commit 5de25de into nodejs:main Oct 1, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 1, 2023

Landed in 5de25de

This was referenced Oct 2, 2023
Open
Open
@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: avoid tick in writable hot path
8dbc435 
PR-URL: nodejs#49966
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Nov 11, 2023
@ronag @targos
stream: avoid tick in writable hot path
def55f8 
PR-URL: #49966
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targos targos mentioned this pull request Nov 12, 2023
Merged
@juanarbol juanarbol mentioned this pull request Dec 6, 2023
Merged
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
@ronag @debadree25
stream: avoid tick in writable hot path
b20e7c8 
PR-URL: nodejs#49966
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes

@benjamingr benjamingr benjamingr approved these changes

@lpinca lpinca lpinca approved these changes

@anonrig anonrig anonrig approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ronag @nodejs-github-bot @lpinca @MoLow @mcollina @benjamingr @anonrig