Skip to content
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

[v14.x backport] fs: improve fsPromises readFile performance #39838

Conversation

MoritzLoewenstein
Copy link

@MoritzLoewenstein MoritzLoewenstein commented Aug 21, 2021

Improve the fsPromises readFile performance
by allocating only one buffer, when size is known,
increase the size of the readbuffer chunks,
and dont read more data if size bytes have been read.
Also moves constants to internal/fs/utils.

refs: #37583
(Old) Backport-PR-URL: #37703
PR-URL: #37608

This PR is backporting the fs readFile promise performance improvements to v14.x, essentially copying these two prs (i wasnt able to cherry-pick, not sure if its my git skills or something else):
#38061 "fs: move constants to internal/fs/utils.js"
#37703 "[v14.x backport] fs: improve fsPromises readFile performance"

@github-actions github-actions bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. v14.x labels Aug 21, 2021
@aduh95
Copy link
Contributor

aduh95 commented Aug 21, 2021

Assuming upstream and origin are configured in a classical way, you should be able to cherry-pick the original commits using the following commands:

git fetch upstream master
git fetch upstream v14.x-staging
git reset upstream/v14.x-staging --hard
git cherry-pick 24fd791184ad0ba27f92ac49b820aec78e491296
git cherry-pick e216d8f4a1277e77903e9c2c716fefa3b196761b
git cherry-pick b9fd4eb651dd26c3845aa46714593abae185455d
git push origin HEAD:fs-read-promises-backport -f

@MoritzLoewenstein MoritzLoewenstein force-pushed the fs-read-promises-backport branch from b4fffb0 to 5d007af Compare August 21, 2021 20:41
@MoritzLoewenstein
Copy link
Author

MoritzLoewenstein commented Aug 21, 2021

Thank you :)

Added ArrayPrototypePush import to 2nd commit, your third cherry-pick is already in v14.

@MoritzLoewenstein MoritzLoewenstein force-pushed the fs-read-promises-backport branch 2 times, most recently from 71a3fb3 to 6f22200 Compare August 21, 2021 21:52
RaisinTen and others added 2 commits September 1, 2021 09:30
Refs: nodejs#38004 (comment)

PR-URL: nodejs#38061
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Improve the fsPromises readFile performance
by allocating only one buffer, when size is known,
increase the size of the readbuffer chunks,
and dont read more data if size bytes have been read

Refs: nodejs#37583
PR-URL: nodejs#37608
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MoritzLoewenstein MoritzLoewenstein force-pushed the fs-read-promises-backport branch from a179bd3 to 2ffa9c5 Compare September 1, 2021 07:30
targos pushed a commit that referenced this pull request Sep 4, 2021
Refs: #38004 (comment)

PR-URL: #38061
Backport-PR-URL: #39838
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Sep 4, 2021
Improve the fsPromises readFile performance
by allocating only one buffer, when size is known,
increase the size of the readbuffer chunks,
and dont read more data if size bytes have been read

Refs: #37583
PR-URL: #37608
Backport-PR-URL: #39838
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@targos
Copy link
Member

targos commented Sep 4, 2021

Landed in f69c934...2b0e270. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants