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 — `node:fs` error path performance improvements by anonrig · Pull Request #49962 · nodejs/node

Conversation

@anonrig

@anonrig anonrig commented Sep 29, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Closes #49863
Closes #49750
Closes #49748

Local benchmarks:

  • readlinkSync, linkSync, symlinkSync
fs/bench-linkSync.js n=1000 type='invalid'               ***     33.31 %       ±1.38% ±1.84%  ±2.40%
fs/bench-linkSync.js n=1000 type='valid'                         -1.06 %       ±3.29% ±4.40%  ±5.77%
fs/bench-readlinkSync.js n=1000 type='invalid'           ***     25.55 %       ±0.96% ±1.28%  ±1.66%
fs/bench-readlinkSync.js n=1000 type='valid'                      4.01 %       ±6.24% ±8.32% ±10.87%
fs/bench-symlinkSync.js n=1000 type='invalid'            ***     31.08 %       ±1.08% ±1.45%  ±1.90%
fs/bench-symlinkSync.js n=1000 type='valid'                       1.37 %       ±3.34% ±4.45%  ±5.79%
  • renameSync
fs/bench-renameSync.js n=2000 type='invalid'        ***     44.88 %       ±4.54% ±6.06% ±7.93%
fs/bench-renameSync.js n=2000 type='valid'                  -1.36 %       ±4.57% ±6.08% ±7.92%
  • chownSync & lchownSync
fs/bench-chownSync.js n=10000 method='chownSync' type='existing'              **      3.08 %       ±1.93% ±2.58% ±3.39%
fs/bench-chownSync.js n=10000 method='chownSync' type='non-existing'         ***     90.16 %       ±1.57% ±2.09% ±2.72%
fs/bench-chownSync.js n=10000 method='lchownSync' type='existing'                    -0.03 %       ±1.99% ±2.65% ±3.45%
fs/bench-chownSync.js n=10000 method='lchownSync' type='non-existing'        ***     87.50 %       ±1.75% ±2.33% ±3.06%
  • mkdtempSync
fs/bench-mkdtempSync.js n=1000 type='invalid'        ***     54.66 %       ±1.72% ±2.30% ±3.01%
fs/bench-mkdtempSync.js n=1000 type='valid'                  -1.20 %       ±3.58% ±4.77% ±6.20%

Ref: nodejs/performance#106

cc @nodejs/performance

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
daniellockyer, H4ad, CanadaHonk, bricss, and Uzlopak reacted with rocket emoji
All reactions
  • 🚀 5 reactions
@anonrig anonrig added performance Issues and PRs related to the performance of Node.js. needs-benchmark-ci PRs that need a benchmark CI run. labels Sep 29, 2023
@anonrig
anonrig requested a review from joyeecheung September 29, 2023 17:20
@nodejs-github-bot

nodejs-github-bot commented Sep 29, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig anonrig changed the title Fs improvements node:fs error path performance improvements Sep 29, 2023
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 29, 2023
@anonrig anonrig mentioned this pull request Sep 29, 2023
fs: improve error performance for fs.renameSync #49863
Closed
@anonrig anonrig added the commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. label Sep 29, 2023
This was referenced Sep 29, 2023
fs: improve error performance for fs.mkdtempSync #49750
Closed
fs: improve error performance of chownSync #49748
Closed
Performance of node:fs nodejs/performance#106
Open
CanadaHonk
CanadaHonk approved these changes Sep 29, 2023
View reviewed changes

@CanadaHonk CanadaHonk left a comment

Copy link
Copy Markdown
Member

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 from initial pass

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@joyeecheung

joyeecheung commented Sep 30, 2023

Copy link
Copy Markdown
Member

This needs a rebase since #49913 landed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig
anonrig force-pushed the fs-improvements branch from 88b3339 to 8c81ead Compare September 30, 2023 18:56
@anonrig
anonrig marked this pull request as ready for review September 30, 2023 18:56
@anonrig
anonrig force-pushed the fs-improvements branch from 8c81ead to 842b986 Compare September 30, 2023 18:57
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 30, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 30, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 30, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54420/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Sep 30, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54424/
All reactions

Sorry, something went wrong.

Uh oh!

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

anonrig commented Sep 30, 2023

Copy link
Copy Markdown
Member Author

