Skip to content

Conversation

@anonrig
Copy link
Member

@anonriganonrig commented Sep 29, 2023

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

@anonriganonrig added performance Issues and PRs related to the performance of Node.js. needs-benchmark-ci PR that need a benchmark CI run. labels Sep 29, 2023
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@anonriganonrig changed the title Fs improvementsnode:fs error path performance improvementsSep 29, 2023
@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Sep 29, 2023
@anonriganonrig added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Sep 29, 2023
Copy link
Member

@CanadaHonkCanadaHonk left a 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

@joyeecheung
Copy link
Member

This needs a rebase since #49913 landed

@anonriganonrig marked this pull request as ready for review September 30, 2023 18:56
@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 30, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 30, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@anonrig
Copy link
MemberAuthor

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

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 1, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 1, 2023
@nodejs-github-bot
Copy link
Collaborator

@anonriganonrigforce-pushed the fs-improvements branch 2 times, most recently from 573b0bf to c437b20CompareOctober 1, 2023 02:27
targos pushed a commit that referenced this pull request Oct 23, 2023
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
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
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
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
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
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]>
@targostargos mentioned this pull request Oct 23, 2023
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
The test is only flaky on x86 Windows. Fixes: nodejs#50220 PR-URL: nodejs#50238 Refs: nodejs#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]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#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]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#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]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#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]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#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]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#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]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#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]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#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]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#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 Nov 11, 2023
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 Nov 11, 2023
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 Nov 11, 2023
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 Nov 11, 2023
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 Nov 11, 2023
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 Nov 11, 2023
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 Nov 11, 2023
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 Nov 11, 2023
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 Nov 11, 2023
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]>
@targostargos mentioned this pull request Nov 12, 2023
targos pushed a commit that referenced this pull request Nov 27, 2023
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]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
The test is only flaky on x86 Windows. Fixes: nodejs/node#50220 PR-URL: nodejs/node#50238 Refs: nodejs/node#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]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
The test is only flaky on x86 Windows. Fixes: nodejs/node#50220 PR-URL: nodejs/node#50238 Refs: nodejs/node#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]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.commit-queue-rebaseAdd this label to allow the Commit Queue to land a PR in several commits.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-benchmark-ciPR that need a benchmark CI run.needs-ciPRs that need a full CI run.performanceIssues and PRs related to the performance of Node.js.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@anonrig@nodejs-github-bot@joyeecheung@tniessen@RafaelGSS@StefanStojanovic@mcollina@Qard@H4ad@CanadaHonk