-
Notifications
You must be signed in to change notification settings - Fork 30.7k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
stream: improve webstream readable async iterator performance #49662
stream: improve webstream readable async iterator performance #49662
Conversation
Benchmark CI (currently 404 as it's in a queue): |
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.
lgtm
Co-authored-by: Benjamin Gruenbaum <inglor@gmail.com>
Commit Queue failed- Loading data for nodejs/node/pull/49662 ✔ Done loading data for nodejs/node/pull/49662 ----------------------------------- PR info ------------------------------------ Title stream: improve webstream readable async iterator performance (#49662) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch rluvaton:improve-readable-stream-async-iterator -> nodejs:main Labels performance, author ready, needs-ci, needs-benchmark-ci, web streams Commits 5 - stream: improve webstream readable async iterator performance - stream: rename, fix lint issues and cleanup - stream: add proto - stream: inline var - stream: remove `__proto__: null` due to performance hit Committers 2 - Raz Luvaton <16746759+rluvaton@users.noreply.github.com> - GitHub PR-URL: https://github.com/nodejs/node/pull/49662 Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/49662 Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 15 Sep 2023 11:32:53 GMT ✔ Approvals: 6 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/49662#pullrequestreview-1629143084 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/49662#pullrequestreview-1629556395 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/49662#pullrequestreview-1629701561 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/49662#pullrequestreview-1629983394 ✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/49662#pullrequestreview-1630072844 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/49662#pullrequestreview-1631104311 ✔ Last GitHub CI successful ℹ Last Benchmark CI on 2023-09-18T13:22:25Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1388/ ℹ Last Full PR CI on 2023-09-18T13:22:25Z: https://ci.nodejs.org/job/node-test-pull-request/54033/ - Querying data for job/node-test-pull-request/1388/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 49662 From https://github.com/nodejs/node * branch refs/pull/49662/merge -> FETCH_HEAD ✔ Fetched commits as b03a757619db..824abd622d65 -------------------------------------------------------------------------------- [main 9e5b3a7ed2] stream: improve webstream readable async iterator performance Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri Sep 15 14:32:31 2023 +0300 1 file changed, 46 insertions(+), 29 deletions(-) [main 2419b69e63] stream: rename, fix lint issues and cleanup Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri Sep 15 14:39:30 2023 +0300 1 file changed, 32 insertions(+), 33 deletions(-) [main bb9ba7cb76] stream: add proto Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun Sep 17 19:38:11 2023 +0300 1 file changed, 1 insertion(+) [main a31438954c] stream: inline var Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun Sep 17 22:58:17 2023 +0300 1 file changed, 1 insertion(+), 3 deletions(-) [main cf191602a9] stream: remove `__proto__: null` due to performance hit Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon Sep 18 10:20:13 2023 +0300 1 file changed, 1 insertion(+), 1 deletion(-) ✔ Patches applied There are 5 commits in the PR. Attempting autorebase. Rebasing (2/10)https://github.com/nodejs/node/actions/runs/6223523578 |
Landed in 5c39ee6 |
I believe this pull request deserves the |
The
notable-change
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. |
PR-URL: #49662 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Notable changes: doc: * deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49683 * promote fetch/webstreams from experimental to stable (Steven) #45684 * deprecate `util.toUSVString` (Yagiz Nizipli) #49725 * deprecate calling `promisify` on a function that returns a promise (Antoine du Hamel) #49647 esm: * set all hooks as release candidate (Geoffrey Booth) #49597 stream: * use bitmap in writable state (Raz Luvaton) #49834 * use bitmap in readable state (Benjamin Gruenbaum) #49745 * improve webstream readable async iterator performance (Raz Luvaton) #49662 PR-URL: TODO
Notable changes: doc: * deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49683 * promote fetch/webstreams from experimental to stable (Steven) #45684 * deprecate `util.toUSVString` (Yagiz Nizipli) #49725 * deprecate calling `promisify` on a function that returns a promise (Antoine du Hamel) #49647 esm: * set all hooks as release candidate (Geoffrey Booth) #49597 stream: * use bitmap in writable state (Raz Luvaton) #49834 * use bitmap in readable state (Benjamin Gruenbaum) #49745 * improve webstream readable async iterator performance (Raz Luvaton) #49662 PR-URL: #49917
Notable changes: doc: * deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49683 * deprecate `util.toUSVString` (Yagiz Nizipli) #49725 * deprecate calling `promisify` on a function that returns a promise (Antoine du Hamel) #49647 esm: * set all hooks as release candidate (Geoffrey Booth) #49597 src: * (SEMVER-MINOR) allow embedders to override NODE_MODULE_VERSION (Cheng Zhao) #49279 stream: * use bitmap in writable state (Raz Luvaton) #49834 * use bitmap in readable state (Benjamin Gruenbaum) #49745 * improve webstream readable async iterator performance (Raz Luvaton) #49662 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) #49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) #49614 PR-URL: TODO
Notable changes: deps: * add v8::Object::SetInternalFieldForNodeCore() (Joyee Cheung) #49874 doc: * deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49683 * deprecate `util.toUSVString` (Yagiz Nizipli) #49725 * deprecate calling `promisify` on a function that returns a promise (Antoine du Hamel) #49647 esm: * set all hooks as release candidate (Geoffrey Booth) #49597 module: * fix the leak in SourceTextModule and ContextifySript (Joyee Cheung) #48510 * fix leak of vm.SyntheticModule (Joyee Cheung) #48510 * use symbol in WeakMap to manage host defined options (Joyee Cheung) #48510 src: * (SEMVER-MINOR) allow embedders to override NODE_MODULE_VERSION (Cheng Zhao) #49279 stream: * use bitmap in writable state (Raz Luvaton) #49834 * use bitmap in readable state (Benjamin Gruenbaum) #49745 * improve webstream readable async iterator performance (Raz Luvaton) #49662 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) #49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) #49614 PR-URL: #49932
Notable changes: deps: * add v8::Object::SetInternalFieldForNodeCore() (Joyee Cheung) #49874 doc: * deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49683 * deprecate `util.toUSVString` (Yagiz Nizipli) #49725 * deprecate calling `promisify` on a function that returns a promise (Antoine du Hamel) #49647 esm: * set all hooks as release candidate (Geoffrey Booth) #49597 module: * fix the leak in SourceTextModule and ContextifySript (Joyee Cheung) #48510 * fix leak of vm.SyntheticModule (Joyee Cheung) #48510 * use symbol in WeakMap to manage host defined options (Joyee Cheung) #48510 src: * (SEMVER-MINOR) allow embedders to override NODE_MODULE_VERSION (Cheng Zhao) #49279 stream: * use bitmap in writable state (Raz Luvaton) #49834 * use bitmap in readable state (Benjamin Gruenbaum) #49745 * improve webstream readable async iterator performance (Raz Luvaton) #49662 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) #49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) #49614 PR-URL: #49932
Notable changes: deps: * add v8::Object::SetInternalFieldForNodeCore() (Joyee Cheung) #49874 doc: * deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49683 * deprecate `util.toUSVString` (Yagiz Nizipli) #49725 * deprecate calling `promisify` on a function that returns a promise (Antoine du Hamel) #49647 esm: * set all hooks as release candidate (Geoffrey Booth) #49597 module: * fix the leak in SourceTextModule and ContextifySript (Joyee Cheung) #48510 * fix leak of vm.SyntheticModule (Joyee Cheung) #48510 * use symbol in WeakMap to manage host defined options (Joyee Cheung) #48510 src: * (SEMVER-MINOR) allow embedders to override NODE_MODULE_VERSION (Cheng Zhao) #49279 stream: * use bitmap in writable state (Raz Luvaton) #49834 * use bitmap in readable state (Benjamin Gruenbaum) #49745 * improve webstream readable async iterator performance (Raz Luvaton) #49662 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) #49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) #49614 PR-URL: #49932
PR-URL: nodejs#49662 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Notable changes: deps: * add v8::Object::SetInternalFieldForNodeCore() (Joyee Cheung) nodejs#49874 doc: * deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) nodejs#49683 * deprecate `util.toUSVString` (Yagiz Nizipli) nodejs#49725 * deprecate calling `promisify` on a function that returns a promise (Antoine du Hamel) nodejs#49647 esm: * set all hooks as release candidate (Geoffrey Booth) nodejs#49597 module: * fix the leak in SourceTextModule and ContextifySript (Joyee Cheung) nodejs#48510 * fix leak of vm.SyntheticModule (Joyee Cheung) nodejs#48510 * use symbol in WeakMap to manage host defined options (Joyee Cheung) nodejs#48510 src: * (SEMVER-MINOR) allow embedders to override NODE_MODULE_VERSION (Cheng Zhao) nodejs#49279 stream: * use bitmap in writable state (Raz Luvaton) nodejs#49834 * use bitmap in readable state (Benjamin Gruenbaum) nodejs#49745 * improve webstream readable async iterator performance (Raz Luvaton) nodejs#49662 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) nodejs#49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) nodejs#49614 PR-URL: nodejs#49932
Notable changes: deps: * add v8::Object::SetInternalFieldForNodeCore() (Joyee Cheung) nodejs#49874 doc: * deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) nodejs#49683 * deprecate `util.toUSVString` (Yagiz Nizipli) nodejs#49725 * deprecate calling `promisify` on a function that returns a promise (Antoine du Hamel) nodejs#49647 esm: * set all hooks as release candidate (Geoffrey Booth) nodejs#49597 module: * fix the leak in SourceTextModule and ContextifySript (Joyee Cheung) nodejs#48510 * fix leak of vm.SyntheticModule (Joyee Cheung) nodejs#48510 * use symbol in WeakMap to manage host defined options (Joyee Cheung) nodejs#48510 src: * (SEMVER-MINOR) allow embedders to override NODE_MODULE_VERSION (Cheng Zhao) nodejs#49279 stream: * use bitmap in writable state (Raz Luvaton) nodejs#49834 * use bitmap in readable state (Benjamin Gruenbaum) nodejs#49745 * improve webstream readable async iterator performance (Raz Luvaton) nodejs#49662 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) nodejs#49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) nodejs#49614 PR-URL: nodejs#49932
140% Improvement
Benchmark CI
My local tests