streams: use Array for Readable buffer #50341

ronag · 2023-10-23T15:04:07Z

This replaces the Readable stream linked list BufferList buffer with an Array and index.

23:49:48 streams/pipe-object-mode.js n=5000000                                                           ***     23.56 %       ±4.03% ±5.36% ±6.98%
23:49:48 streams/pipe.js n=5000000                                                                       ***     18.77 %       ±2.88% ±3.84% ±5.00%
23:49:48 streams/readable-async-iterator.js sync='no' n=100000                                                    0.86 %       ±3.80% ±5.06% ±6.59%
23:49:48 streams/readable-async-iterator.js sync='yes' n=100000                                          ***      7.33 %       ±3.76% ±5.01% ±6.52%
23:49:48 streams/readable-bigread.js n=1000                                                              ***      8.48 %       ±4.51% ±6.01% ±7.82%
23:49:48 streams/readable-bigunevenread.js n=1000                                                        ***     13.56 %       ±1.78% ±2.37% ±3.08%
23:49:48 streams/readable-boundaryread.js type='buffer' n=2000                                           ***     11.92 %       ±1.39% ±1.85% ±2.41%
23:49:48 streams/readable-boundaryread.js type='string' n=2000                                           ***      1.99 %       ±0.99% ±1.32% ±1.72%
23:49:48 streams/readable-from.js n=10000000                                                             ***     17.22 %       ±5.10% ±6.80% ±8.87%
23:49:48 streams/readable-readall.js n=5000                                                              ***      9.66 %       ±5.47% ±7.28% ±9.49%
23:49:48 streams/readable-unevenread.js n=1000                                                           ***      4.80 %       ±0.40% ±0.54% ±0.70%

nodejs-github-bot · 2023-10-23T15:04:13Z

Review requested:

@nodejs/streams

lib/internal/streams/readable.js

PR-URL: nodejs#50341

Notable changes: doc: * add MrJithil to collaborators (Jithil P Ponnan) #50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740 fs: * add stacktrace to fs/promises (翠 / green) #49849 lib: * (SEMVER-MINOR) add navigator.platform (Aras Abbasi) #50385 stream: * (SEMVER-MINOR) add support for `deflate-raw` format to webstreams compression (Damian Krzeminski) #50097 * use Array for Readable buffer (Robert Nagy) #50341 * optimize creation (Robert Nagy) #50337 test_runner: * adds built in lcov reporter (Phil Nash) #50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443 PR-URL: #50681

Notable changes: doc: * add MrJithil to collaborators (Jithil P Ponnan) #50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740 fs: * add stacktrace to fs/promises (翠 / green) #49849 lib: * (SEMVER-MINOR) add `--no-experimental-global-navigator` CLI flag (Antoine du Hamel) #50562 * (SEMVER-MINOR) add navigator.language & navigator.languages (Aras Abbasi) #50303 * (SEMVER-MINOR) add navigator.platform (Aras Abbasi) #50385 stream: * (SEMVER-MINOR) add support for `deflate-raw` format to webstreams compression (Damian Krzeminski) #50097 * use Array for Readable buffer (Robert Nagy) #50341 * optimize creation (Robert Nagy) #50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443 PR-URL: #50681

Notable changes: doc: * add MrJithil to collaborators (Jithil P Ponnan) nodejs#50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) nodejs#50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) nodejs#48740 fs: * add stacktrace to fs/promises (翠 / green) nodejs#49849 lib: * (SEMVER-MINOR) add `--no-experimental-global-navigator` CLI flag (Antoine du Hamel) nodejs#50562 * (SEMVER-MINOR) add navigator.language & navigator.languages (Aras Abbasi) nodejs#50303 * (SEMVER-MINOR) add navigator.platform (Aras Abbasi) nodejs#50385 stream: * (SEMVER-MINOR) add support for `deflate-raw` format to webstreams compression (Damian Krzeminski) nodejs#50097 * use Array for Readable buffer (Robert Nagy) nodejs#50341 * optimize creation (Robert Nagy) nodejs#50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) nodejs#50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) nodejs#48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) nodejs#50443 PR-URL: nodejs#50681

PR-URL: #50341 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) #50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) #50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) #49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) #48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) #50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) #48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) #48655 stream: * use Array for Readable buffer (Robert Nagy) #50341 * optimize creation (Robert Nagy) #50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443 PR-URL: TODO

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) #50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) #50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) #49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) #48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) #50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) #48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) #48655 stream: * use Array for Readable buffer (Robert Nagy) #50341 * optimize creation (Robert Nagy) #50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443 PR-URL: #51124

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) #50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) #50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) #49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) #48655 * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) #48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) #50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) #48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) #48655 stream: * use Array for Readable buffer (Robert Nagy) #50341 * optimize creation (Robert Nagy) #50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443 PR-URL: #51124

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) nodejs#50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) nodejs#50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) nodejs#50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) nodejs#48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) nodejs#49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) nodejs#48655 * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) nodejs#48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) nodejs#50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) nodejs#48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) nodejs#48655 stream: * use Array for Readable buffer (Robert Nagy) nodejs#50341 * optimize creation (Robert Nagy) nodejs#50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) nodejs#50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) nodejs#48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) nodejs#50443 PR-URL: nodejs#51124

nadavzipo · 2024-01-14T12:45:00Z

@ronag I think this change caused breaking changes in the streams api.
Please see the following related issue - googleapis/nodejs-storage#2391

ronag · 2024-01-14T13:21:19Z

@ronag I think this change caused breaking changes in the streams api.

Please see the following related issue - googleapis/nodejs-storage#2391

The stream-shift module is using private properties. So while this change does break it, it's using stuff it shouldn't be using.

I'll give it some thought whether we can add some compat.

I would recommend either fixing g stream-shift or stop using it.

ronag · 2024-01-14T13:22:02Z

Even better, stop using duplexify package. We have correspond functionality in node core.

nadavzipo · 2024-01-14T13:40:32Z

@ronag thanks for taking a look.
I will need to revert to node 20.10.0 to solve this until googleapis/nodejs-storage will address this.

Node removed buffer_list object and uses a raw array now to store readable buffers: nodejs/node#50341

Notable changes: crypto: * update root certificates to NSS 3.95 (Node.js GitHub Bot) nodejs#50805 doc: * add MrJithil to collaborators (Jithil P Ponnan) nodejs#50666 * add Ethan-Arrowood as a collaborator (Ethan Arrowood) nodejs#50393 esm: * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) nodejs#48740 fs: * add c++ fast path for writeFileSync utf8 (CanadaHonk) nodejs#49884 module: * (SEMVER-MINOR) remove useCustomLoadersIfPresent flag (Chengzhong Wu) nodejs#48655 * (SEMVER-MINOR) bootstrap module loaders in shadow realm (Chengzhong Wu) nodejs#48655 src: * (SEMVER-MINOR) add `--disable-warning` option (Ethan Arrowood) nodejs#50661 * (SEMVER-MINOR) create per isolate proxy env template (Chengzhong Wu) nodejs#48655 * (SEMVER-MINOR) make process binding data weak (Chengzhong Wu) nodejs#48655 stream: * use Array for Readable buffer (Robert Nagy) nodejs#50341 * optimize creation (Robert Nagy) nodejs#50337 test_runner: * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) nodejs#50018 * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) nodejs#48638 test_runner, cli: * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) nodejs#50443 PR-URL: nodejs#51124

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 23, 2023

ronag force-pushed the readable-array-buffer branch 2 times, most recently from 38a496c to 19d2341 Compare October 23, 2023 15:06

anonrig reviewed Oct 23, 2023

View reviewed changes

lib/internal/streams/readable.js Outdated Show resolved Hide resolved

lib/internal/streams/readable.js Show resolved Hide resolved

ronag force-pushed the readable-array-buffer branch 4 times, most recently from ca3438d to b31ef38 Compare October 23, 2023 18:58

This comment was marked as outdated.

Sign in to view

ronag force-pushed the readable-array-buffer branch 2 times, most recently from 2944cda to 03d9416 Compare October 23, 2023 19:15

This comment was marked as outdated.

Sign in to view

ronag force-pushed the readable-array-buffer branch 3 times, most recently from 492bb00 to 1bffece Compare October 23, 2023 19:30

This comment was marked as outdated.

Sign in to view

ronag force-pushed the readable-array-buffer branch 2 times, most recently from 30deb62 to beb5c0e Compare October 23, 2023 19:55

ronag requested a review from mcollina October 23, 2023 20:05

ronag force-pushed the readable-array-buffer branch from beb5c0e to d302834 Compare October 23, 2023 20:05

ronag requested a review from benjamingr October 23, 2023 20:05

ronag marked this pull request as ready for review October 23, 2023 20:05

ronag added a commit to nxtedition/node that referenced this pull request Oct 23, 2023

stream: use Array for Readable buffer

b4577b0

PR-URL: nodejs#50341

ronag force-pushed the readable-array-buffer branch from d302834 to b4577b0 Compare October 23, 2023 20:06

ronag added a commit to nxtedition/node that referenced this pull request Oct 23, 2023

stream: use Array for Readable buffer

442787b

PR-URL: nodejs#50341

ronag force-pushed the readable-array-buffer branch from b4577b0 to 442787b Compare October 23, 2023 20:07

ronag added a commit to nxtedition/node that referenced this pull request Oct 23, 2023

stream: use Array for Readable buffer

59f941e

PR-URL: nodejs#50341

ronag force-pushed the readable-array-buffer branch from 442787b to 59f941e Compare October 23, 2023 20:16

ronag added a commit to nxtedition/node that referenced this pull request Oct 23, 2023

stream: use Array for Readable buffer

aadd445

PR-URL: nodejs#50341

targos mentioned this pull request Nov 12, 2023

v21.2.0 release proposal #50681

Merged

jamie-phlo mentioned this pull request Nov 20, 2023

Node 21.2 breaking changes mafintosh/stream-shift#6

Closed

UlisesGascon mentioned this pull request Dec 12, 2023

v20.11.0 proposal #51124

Merged

nadavzipo mentioned this pull request Jan 14, 2024

Downloading file fails when running on Node version 20.11.0 'Iron' (LTS) googleapis/nodejs-storage#2391

Closed

fbeaudoincoveo mentioned this pull request Jan 15, 2024

fix(deps): update all dependencies j:kit-282 coveo/ui-kit#3522

Merged

1 task

Chocobozzz added a commit to Chocobozzz/net-ipc that referenced this pull request Jan 19, 2024

Fix compatibility with node 20.11

237d678

Node removed buffer_list object and uses a raw array now to store readable buffers: nodejs/node#50341

Chocobozzz mentioned this pull request Jan 19, 2024

Fix compatibility with node 20.11 timotejroiko/net-ipc#7

Merged

HeverFarber mentioned this pull request Mar 12, 2024

Uncaught Exception: "Cannot read properties of null (reading length)" when use encrypt function on lambda 20.x aws/aws-encryption-sdk-javascript#1374

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

streams: use Array for Readable buffer #50341

streams: use Array for Readable buffer #50341

ronag commented Oct 23, 2023 •

edited

nodejs-github-bot commented Oct 23, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nadavzipo commented Jan 14, 2024

ronag commented Jan 14, 2024

ronag commented Jan 14, 2024 •

edited

nadavzipo commented Jan 14, 2024

streams: use Array for Readable buffer #50341

streams: use Array for Readable buffer #50341

Conversation

ronag commented Oct 23, 2023 • edited

nodejs-github-bot commented Oct 23, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nadavzipo commented Jan 14, 2024

ronag commented Jan 14, 2024

ronag commented Jan 14, 2024 • edited

nadavzipo commented Jan 14, 2024

ronag commented Oct 23, 2023 •

edited

ronag commented Jan 14, 2024 •

edited