-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
stream: improve Readable#from
perf
#50359
Conversation
Review requested:
|
This comment was marked as outdated.
This comment was marked as outdated.
Not sure I'm convinced the extra complexity is worth it. Let's see what the benchmark says. |
@rluvaton when you start a benchmark ci in cases like this, add a filter e,g, |
|
I'm -0. There is a perf improvement but the maintainability cost is quite high. |
due to the removal of the async modifier I rerun the benchmark:
I'm on the fence as well... |
@ronag Updated the benchmarks... |
I'm tired so if I don't make sense let me know - In the array case can't we just set the stream's buffer to |
bdcda68
to
aa3d6c2
Compare
it would really improve the performance for sure but wouldn't it be dangerous as we need to make the stream reach the state where it would have data after init and everything |
Not really? |
Yeah that was my point in #50359 I think that would be way faster and probably semver-major |
Because it needs a semver major, I will create a new PR with the new code after that |
e5352d6
to
ae6dd74
Compare
Landed in 10d51e8 |
PR-URL: nodejs#50359 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: #50359 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: #50359 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Benchmarks
Benchmark URL