Skip to content

Conversation

@aduh95
Copy link
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
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
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
Member

mcollina commented Feb 1, 2021

I want to wait on http as well.

@aduh95
Copy link
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
Member

mcollina commented Feb 2, 2021

@aduh95
Copy link
Contributor Author

aduh95 commented Feb 2, 2021

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

@mcollina
Copy link
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