Skip to content

fs: improve error performance for mkdirSync - #49847

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
CanadaHonk:perf-mkdirsync
Oct 20, 2023
Merged

fs: improve error performance for mkdirSync#49847
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
CanadaHonk:perf-mkdirsync

Conversation

@CanadaHonk

Copy link
Copy Markdown
Member

Results from i7 Windows laptop:

 confidence improvement accuracy (*) (**) (***)
fs\bench-mkdirSync.js n=1000 recursive='false' type='existing' *** 66.99 % ±3.62% ±4.85% ±6.38%
fs\bench-mkdirSync.js n=1000 recursive='false' type='non-existing' 3.40 % ±4.02% ±5.35% ±6.96%
fs\bench-mkdirSync.js n=1000 recursive='true' type='existing' 4.32 % ±5.81% ±7.76% ±10.16%
fs\bench-mkdirSync.js n=1000 recursive='true' type='non-existing' * 3.36 % ±2.72% ±3.62% ±4.71%

Ref: nodejs/performance#106

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Sep 25, 2023
Comment threadsrc/node_file.cc Outdated
Comment threadsrc/node_file.cc Outdated
Comment threadsrc/node_file.cc Outdated
@CanadaHonk

Copy link
Copy Markdown
MemberAuthor

Updated to fix all review comments, thanks!

@anonriganonrig added performance Issues and PRs related to the performance of Node.js. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 25, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 25, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 25, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 25, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@CanadaHonk

Copy link
Copy Markdown
MemberAuthor

Flaky CI?

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@CanadaHonk

Copy link
Copy Markdown
MemberAuthor

Broken (empty?) CI again?

@CanadaHonk

Copy link
Copy Markdown
MemberAuthor

Rebased on latest main to fix merge conflict.

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonrig

Copy link
Copy Markdown
Member

@CanadaHonk can you resolve the conflicts?

@anonriganonrig added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 27, 2023
@joyeecheung

Copy link
Copy Markdown
Member

#49913 has landed. Can you move the JS code back to lib/fs.js and switch to use SyncCallAndThrowOnError in the original implementation instead of introducing a new binding? Thanks.

@anonriganonrig added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Oct 11, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 11, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 11, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 11, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Comment threadsrc/node_file.cc Outdated
@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 17, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonriganonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 20, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 20, 2023
@nodejs-github-bot
nodejs-github-bot merged commit c37cf18 into nodejs:mainOct 20, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in c37cf18

targos pushed a commit that referenced this pull request Oct 23, 2023
PR-URL: #49847
Refs: nodejs/performance#106
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@targostargos mentioned this pull request Oct 23, 2023
targos pushed a commit that referenced this pull request Nov 11, 2023
PR-URL: #49847
Refs: nodejs/performance#106
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@targostargos mentioned this pull request Nov 12, 2023
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++.fsIssues and PRs related to the fs subsystem / file system.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.

4 participants

@CanadaHonk@nodejs-github-bot@anonrig@joyeecheung