Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: improve error performance for fs.renameSync #49863

Closed
wants to merge 1 commit into from

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Sep 25, 2023

                                              confidence improvement accuracy (*)    (**)   (***)
fs/bench-renameSync.js n=1000 type='invalid'        ***     50.69 %      ±16.74% ±22.31% ±29.10%
fs/bench-renameSync.js n=1000 type='valid'                   1.87 %      ±12.40% ±16.50% ±21.48%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 2 comparisons, you can thus expect the following amount of false-positive results:
  0.10 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.02 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

Ref: nodejs/performance#106

@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 related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Sep 25, 2023
@anonrig
Copy link
Member Author

anonrig commented Sep 25, 2023

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1417/

22:15:50                                              confidence improvement accuracy (*)   (**)  (***)
22:15:50 fs/bench-renameSync.js n=1000 type='invalid'        ***     64.92 %       ±1.56% ±2.08% ±2.71%
22:15:50 fs/bench-renameSync.js n=1000 type='valid'                   0.85 %       ±4.16% ±5.56% ±7.29%

@anonrig anonrig added performance Issues and PRs related to the performance of Node.js. needs-benchmark-ci PR that need a benchmark CI run. labels Sep 25, 2023
src/node_file.cc Outdated Show resolved Hide resolved
@anonrig anonrig requested a review from targos September 25, 2023 18:57
@anonrig anonrig force-pushed the improve-fs-renamesync branch from 4c1f660 to 1d48a89 Compare September 26, 2023 15:23
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig force-pushed the improve-fs-renamesync branch from 1d48a89 to 2b4baa4 Compare September 27, 2023 13:14
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 27, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 27, 2023
@nodejs-github-bot
Copy link
Collaborator

@anonrig
Copy link
Member Author

anonrig commented Sep 29, 2023

Closing in favor of #49962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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-benchmark-ci PR 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
Development

Successfully merging this pull request may close these issues.

3 participants