perf: optimize Writable #50012

ronag · 2023-10-02T10:06:21Z

Try to inline condition checks and try to avoid read state properties in hot paths.

                                                                                          confidence improvement accuracy (*)   (**)  (***)
streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='no' n=100000            ***      8.14 %       ±4.15% ±5.56% ±7.29%
streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='yes' n=100000           ***     11.35 %       ±2.60% ±3.46% ±4.50%
streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='no' n=100000             *      2.04 %       ±2.03% ±2.71% ±3.54%
streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='yes' n=100000          ***     17.89 %       ±2.05% ±2.73% ±3.56%
streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='no' n=100000           ***      5.99 %       ±2.28% ±3.04% ±3.97%
streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='yes' n=100000          ***     11.62 %       ±2.35% ±3.13% ±4.08%
streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='no' n=100000          ***      3.82 %       ±1.59% ±2.12% ±2.78%
streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='yes' n=100000         ***     14.31 %       ±3.41% ±4.56% ±5.99%
streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='no' n=100000           ***      6.87 %       ±2.67% ±3.55% ±4.63%
streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='yes' n=100000          ***      7.86 %       ±2.17% ±2.90% ±3.79%
streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='no' n=100000          ***      8.60 %       ±4.29% ±5.74% ±7.53%
streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='yes' n=100000         ***      7.28 %       ±2.42% ±3.24% ±4.26%
streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='no' n=100000          ***      6.23 %       ±1.55% ±2.07% ±2.69%
streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='yes' n=100000         ***     10.18 %       ±1.71% ±2.28% ±2.98%
streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='no' n=100000         ***      5.76 %       ±1.70% ±2.26% ±2.95%
streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='yes' n=100000        ***     10.23 %       ±2.66% ±3.55% ±4.64%

nodejs-github-bot · 2023-10-02T10:06:26Z

Review requested:

@nodejs/streams

PR-URL: nodejs#50012

ronag · 2023-10-02T10:07:08Z

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

mcollina

lgtm

lib/internal/streams/writable.js

PR-URL: nodejs#50012

benjamingr · 2023-10-02T10:18:53Z

lib/internal/streams/writable.js

      } else {
        state.pendingcb--;
-        finishMaybe(stream, state, true);
+        if ((state.state & kEnding) !== 0) {


We may as well optimize needFinish?

I think this is more about inlining.

We could maybe inline needFinish but that would impact readability for little gain

anonrig

🚀

ronag · 2023-10-02T11:45:19Z

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

nodejs-github-bot · 2023-10-02T11:52:07Z

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

nodejs-github-bot · 2023-10-02T12:29:47Z

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

PR-URL: nodejs#50012

benjamingr · 2023-10-03T16:56:13Z

The Node.js streams team keeps on optimizing Writable and Readable streams with more double-digit performance gains. In this version streams maintainer Robert Nagy lead an effort to further optimize streams by reducing the number of checks performed, using bitmaps and scheduling callbacks in a more clever way.

^ This is assuming the other streams changes land in the next version too

nodejs-github-bot · 2023-10-04T10:13:08Z

Landed in 95b8f5d

RafaelGSS · 2023-10-16T12:55:03Z

The Node.js streams team keeps on optimizing Writable and Readable streams with more double-digit performance gains. In this version streams maintainer Robert Nagy lead an effort to further optimize streams by reducing the number of checks performed, using bitmaps and scheduling callbacks in a more clever way.

^ This is assuming the other streams changes land in the next version too

@benjamingr which other streams changes?

ronag requested review from benjamingr and mcollina October 2, 2023 10:06

ronag added the stream Issues and PRs related to the stream subsystem. label Oct 2, 2023

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

ronag added a commit to nxtedition/node that referenced this pull request Oct 2, 2023

perf: optimize Writable

42a0fba

PR-URL: nodejs#50012

ronag force-pushed the writable-opt branch from a15545a to 42a0fba Compare October 2, 2023 10:06

mcollina approved these changes Oct 2, 2023

View reviewed changes

benjamingr reviewed Oct 2, 2023

View reviewed changes

lib/internal/streams/writable.js Outdated Show resolved Hide resolved

ronag added a commit to nxtedition/node that referenced this pull request Oct 2, 2023

perf: optimize Writable

c11ee72

PR-URL: nodejs#50012

ronag force-pushed the writable-opt branch from 42a0fba to c11ee72 Compare October 2, 2023 10:15

benjamingr approved these changes Oct 2, 2023

View reviewed changes

benjamingr reviewed Oct 2, 2023

View reviewed changes

ronag force-pushed the writable-opt branch 2 times, most recently from 1dfbfb5 to dc1debf Compare October 2, 2023 10:22

anonrig approved these changes Oct 2, 2023

View reviewed changes

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 Oct 2, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2023

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2023

ronag force-pushed the writable-opt branch from dc1debf to 33635e7 Compare October 2, 2023 12:21

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2023

stream: optimize Writable

7ac75c4

PR-URL: nodejs#50012

ronag force-pushed the writable-opt branch from 33635e7 to 7ac75c4 Compare October 2, 2023 12:56

github-actions bot mentioned this pull request Oct 4, 2023

CI Reliability 2023-10-04 nodejs/reliability#683

Open

14 tasks

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 4, 2023

nodejs-github-bot merged commit 95b8f5d into nodejs:main Oct 4, 2023

This was referenced Oct 5, 2023

CI Reliability 2023-10-05 nodejs/reliability#684

Open

CI Reliability 2023-10-06 nodejs/reliability#685

Open

RafaelGSS mentioned this pull request Oct 9, 2023

2023-10-17, Version 21.0.0 (Current) #49870

Merged

targos mentioned this pull request Nov 12, 2023

v20.10.0 release proposal #50682

Merged

juanarbol mentioned this pull request Dec 6, 2023

2023-12-06, N|Solid v5.0.1 Iron Release nodesource/nsolid#41

Merged

dead-claudia mentioned this pull request Feb 12, 2024

Bitwise operations are seemingly not minimized or optimized at all terser/terser#1343

Closed

Uh oh!

perf: optimize Writable #50012

perf: optimize Writable #50012

Uh oh!

Conversation

ronag commented Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 2, 2023

Uh oh!

ronag commented Oct 2, 2023

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

benjamingr Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

ronag Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

ronag Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

anonrig left a comment

Choose a reason for hiding this comment

Uh oh!

ronag commented Oct 2, 2023

Uh oh!

nodejs-github-bot commented Oct 2, 2023

Uh oh!

nodejs-github-bot commented Oct 2, 2023

Uh oh!

benjamingr commented Oct 3, 2023

Uh oh!

nodejs-github-bot commented Oct 4, 2023

Uh oh!

RafaelGSS commented Oct 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ronag commented Oct 2, 2023 •

edited

Loading