stream: avoid possible slow paths w UInt8Array#13956
stream: avoid possible slow paths w UInt8Array#13956mcollina wants to merge 1 commit intonodejs:masterfrom
Conversation
|
Have you ran benchmarks to check the effects of this? I would think non-object mode streams are more common than object mode streams and this could negatively non-object mode streams because the |
This is mostly related to readable-stream, after the whole transpiling process. In Node core, things should be equivalent. I discovered some of those while hunting for nodejs/readable-stream#304.
All conditions are at |
lib/_stream_readable.js
Outdated
There was a problem hiding this comment.
You are right about this, I will need to check if this does not negatively effect benchmarks.
lib/_stream_writable.js
Outdated
There was a problem hiding this comment.
In my mind this is surely going to be be faster when !state.objectMode.
A chunk validity checks verifie if a chunk is a UInt8Array. We should defer it as it might be very expensive in older Node.js platforms.
4f6fa5c to
e66fe47
Compare
|
@mscdex I've updated the problematic part, and just left with the change on |
|
LGTM now. |
|
Landed as 0279602 |
A chunk validity checks verifie if a chunk is a UInt8Array. We should defer it as it might be very expensive in older Node.js platforms. PR-URL: #13956 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
A chunk validity checks verifie if a chunk is a UInt8Array. We should defer it as it might be very expensive in older Node.js platforms. PR-URL: nodejs#13956 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
A chunk validity checks verifie if a chunk is a UInt8Array. We should defer it as it might be very expensive in older Node.js platforms. PR-URL: #13956 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
A chunk validity checks verifie if a chunk is a UInt8Array. We should defer it as it might be very expensive in older Node.js platforms. PR-URL: #13956 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
|
@mcollina I'm pretty sure this doesn't apply to v6.x the check is way different. var isBuf = (chunk instanceof Buffer);Can you add |
|
@MylesBorins this should not be backported. |
A chunk validity checks verifie if a chunk is a UInt8Array.
We should defer it as it might be very expensive in older Node.js
platforms.
It avoid the UInt8Array checks if the streams are in
objectMode: true.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
stream