stream: do not chunk strings and Buffer in Readable.from. #30912
Conversation
|
|
||
| const { | ||
| ERR_INVALID_ARG_TYPE | ||
| } = require('internal/errors').codes; | ||
|
|
||
| function from(Readable, iterable, opts) { | ||
| let iterator; | ||
| if (typeof iterable === 'string' || iterable instanceof Buffer) { | ||
| return new Readable({ | ||
| objectMode: true, |
There was a problem hiding this comment.
nit, Not sure of the purpose here, but wouldn't objectMode: false make more sense here as a default? Or is that breaking?
There was a problem hiding this comment.
I would keep it consistent with the rest. Also, it would change the encoding, so possibly it's not a good idea.
|
This does change behaviour when a buffer is passed, is that intentional? |
Yes, instead of emitting one byte at a time, we will push through the full chunk. This will remove a lot of overhead when using this API. |
|
@mcollina yes, but instead of emitting numbers, it now emits a Buffer |
|
Ouch. I would consider this a bug :/. |
|
|
||
| const { | ||
| ERR_INVALID_ARG_TYPE | ||
| } = require('internal/errors').codes; | ||
|
|
||
| function from(Readable, iterable, opts) { | ||
| let iterator; | ||
| if (typeof iterable === 'string' || iterable instanceof Buffer) { |
There was a problem hiding this comment.
Should we check for Stream._isUint8Array as well?
There was a problem hiding this comment.
That would only be okay if we turn off object mode, I think
I would agree, but could you document that |
mcollina
force-pushed
the
readable-from-fast-strings
branch
from
378e505
to
019d0d0
Compare
mcollina
force-pushed
the
readable-from-fast-strings
branch
from
019d0d0
to
4dbdd43
Compare
This comment has been minimized.
This comment has been minimized.
|
Landed in f8018f2 |
PR-URL: #30912 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #30912 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #30912 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #30912 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
This PR makes
Readable.from()to not iterate over strings and buffers to avoid unnecessary overhead.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes