FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

stream: replace queue and readRequests with buffer list for performance improvement by rluvaton · Pull Request #49554 · nodejs/node · GitHub

/ node Public

stream: replace queue and readRequests with buffer list for performance improvement - #49554

Closed
rluvaton wants to merge 3 commits into
nodejs:mainfrom
rluvaton:improve-readable-stream-perf-by-moving-to-list
ConversationCommits (3)ChecksFiles changed
Closed

stream: replace queue and readRequests with buffer list for performance improvement#49554
rluvaton wants to merge 3 commits into
nodejs:mainfrom
rluvaton:improve-readable-stream-perf-by-moving-to-list

Conversation

rluvaton commented Sep 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

webstream possible improvement:

BufferList implementation

00:24:29.343                                                                        confidence improvement accuracy (*)   (**)   (***)
00:24:29.343 webstreams/creation.js kind='ReadableStream' n=50000                                   1.04 %       ±3.94% ±5.25%  ±6.83%
00:24:29.343 webstreams/creation.js kind='TransformStream' n=50000                                  0.06 %       ±2.59% ±3.45%  ±4.52%
00:24:29.343 webstreams/creation.js kind='WritableStream' n=50000                                   0.19 %       ±4.60% ±6.12%  ±7.98%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=1024 n=100000                 1.58 %       ±4.24% ±5.64%  ±7.34%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=2048 n=100000                 1.13 %       ±4.28% ±5.69%  ±7.41%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=4096 n=100000                 1.50 %       ±4.50% ±5.99%  ±7.79%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=512 n=100000                  1.04 %       ±5.26% ±7.00%  ±9.12%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=1024 n=100000                 2.24 %       ±5.31% ±7.07%  ±9.20%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=2048 n=100000                 3.03 %       ±4.62% ±6.15%  ±8.00%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=4096 n=100000                 2.97 %       ±5.12% ±6.81%  ±8.86%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=512 n=100000                  3.92 %       ±4.65% ±6.19%  ±8.06%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=1024 n=100000                -0.04 %       ±4.14% ±5.50%  ±7.16%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=2048 n=100000                -1.05 %       ±4.87% ±6.48%  ±8.43%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=4096 n=100000                 2.22 %       ±4.57% ±6.07%  ±7.91%
00:24:29.343 webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=512 n=100000                 -2.14 %       ±4.71% ±6.27%  ±8.16%
00:24:29.344 webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=1024 n=100000                  1.56 %       ±4.40% ±5.85%  ±7.62%
00:24:29.344 webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=2048 n=100000                  0.54 %       ±4.56% ±6.06%  ±7.89%
00:24:29.344 webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=4096 n=100000                  3.16 %       ±4.44% ±5.91%  ±7.70%
00:24:29.344 webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=512 n=100000                   0.90 %       ±4.70% ±6.26%  ±8.16%
00:24:29.344 webstreams/readable-async-iterator.js n=100000                                         0.03 %       ±7.40% ±9.84% ±12.81%
00:24:29.344 
00:24:29.344 Be aware that when doing many comparisons the risk of a false-positive
00:24:29.344 result increases. In this case, there are 20 comparisons, you can thus
00:24:29.344 expect the following amount of false-positive results:
00:24:29.344   1.00 false positives, when considering a   5% risk acceptance (*, **, ***),
00:24:29.344   0.20 false positives, when considering a   1% risk acceptance (**, ***),
00:24:29.344   0.02 false positives, when considering a 0.1% risk acceptance (***)

From my local tests:

Before

❯ ./node ./benchmark/webstreams/pipe-to.js
webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=512 n=5000000: 1,073,259.5235043082
webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=512 n=5000000: 1,112,582.388083188
webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=512 n=5000000: 1,130,721.9334553012
webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=512 n=5000000: 1,137,875.2107408328
webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=1024 n=5000000: 1,145,415.4228369598
webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=1024 n=5000000: 1,119,071.6166112064
webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=1024 n=5000000: 1,112,401.620591177
webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=1024 n=5000000: 1,122,177.222643778
webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=2048 n=5000000: 1,154,120.0021085772
webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=2048 n=5000000: 1,137,931.8271974607
webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=2048 n=5000000: 1,150,085.2975136968
webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=2048 n=5000000: 1,120,964.4298001872
webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=4096 n=5000000: 1,126,744.873505527
webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=4096 n=5000000: 1,148,843.5697545216
webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=4096 n=5000000: 1,133,865.2778680783
webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=4096 n=5000000: 1,132,648.4636818648

After

❯ ./node ./benchmark/webstreams/pipe-to.js
webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=512 n=5000000: 1,191,920.5427624737
webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=512 n=5000000: 1,199,980.044139869
webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=512 n=5000000: 1,196,830.8757150145
webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=512 n=5000000: 1,209,921.9163751516
webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=1024 n=5000000: 1,199,785.7782492938
webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=1024 n=5000000: 1,191,548.2411812833
webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=1024 n=5000000: 1,205,993.3892632995
webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=1024 n=5000000: 1,191,874.810445273
webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=2048 n=5000000: 1,209,462.7154360572
webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=2048 n=5000000: 1,187,768.408534668
webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=2048 n=5000000: 1,195,187.8509158778
webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=2048 n=5000000: 1,201,572.702366821
webstreams/pipe-to.js highWaterMarkW=512 highWaterMarkR=4096 n=5000000: 1,197,150.4825638772
webstreams/pipe-to.js highWaterMarkW=1024 highWaterMarkR=4096 n=5000000: 1,179,266.3281946357
webstreams/pipe-to.js highWaterMarkW=2048 highWaterMarkR=4096 n=5000000: 1,205,445.1767767125
webstreams/pipe-to.js highWaterMarkW=4096 highWaterMarkR=4096 n=5000000: 1,235,058.8880696953

nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Sep 8, 2023
rluvaton force-pushed the improve-readable-stream-perf-by-moving-to-list branch from 8465d8b to 4a982c6 Compare September 8, 2023 12:15
rluvaton added performance Issues and PRs related to the performance of Node.js. needs-benchmark-ci PR that need a benchmark CI run. labels Sep 8, 2023
rluvaton changed the title stream: replace queue with buffer list for performance improvement stream: replace queue and readRequests with buffer list for performance improvement Sep 8, 2023
ronag reviewed Sep 8, 2023
View reviewed changes

ronag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think using FixedQueue would have better performance?

rluvaton commented Sep 8, 2023
View reviewed changes
Comment thread lib/internal/webstreams/readablestream.js Outdated
Comment on lines 3114 to 3116

rluvaton Sep 8, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

remove todos

rluvaton commented Sep 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

Benchmark CI for BufferList implementation

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

rluvaton force-pushed the improve-readable-stream-perf-by-moving-to-list branch from 5943045 to 4e39d86 Compare September 8, 2023 13:59

rluvaton commented Sep 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

the pipe-to benchmark takes too long, it's currently running for an hour.

Each iteration (single combination) takes 9 seconds. there are 16 possible combinations.
so 9 seconds * 16 combination * 30 runs * 2 versions = 2 hours and 24 minutes...

new Benchmark CI that has reduced the number of chunks from 5M to 100K for BufferList implementation
https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1370/

reduce number of chunks in pipe benchmark
04f02a1 
Each iteration (single combination) takes 9 seconds. there are 16 possible combinations.
so 9 seconds * 16 combination * 30 runs * 2 versions = 2 hours and 24 minutes...
rluvaton commented Sep 8, 2023
View reviewed changes
Comment thread lib/internal/webstreams/util.js
assert(controller[kState].queueTotalSize !== undefined);
assert(controller[kState].queue.length);
return controller[kState].queue[0].value;
debugger;

rluvaton Sep 8, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

remove

rluvaton commented Sep 8, 2023

Copy link
Copy Markdown
Member Author

I think using FixedQueue would have better performance?

Could not make it work, the tests failed for some reason...

current BufferList does not show any improvement so closing this

rluvaton closed this Sep 8, 2023
rluvaton deleted the improve-readable-stream-perf-by-moving-to-list branch September 8, 2023 15:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ronag ronag left review comments

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. web streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL