Skip to content

stream: improve WebStreams creation performance - #49089

Merged
nodejs-github-bot merged 9 commits into
nodejs:mainfrom
rluvaton:improve-webstream-perf
Aug 13, 2023
Merged

nodejs-github-bot merged 9 commits into
nodejs:mainfrom
rluvaton:improve-webstream-perf

Conversation

@rluvaton

@rluvaton rluvaton commented Aug 9, 2023 •

Copy link
Copy Markdown
Member

From my local tests, this improves the benchmark/webstreams/creation.js by 2 to 3 times

could someone please run the benchmark in the CI

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams Issues and PRs related to the Web Streams API. labels Aug 9, 2023
@Xstoudi

Xstoudi commented Aug 10, 2023

Copy link
Copy Markdown
Contributor

Ubuntu on WSL2

Without the change:

webstreams/creation.js kind="ReadableStream" n=50000: 245,842.89135667187
webstreams/creation.js kind="TransformStream" n=50000: 64,412.45274303109
webstreams/creation.js kind="WritableStream" n=50000: 235,646.29630427333

With change:

webstreams/creation.js kind="ReadableStream" n=50000: 796,678.8307839505
webstreams/creation.js kind="TransformStream" n=50000: 137,357.95549408294
webstreams/creation.js kind="WritableStream" n=50000: 503,433.1675202359

If values are rates and not times, your change seems to improve it on my end too!

@debadree25

Copy link
Copy Markdown
Contributor

Seems to be failing a lot of WPTs too

Comment thread lib/internal/webstreams/readablestream.js Outdated
@rluvaton rluvaton changed the title stream: improve WebStreams performance stream: improve WebStreams creation performance Aug 10, 2023
@rluvaton

rluvaton commented Aug 10, 2023 •

Copy link
Copy Markdown
Member Author

@mcollina

Copy link
Copy Markdown
Member

cc @jasnell

@rluvaton
rluvaton force-pushed the improve-webstream-perf branch from 4bb72eb to ede6c54 Compare August 10, 2023 17:11
@rluvaton

Copy link
Copy Markdown
Member Author

fixed the tests

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 11, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 11, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonrig anonrig added needs-benchmark-ci PRs that need a benchmark CI run. performance Issues and PRs related to the performance of Node.js. labels Aug 11, 2023
@anonrig

anonrig commented Aug 11, 2023

Copy link
Copy Markdown
Member

@rluvaton

rluvaton commented Aug 11, 2023 •

Copy link
Copy Markdown
Member Author

benchmark output:

                                                       confidence improvement accuracy (*)   (**)   (***)
webstreams/creation.js kind='ReadableStream' n=50000         ***    144.39 %       ±6.00% ±8.00% ±10.45%
webstreams/creation.js kind='TransformStream' n=50000        ***     98.61 %       ±3.81% ±5.11%  ±6.72%
webstreams/creation.js kind='WritableStream' n=50000         ***     85.25 %       ±6.30% ±8.44% ±11.08%
 
Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 3 comparisons, you can thus
expect the following amount of false-positive results:
  0.15 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.03 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

@mcollina mcollina 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.

lgtm

anonrig
anonrig approved these changes