Skip to content

fs: improve error performance for writeSync #50354

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 2 commits into from

Conversation

pluris
Copy link
Contributor

@pluris pluris commented Oct 24, 2023

I worked on it because it seemed like there was still work left to do in nodejs/performance#106.

                                              confidence improvement accuracy (*)    (**)   (***)
fs/bench-writeSync.js n=100000 type='invalid'        ***    116.41 %       ±9.35% ±12.49% ±16.36%
fs/bench-writeSync.js n=100000 type='valid'                   0.46 %       ±6.13%  ±8.16% ±10.62%

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 (***)

Refs: nodejs/performance#106

@nodejs-github-bot nodejs-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. typings labels Oct 24, 2023
@anonrig anonrig added request-ci Add this label to start a Jenkins CI on a PR. 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. labels Oct 24, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@nodejs-github-bot
Copy link
Collaborator

@anonrig
Copy link
Member

anonrig commented Oct 24, 2023

@pluris the tests are failing

@pluris
Copy link
Contributor Author

pluris commented Oct 24, 2023

@anonrig After rebasing and checking, there seems to be a problem. I will fix it and let you know. thank you

@pluris
Copy link
Contributor Author

pluris commented Oct 24, 2023

Something is weird. It seems like a fail occurs in parallel.test-release-changelog.
It seems to be happening in other PR as well. (#50361)
Could it be related to the recent commit that changed the CHANGELOG file?

@richardlau
Copy link
Member

Something is weird. It seems like a fail occurs in parallel.test-release-changelog. It seems to be happening in other PR as well. (#50361) Could it be related to the recent commit that changed the CHANGELOG file?

#50373. Being fixed by #50375. Will need a fresh (not resumed) CI run to pick up the change when it lands on main.

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@nodejs-github-bot
Copy link
Collaborator

@H4ad H4ad added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Oct 25, 2023
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 25, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 25, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@pluris pluris force-pushed the feat/write_sync branch 2 times, most recently from 3824b3a to 0d46fd4 Compare December 10, 2023 14:23
@deokjinkim deokjinkim added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 25, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 25, 2023
@nodejs-github-bot
Copy link
Collaborator

@pluris pluris force-pushed the feat/write_sync branch 4 times, most recently from e08d243 to 1055136 Compare December 25, 2023 09:55
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 27, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 27, 2023
@nodejs-github-bot
Copy link
Collaborator

@pluris pluris force-pushed the feat/write_sync branch 3 times, most recently from d68e8ef to 8f8be72 Compare December 27, 2023 16:22
@pluris pluris closed this Dec 29, 2023
@pluris pluris reopened this Dec 29, 2023
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2023
@aduh95
Copy link
Contributor

aduh95 commented Dec 30, 2023

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2023
@nodejs-github-bot
Copy link
Collaborator

@PodaruDragos
Copy link

hey guys, is there anything that's blocking this one out ?

@pluris pluris closed this Apr 11, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs 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++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js. typings
Projects
None yet
Development

Successfully merging this pull request may close these issues.