Skip to content

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-botnodejs-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
@H4adH4ad 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
H4ad
H4ad approved these changes Nov 7, 2023
Copy link
Member

@H4adH4ad 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-actionsgithub-actionsbot 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

@nodejs-github-bot
Copy link
Collaborator

@Septa2112
Copy link
ContributorAuthor

@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
ContributorAuthor

@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-botnodejs-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

@debadree25debadree25 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-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 9, 2023
@nodejs-github-botnodejs-github-bot merged commit bb2dd0e into nodejs:mainNov 9, 2023
@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]>
@targostargos mentioned this pull request Nov 12, 2023
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]>
@UlisesGasconUlisesGascon mentioned this pull request Dec 12, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.benchmarkIssues and PRs related to the benchmark subsystem.v8 engineIssues 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

@Septa2112@nodejs-github-bot@H4ad@debadree25