Skip to content

Conversation

@rluvaton
Copy link
Member

@rluvatonrluvaton commented Sep 8, 2023

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-botnodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Sep 8, 2023
@rluvatonrluvatonforce-pushed the improve-readable-stream-perf-by-moving-to-list branch from 8465d8b to 4a982c6CompareSeptember 8, 2023 12:15
@rluvatonrluvaton 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
@rluvatonrluvaton changed the title stream: replace queue with buffer list for performance improvementstream: replace queue and readRequests with buffer list for performance improvementSep 8, 2023
Copy link
Member

@ronagronag left a comment

Choose a reason for hiding this comment

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

I think using FixedQueue would have better performance?

Comment on lines +3114 to +3116
// TODO - may be able to change this to next next next
while(reader[kState].readRequests.length>0){
// TODO - why this is in the while loop?
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

remove todos

@rluvaton
Copy link
MemberAuthor

rluvaton commented Sep 8, 2023

@rluvatonrluvatonforce-pushed the improve-readable-stream-perf-by-moving-to-list branch from 5943045 to 4e39d86CompareSeptember 8, 2023 13:59
@rluvaton
Copy link
MemberAuthor

rluvaton commented Sep 8, 2023

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/

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...
assert(controller[kState].queueTotalSize!==undefined);
assert(controller[kState].queue.length);
returncontroller[kState].queue[0].value;
debugger;
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

remove

@rluvaton
Copy link
MemberAuthor

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

@rluvatonrluvaton closed this Sep 8, 2023
@rluvatonrluvaton deleted the improve-readable-stream-perf-by-moving-to-list branch September 8, 2023 15:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-benchmark-ciPR that need a benchmark CI run.needs-ciPRs that need a full CI run.performanceIssues and PRs related to the performance of Node.js.web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@rluvaton@ronag@nodejs-github-bot