-
Notifications
You must be signed in to change notification settings - Fork 28.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
benchmark: include webstreams benchmark #45876
benchmark: include webstreams benchmark #45876
Conversation
benchmark/webstreams/creation.js
Outdated
function main({ n, kind }) { | ||
switch (kind) { | ||
case 'ReadableStream': | ||
new ReadableStream({}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to assign the value to a binding outside the loop and do something about it in the end, otherwise the optimizing compiler might be smart enough to perform a dead code elimination and make the benchmark pointless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A typical technique for micro-benchmarking JS would be to pre-fill an array before the measurement starts, and you assign the values into that array during the benchmark (but do not grow or shrink it), then do something about the array in the end, in general the optimizing compiler won't attempt to eliminate the initializations in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a way to find dead code elimination? I tried some time ago with some V8 flags but it didn't work.
I've updated the code, does it solve? The benchmark results are the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a way to find dead code elimination? I tried some time ago with some V8 flags but it didn't work.
You can try --trace-turbo*
flags with --turbo-filter
to filter out specific functions you want to take a look at (and use v8/tools/turbolizer to help looking at the output). But I'd say to benchmark Node.js's JS code the best solution is to just avoid micro-benchmarks and test a comprehensive combination of operations with varied input ;) e.g. the pipe-to benchmarks are less likely to be influenced by that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. I've created the creation.js to compare with streams/creation.js
. This also shows how much memory it consumes (potential memory leak at some point IMHO - if you use the same n
as streams
it will get OOM).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test file for testing the benchmark? Like: test/benchmark/test-webstreams.js
e880275
to
846b57b
Compare
846b57b
to
931c086
Compare
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
931c086
to
0432fd4
Compare
Landed in 70d269d |
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: #45876 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: #45876 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: #45876 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: #45876 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: #45876 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
This PR includes some basic benchmarks for Readable and Writable web streams. @jasnell do you think we should measure something in particular?
It's on my radar to improve the performance of web streams, but I'm not sure how fair is to compare it with regular streams.