GeezerVison

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

GeezerVison is showing github.com — fs: improve error performance for `mkdirSync` by CanadaHonk · Pull Request #49847 · nodejs/node

Conversation

CanadaHonk commented Sep 25, 2023

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

Sorry, something went wrong.

Uh oh!

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

bricss and H4ad reacted with rocket emoji

All reactions

🚀 2 reactions

nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

Issues and PRs related to file-system APIs and the fs module.

needs-ci PRs that need a full CI run. labels

Sep 25, 2023

anonrig reviewed

Sep 25, 2023

View reviewed changes

Comment thread

src/node_file.cc Resolved

Uh oh!

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

Comment thread

src/node_file.cc Resolved

Uh oh!

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

Comment thread

src/node_file.cc Resolved

Uh oh!

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

CanadaHonk commented Sep 25, 2023

Copy link

Copy Markdown

Member Author

Updated to fix all review comments, thanks!

All reactions

Sorry, something went wrong.

Uh oh!

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

anonrig approved these changes

Sep 25, 2023

View reviewed changes

anonrig approved these changes

Sep 25, 2023

View reviewed changes

anonrig added performance

Issues and PRs related to the performance of Node.js.

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels

Sep 25, 2023

anonrig mentioned this pull request

Sep 25, 2023

Performance of node:fs nodejs/performance#106

Open

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 25, 2023

nodejs-github-bot commented Sep 25, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54210/

All reactions

Sorry, something went wrong.

Uh oh!

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

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 25, 2023

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 25, 2023

nodejs-github-bot commented Sep 25, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54231/

All reactions

Sorry, something went wrong.

Uh oh!

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

CanadaHonk commented Sep 25, 2023

Copy link

Copy Markdown

Member Author

Flaky CI?

anonrig reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

nodejs-github-bot commented Sep 25, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54238/

All reactions

Sorry, something went wrong.

Uh oh!

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

CanadaHonk commented Sep 26, 2023

Copy link

Copy Markdown

Member Author

Broken (empty?) CI again?

All reactions

Sorry, something went wrong.

Uh oh!

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

CanadaHonk force-pushed the perf-mkdirsync branch from 66f03f6 to b68c5d8 Compare

September 26, 2023 00:52

CanadaHonk commented Sep 26, 2023

Copy link

Copy Markdown

Member Author

Rebased on latest main to fix merge conflict.

All reactions

Sorry, something went wrong.

Uh oh!

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

anonrig approved these changes

Sep 26, 2023

View reviewed changes

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 26, 2023

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 26, 2023

nodejs-github-bot commented Sep 26, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54242/

All reactions

Sorry, something went wrong.

Uh oh!

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

nodejs-github-bot commented Sep 26, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54255/

All reactions

Sorry, something went wrong.

Uh oh!

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

github-actions Bot mentioned this pull request

Sep 27, 2023

CI Reliability 2023-09-27 nodejs/reliability#676

Open

21 tasks

anonrig commented Sep 27, 2023

Copy link

Copy Markdown

Member

@CanadaHonk can you resolve the conflicts?

All reactions

Sorry, something went wrong.

Uh oh!

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

anonrig added commit-queue-squash

PRs the Commit Queue should land as one squashed commit.

request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels

Sep 27, 2023

joyeecheung commented Oct 2, 2023

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.

All reactions

Sorry, something went wrong.

Uh oh!

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

anonrig force-pushed the perf-mkdirsync branch from fe28c76 to 92b3a70 Compare

October 11, 2023 18:26

anonrig added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels

Oct 11, 2023

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 11, 2023

nodejs-github-bot commented Oct 11, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54711/

All reactions

Sorry, something went wrong.

Uh oh!

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

anonrig force-pushed the perf-mkdirsync branch from 92b3a70 to b826f39 Compare

October 11, 2023 19:18

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 11, 2023

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 11, 2023

nodejs-github-bot commented Oct 11, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54717/

All reactions

Sorry, something went wrong.

Uh oh!

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

joyeecheung reviewed

Oct 12, 2023

View reviewed changes

Comment thread

src/node_file.cc Resolved

Uh oh!

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

anonrig force-pushed the perf-mkdirsync branch from b826f39 to 6b3b154 Compare

October 12, 2023 15:11


          fs: improve error performance for mkdirSync

569c96b

anonrig force-pushed the perf-mkdirsync branch from 6b3b154 to 569c96b Compare

October 15, 2023 16:58

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 17, 2023

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 17, 2023

nodejs-github-bot commented Oct 17, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54898/

All reactions

Sorry, something went wrong.

Uh oh!

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

github-actions Bot mentioned this pull request

Oct 19, 2023

CI Reliability 2023-10-19 nodejs/reliability#691

Open

25 tasks

joyeecheung approved these changes

Oct 19, 2023

View reviewed changes

nodejs-github-bot commented Oct 19, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55006/

All reactions

Sorry, something went wrong.

Uh oh!

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

anonrig added the commit-queue PRs queued for automated landing through the Commit Queue. label

Oct 20, 2023

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Oct 20, 2023

nodejs-github-bot merged commit c37cf18 into nodejs:main

Oct 20, 2023

nodejs-github-bot commented Oct 20, 2023

Copy link

Copy Markdown

Collaborator

Landed in c37cf18

All reactions

Sorry, something went wrong.

Uh oh!

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

targos pushed a commit that referenced this pull request

Oct 23, 2023


          fs: improve error performance for mkdirSync

6ec5aba

PR-URL: #49847
Refs: nodejs/performance#106
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>

targos mentioned this pull request

Oct 23, 2023

v21.1.0 release proposal #50335

Merged

targos pushed a commit that referenced this pull request

Nov 11, 2023


          fs: improve error performance for mkdirSync

8d0edc6

PR-URL: #49847
Refs: nodejs/performance#106
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>

targos mentioned this pull request

Nov 12, 2023

v20.10.0 release proposal #50682

Merged

juanarbol mentioned this pull request

Dec 6, 2023

2023-12-06, N|Solid v5.0.1 Iron Release nodesource/nsolid#41

Merged

Reviewers

anonrig

anonrig approved these changes

joyeecheung

joyeecheung approved these changes

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++.

Issues and PRs related to file-system APIs and the fs module.

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.

4 participants

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.