GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [v14.x backport] fs: improve fsPromises readFile performance by MoritzLoewenstein · Pull Request #39838 · nodejs/node

Conversation

@MoritzLoewenstein

@MoritzLoewenstein MoritzLoewenstein commented Aug 21, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

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"

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@github-actions github-actions Bot added fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. v14.x labels Aug 21, 2021
@aduh95

aduh95 commented Aug 21, 2021

Copy link
Copy Markdown
Contributor

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoritzLoewenstein
MoritzLoewenstein force-pushed the fs-read-promises-backport branch from b4fffb0 to 5d007af Compare August 21, 2021 20:41
@MoritzLoewenstein

MoritzLoewenstein commented Aug 21, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Author

Thank you :)

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoritzLoewenstein
MoritzLoewenstein force-pushed the fs-read-promises-backport branch 2 times, most recently from 71a3fb3 to 6f22200 Compare August 21, 2021 21:52
Linkgoron
Linkgoron reviewed Aug 25, 2021
View reviewed changes
Comment thread lib/internal/fs/promises.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@MoritzLoewenstein
MoritzLoewenstein force-pushed the fs-read-promises-backport branch from 81f457d to a179bd3 Compare August 25, 2021 21:35
@MylesBorins
MylesBorins force-pushed the v14.x-staging branch from b3f51ee to 327838d Compare August 31, 2021 14:58
RaisinTen and others added 2 commits September 1, 2021 09:30
@RaisinTen @MoritzLoewenstein
fs: move constants to internal/fs/utils.js
07d5dd2 
Refs: nodejs#38004 (comment)

PR-URL: nodejs#38061
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@Linkgoron @MoritzLoewenstein
fs: improve fsPromises readFile performance
2ffa9c5 
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@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
@RaisinTen @targos
fs: move constants to internal/fs/utils.js
a4d6f78 
Refs: #38004 (comment)

PR-URL: #38061
Backport-PR-URL: #39838
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Sep 4, 2021
@Linkgoron @targos
fs: improve fsPromises readFile performance
2b0e270 
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos
targos approved these changes Sep 4, 2021
View reviewed changes
@targos

targos commented Sep 4, 2021

Copy link
Copy Markdown
Member

Landed in f69c934...2b0e270. Thank you!
MoritzLoewenstein and Linkgoron reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos targos closed this Sep 4, 2021
@MoritzLoewenstein MoritzLoewenstein mentioned this pull request Sep 4, 2021
[v14.x backport] fs: improve fsPromises writeFile performance #39993
Closed
@MoritzLoewenstein
MoritzLoewenstein deleted the fs-read-promises-backport branch September 6, 2021 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@Linkgoron Linkgoron Linkgoron left review comments
@targos targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@MoritzLoewenstein @aduh95 @targos @Linkgoron @RaisinTen
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.