Skip to content

Revert stream primordials#37168

Closed
aduh95 wants to merge 2 commits intonodejs:masterfrom
aduh95:revert-stream-primordials
Closed

Revert stream primordials#37168
aduh95 wants to merge 2 commits intonodejs:masterfrom
aduh95:revert-stream-primordials

Conversation

@aduh95
Copy link
Copy Markdown
Contributor

@aduh95 aduh95 commented Feb 1, 2021

Revert PR to run benchmarks.

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Feb 1, 2021
@aduh95 aduh95 requested a review from mcollina February 1, 2021 14:41
@mcollina
Copy link
Copy Markdown
Member

mcollina commented Feb 1, 2021

Let's just run the benchmark CI on this. We can close if not needed. Thanks

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Feb 1, 2021

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Feb 1, 2021

The perf gain is there for Writable (minus is good):

17:26:39 streams/creation.jskind='duplex' n=50000000                                                             -0.31 %       ±2.33%  ±3.10%  ±4.04%
17:26:39 streams/creation.jskind='readable' n=50000000                                                            0.50 %       ±1.69%  ±2.24%  ±2.92%
17:26:39 streams/creation.jskind='transform' n=50000000                                                           0.90 %       ±3.56%  ±4.75%  ±6.19%
17:26:39 streams/creation.jskind='writable' n=50000000                                                   ***     -5.69 %       ±2.12%  ±2.82%  ±3.68%
17:26:39 streams/pipe.jsn=5000000                                                                                 1.43 %       ±2.77%  ±3.68%  ±4.79%
17:26:39 streams/pipe-object-mode.jsn=5000000                                                                    -1.45 %       ±3.54%  ±4.71%  ±6.12%
17:26:39 streams/readable-async-iterator.jssync='no' n=100000                                                     1.44 %       ±2.85%  ±3.79%  ±4.94%
17:26:39 streams/readable-async-iterator.jssync='yes' n=100000                                                    2.84 %       ±5.29%  ±7.03%  ±9.16%
17:26:39 streams/readable-bigread.jsn=1000                                                                       -3.32 %       ±4.55%  ±6.06%  ±7.90%
17:26:39 streams/readable-bigunevenread.jsn=1000                                                                 -0.55 %       ±7.71% ±10.26% ±13.36%
17:26:39 streams/readable-boundaryread.jstype='buffer' n=2000                                                     1.74 %       ±3.59%  ±4.81%  ±6.32%
17:26:39 streams/readable-boundaryread.jstype='string' n=2000                                                     1.67 %       ±1.82%  ±2.44%  ±3.23%
17:26:39 streams/readable-readall.jsn=5000                                                                        0.81 %       ±2.56%  ±3.40%  ±4.43%
17:26:39 streams/readable-unevenread.jsn=1000                                                                    -1.52 %       ±2.89%  ±3.85%  ±5.01%
17:26:39 streams/writable-manywrites.jslen=1024 callback='no' writev='no' sync='no' n=2000000                    -1.17 %       ±1.33%  ±1.77%  ±2.31%
17:26:39 streams/writable-manywrites.jslen=1024 callback='no' writev='no' sync='yes' n=2000000                   -1.10 %       ±3.89%  ±5.18%  ±6.74%
17:26:39 streams/writable-manywrites.jslen=1024 callback='no' writev='yes' sync='no' n=2000000                   -1.35 %       ±2.41%  ±3.21%  ±4.18%
17:26:39 streams/writable-manywrites.jslen=1024 callback='no' writev='yes' sync='yes' n=2000000                  -1.07 %       ±3.32%  ±4.42%  ±5.75%
17:26:39 streams/writable-manywrites.jslen=1024 callback='yes' writev='no' sync='no' n=2000000                   -0.54 %       ±1.04%  ±1.38%  ±1.81%
17:26:39 streams/writable-manywrites.jslen=1024 callback='yes' writev='no' sync='yes' n=2000000                   1.03 %       ±3.30%  ±4.41%  ±5.76%
17:26:39 streams/writable-manywrites.jslen=1024 callback='yes' writev='yes' sync='no' n=2000000                   0.41 %       ±2.56%  ±3.40%  ±4.44%
17:26:39 streams/writable-manywrites.jslen=1024 callback='yes' writev='yes' sync='yes' n=2000000                 -0.84 %       ±3.83%  ±5.10%  ±6.65%
17:26:39 streams/writable-manywrites.jslen=32768 callback='no' writev='no' sync='no' n=2000000                    0.77 %       ±1.49%  ±1.99%  ±2.59%
17:26:39 streams/writable-manywrites.jslen=32768 callback='no' writev='no' sync='yes' n=2000000                  -0.30 %       ±2.33%  ±3.10%  ±4.04%
17:26:39 streams/writable-manywrites.jslen=32768 callback='no' writev='yes' sync='no' n=2000000                  -1.40 %       ±4.64%  ±6.22%  ±8.18%
17:26:39 streams/writable-manywrites.jslen=32768 callback='no' writev='yes' sync='yes' n=2000000                  0.76 %       ±1.60%  ±2.13%  ±2.78%
17:26:39 streams/writable-manywrites.jslen=32768 callback='yes' writev='no' sync='no' n=2000000                   0.52 %       ±1.38%  ±1.85%  ±2.41%
17:26:39 streams/writable-manywrites.jslen=32768 callback='yes' writev='no' sync='yes' n=2000000                 -0.04 %       ±1.68%  ±2.24%  ±2.91%
17:26:39 streams/writable-manywrites.jslen=32768 callback='yes' writev='yes' sync='no' n=2000000                 -1.21 %       ±1.55%  ±2.06%  ±2.69%
17:26:39 streams/writable-manywrites.jslen=32768 callback='yes' writev='yes' sync='yes' n=2000000                -0.91 %       ±2.08%  ±2.77%  ±3.60%

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Feb 1, 2021

I want to wait on http as well.

@aduh95
Copy link
Copy Markdown
Contributor Author

aduh95 commented Feb 2, 2021

@mcollina It seems the CI is hanging (it's been 16 hours since a log was emitted to the console output). It might be worth splitting it into several jobs.

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Feb 2, 2021

@aduh95
Copy link
Copy Markdown
Contributor Author

aduh95 commented Feb 2, 2021

CI is stuck again (last log was 1 hour ago)

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Feb 5, 2021

52 hours later, the benches are still running on my server. Anyway, the preliminary results are good (no regression) closing this.

@mcollina mcollina closed this Feb 5, 2021
@aduh95 aduh95 deleted the revert-stream-primordials branch February 5, 2021 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants