Skip to content
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

url: reduce pathToFileURL cpp calls #48709

Merged
merged 2 commits into from
Jul 16, 2023

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Jul 9, 2023

My local benchmarks show the following changes:

                                                                                                                 confidence improvement accuracy (*)   (**)  (***)
url/whatwg-url-to-and-from-path.js n=1000000 method='fileURLToPath' input='file:///dev/null?key=param&bool'             ***     12.59 %       ±1.21% ±1.62% ±2.12%
url/whatwg-url-to-and-from-path.js n=1000000 method='fileURLToPath' input='file:///dev/null?key=param&bool#hash'        ***      9.72 %       ±0.70% ±0.94% ±1.22%
url/whatwg-url-to-and-from-path.js n=1000000 method='fileURLToPath' input='file:///dev/null'                            ***     18.85 %       ±1.02% ±1.36% ±1.78%
url/whatwg-url-to-and-from-path.js n=1000000 method='pathToFileURL' input='file:///dev/null?key=param&bool'             ***     11.93 %       ±0.78% ±1.04% ±1.35%
url/whatwg-url-to-and-from-path.js n=1000000 method='pathToFileURL' input='file:///dev/null?key=param&bool#hash'        ***     10.31 %       ±0.67% ±0.89% ±1.16%
url/whatwg-url-to-and-from-path.js n=1000000 method='pathToFileURL' input='file:///dev/null'                            ***     18.86 %       ±1.25% ±1.67% ±2.18%

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

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Jul 9, 2023
@anonrig anonrig added the performance Issues and PRs related to the performance of Node.js. label Jul 9, 2023
@anonrig anonrig force-pushed the improve-pathToFileURL branch from a864b46 to ce3764d Compare July 9, 2023 18:31
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 9, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 9, 2023
@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig force-pushed the improve-pathToFileURL branch from 99f3dfe to 6d58fe4 Compare July 11, 2023 19:59
@anonrig anonrig requested a review from lpinca July 11, 2023 19:59
@anonrig anonrig added 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 Jul 12, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 12, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@anonrig
Copy link
Member Author

anonrig commented Jul 14, 2023

@nodejs/url ping

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 16, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 16, 2023
@nodejs-github-bot nodejs-github-bot merged commit 7196946 into nodejs:main Jul 16, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 7196946

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023
PR-URL: #48709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@UlisesGascon UlisesGascon mentioned this pull request Aug 15, 2023
targos pushed a commit that referenced this pull request Nov 27, 2023
PR-URL: #48709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
PR-URL: nodejs/node#48709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
PR-URL: nodejs/node#48709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
# 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. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants