-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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 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