Errors seems to be related. I'll look into it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Oct 1, 2023
CI Reliability 2023-10-01 nodejs/reliability#680
Open
16 tasks
@anonrig
anonrig force-pushed the fs-improvements branch from da5fcd9 to faf02c5 Compare October 1, 2023 00:19
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 1, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 1, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 1, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54425/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig
anonrig force-pushed the fs-improvements branch 2 times, most recently from 573b0bf to c437b20 Compare October 1, 2023 02:27
@nodejs-github-bot

nodejs-github-bot commented Oct 18, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54920/
All reactions

Sorry, something went wrong.

Uh oh!

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

StefanStojanovic commented Oct 18, 2023

Copy link
Copy Markdown
Contributor

The test that's problematic on Windows in the last few runs (parallel/test-worker-nearheaplimit-deadlock) is flaky, but only on x86. I've tested it locally on main to confirm that. The CI could be rerun until it eventually passes, but regardless of that, I plan to open a PR marking the flaky test on x86 Windows.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@StefanStojanovic StefanStojanovic mentioned this pull request Oct 18, 2023
test: set test-worker-nearheaplimit-deadlock flaky #50238
Merged
@nodejs-github-bot

nodejs-github-bot commented Oct 18, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54932/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Oct 18, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54943/
All reactions

Sorry, something went wrong.

Uh oh!

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

anonrig commented Oct 18, 2023

Copy link
Copy Markdown
Member Author

We finally got the green CI. Appreciate it if anybody can leave a new review, so we can land this!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@StefanStojanovic
test: set test-worker-nearheaplimit-deadlock flaky
dbd0ffa 
The test is only flaky on x86 Windows.

Fixes: #50220
PR-URL: #50238
Refs: #49962
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
H4ad
H4ad approved these changes Oct 18, 2023
View reviewed changes
@H4ad H4ad added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Oct 18, 2023
@anonrig anonrig added the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 18, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 18, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 18, 2023

Copy link
Copy Markdown
Collaborator

Landed in dbd0ffa...1a5db6f
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot closed this Oct 18, 2023
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@anonrig @nodejs-github-bot
fs: improve error performance of renameSync
a7e40f5 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@anonrig @nodejs-github-bot
fs: improve error performance of chownSync
1635366 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@anonrig @nodejs-github-bot
fs: improve error performance of linkSync
6b56641 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@anonrig @nodejs-github-bot
fs: improve error performance of mkdtempSync
d689965 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@anonrig @nodejs-github-bot
fs: improve error performance of readlinkSync
7603bf5 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@anonrig @nodejs-github-bot
fs: improve error performance of symlinkSync
449dc45 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@anonrig @nodejs-github-bot
fs: improve error performance of lchownSync
5f0276d 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2023
@anonrig @nodejs-github-bot
fs: improve error performance of realpathSync
1a5db6f 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
This was referenced Oct 19, 2023
CI Reliability 2023-10-19 nodejs/reliability#691
Open
CI Reliability 2023-10-20 nodejs/reliability#692
Open
targos pushed a commit that referenced this pull request Oct 23, 2023
@StefanStojanovic @targos
test: set test-worker-nearheaplimit-deadlock flaky
a38d131 
The test is only flaky on x86 Windows.

Fixes: #50220
PR-URL: #50238
Refs: #49962
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this pull request Oct 23, 2023
@anonrig @targos
fs: improve error performance of renameSync
610036c 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
targos pushed a commit that referenced this pull request Oct 23, 2023
@anonrig @targos
fs: improve error performance of chownSync
f766c04 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
targos pushed a commit that referenced this pull request Oct 23, 2023
@anonrig @targos
fs: improve error performance of linkSync
81f1527 
PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
@targos targos mentioned this pull request Oct 23, 2023
v21.1.0 release proposal #50335
Merged
@targos targos mentioned this pull request Nov 12, 2023
v20.10.0 release proposal #50682
Merged
@juanarbol juanarbol mentioned this pull request Dec 6, 2023
2023-12-06, N|Solid v5.0.1 Iron Release nodesource/nsolid#41
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes
@Qard Qard Qard approved these changes
@H4ad H4ad H4ad approved these changes
@joyeecheung joyeecheung Awaiting requested review from joyeecheung
+1 more reviewer
@CanadaHonk CanadaHonk CanadaHonk 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

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-benchmark-ci PRs that need a benchmark CI run. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js.

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.

10 participants

@anonrig @nodejs-github-bot @joyeecheung @tniessen @RafaelGSS @StefanStojanovic @mcollina @Qard @H4ad @CanadaHonk
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.