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: change iterations in benchmark/es/string-concatenations.js #50585

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

Septa2112
Copy link
Contributor

Increase the number of iterations from 1e3 to 1e6 to avoid the test performance gap caused by inactive V8 optimization caused by too few iterations.

Fixes: #50571

@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. v8 engine Issues and PRs related to the V8 dependency. labels Nov 7, 2023
@H4ad H4ad 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. commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 7, 2023
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!

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Septa2112
Copy link
Contributor Author

@H4ad I found that three checks failed. But I don't think my modifications should trigger these issues. Could you help me retrigger them? Thanks.

@nodejs-github-bot
Copy link
Collaborator

@Septa2112
Copy link
Contributor Author

@H4ad I found that three checks failed. But I don't think my modifications should trigger these issues. Could you help me retrigger them? Thanks.

It looks okay now.

@H4ad
Copy link
Member

H4ad commented Nov 7, 2023

@Septa2112 is common, some checks can fail because of flaky tests.

Now the PR will wait some hours until it gets merged automatically.

Increase the number of iterations from `1e3` to `1e6`
to avoid the test performance gap caused by inactive
V8 optimization caused by too few iterations.

Fixes: nodejs#50571
@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 Nov 9, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/50585
✔  Done loading data for nodejs/node/pull/50585
----------------------------------- PR info ------------------------------------
Title      benchmark: change iterations in benchmark/es/string-concatenations.js (#50585)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     Septa2112:test-branch -> nodejs:main
Labels     v8 engine, benchmark, author ready
Commits    1
 - benchmark: change iterations in benchmark/es/string-concatenations.js
Committers 1
 - Jia Liu 
PR-URL: https://github.com/nodejs/node/pull/50585
Fixes: 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/50585
Fixes: https://github.com/nodejs/node/issues/50571
Reviewed-By: Vinícius Lourenço Claro Cardoso 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - benchmark: change iterations in benchmark/es/string-concatenations.js
   ℹ  This PR was created on Tue, 07 Nov 2023 02:43:49 GMT
   ✔  Approvals: 1
   ✔  - Vinícius Lourenço Claro Cardoso (@H4ad): https://github.com/nodejs/node/pull/50585#pullrequestreview-1716697270
   ✘  This PR needs to wait 119 more hours to land (or 0 hours if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/6806760384

@debadree25 debadree25 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 Nov 9, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 9, 2023
@nodejs-github-bot nodejs-github-bot merged commit bb2dd0e into nodejs:main Nov 9, 2023
68 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in bb2dd0e

targos pushed a commit that referenced this pull request Nov 11, 2023
Increase the number of iterations from `1e3` to `1e6`
to avoid the test performance gap caused by inactive
V8 optimization caused by too few iterations.

Fixes: #50571
PR-URL: #50585
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
targos pushed a commit that referenced this pull request Nov 14, 2023
Increase the number of iterations from `1e3` to `1e6`
to avoid the test performance gap caused by inactive
V8 optimization caused by too few iterations.

Fixes: #50571
PR-URL: #50585
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
Increase the number of iterations from `1e3` to `1e6`
to avoid the test performance gap caused by inactive
V8 optimization caused by too few iterations.

Fixes: #50571
PR-URL: #50585
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
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. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The performance gap between node16 and node21 changes as the n of the benchmark changes
4 participants