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

benchmark: increase iteration counts for 4 cases #50869

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

lucshi
Copy link
Contributor

@lucshi lucshi commented Nov 23, 2023

Increase the iteration counts to 10X to reflect real performance of perf hooks timing.

Refs: #50571

Belowing are the improved ratio with 10X iteration:

performance-observer.js
./node/benchmark//perf_hooks/performance-observer.js n=1000000
perf_hooks/performance-observer.js pending=1 n=1000000 percent=111.33%
perf_hooks/performance-observer.js pending=10 n=1000000 percent=127.58%

resourcetiming.js
./node/benchmark//perf_hooks/resourcetiming.js n=1000000
perf_hooks/resourcetiming.js observe="resource" n=1000000 percent=135.24%

timerfied.js
./node/benchmark//perf_hooks/timerfied.js n=1000000
perf_hooks/timerfied.js observe="function" n=1000000 percent=116.11%

usertiming.js
./node/benchmark//perf_hooks/usertiming.js n=1000000
perf_hooks/usertiming.js observe="all" n=1000000 percent=823.76%
perf_hooks/usertiming.js observe="measure" n=1000000 percent=1039.62%

@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. perf_hooks Issues and PRs related to the implementation of the Performance Timing API. labels Nov 23, 2023
@H4ad
Copy link
Member

H4ad commented Nov 26, 2023

Can you please update the first commit message to something like: Septa2112@a13bb22

Maybe benchmark: update iterations in perf_hooks.

Also, how are you generating this percent metric?

@lucshi
Copy link
Contributor Author

lucshi commented Nov 27, 2023

please update the first commit message to something like: Septa2112@a13bb22

Maybe benchmark: update iterations in perf_hooks.

Also, how are you generating this percent metric?

I wrote a script to compare the before and after score.

As the perf_hooks testing methods are only a few, so I will not split the commit. I have updated the commit msg. Could you please go ahead to approve it? Thanks!

Copy link
Member

@H4ad H4ad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR

@H4ad H4ad added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 27, 2023
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 5, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 5, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/50869
✔  Done loading data for nodejs/node/pull/50869
----------------------------------- PR info ------------------------------------
Title      benchmark: increase iteration counts for 4 cases (#50869)
Author     Lei Shi  (@lucshi)
Branch     lucshi:my-branch-5 -> nodejs:main
Labels     benchmark, author ready, perf_hooks
Commits    1
 - benchmark: update iterations in benchmark/perf_hooks
Committers 1
 - Lei Shi 
PR-URL: https://github.com/nodejs/node/pull/50869
Refs: https://github.com/nodejs/node/issues/50571
Reviewed-By: Vinícius Lourenço Claro Cardoso 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50869
Refs: https://github.com/nodejs/node/issues/50571
Reviewed-By: Vinícius Lourenço Claro Cardoso 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 23 Nov 2023 07:06:59 GMT
   ✔  Approvals: 1
   ✔  - Vinícius Lourenço Claro Cardoso (@H4ad): https://github.com/nodejs/node/pull/50869#pullrequestreview-1750127621
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 50869
From https://github.com/nodejs/node
 * branch                  refs/pull/50869/merge -> FETCH_HEAD
✔  Fetched commits as 27b2ce5ba633..3c6f29b785d1
--------------------------------------------------------------------------------
error: cherry-pick is already in progress
hint: try "git cherry-pick (--continue | --abort | --quit)"
fatal: cherry-pick failed
   ✘  Failed to apply patches
https://github.com/nodejs/node/actions/runs/7105099049

@lpinca lpinca added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 10, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 10, 2023
@nodejs-github-bot nodejs-github-bot merged commit 3b758b3 into nodejs:main Dec 10, 2023
29 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 3b758b3

RafaelGSS pushed a commit that referenced this pull request Dec 15, 2023
Fixes: #50571
PR-URL: #50869
Refs: #50571
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Dec 15, 2023
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Fixes: #50571
PR-URL: #50869
Refs: #50571
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. benchmark Issues and PRs related to the benchmark subsystem. perf_hooks Issues and PRs related to the implementation of the Performance Timing API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants