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: update iterations of benchmark/domain/domain-fn-args.js #51408

Merged

Conversation

lucshi
Copy link
Contributor

@lucshi lucshi commented Jan 9, 2024

Fixes: #50571

Found 40% regression on node20 compared with older version:
benchmark@benchmark-S2600WFT:
/luc/node_new$ /home/benchmark/.nvm/versions/node/v16.20.2/bin/node benchmark/domain/domain-fn-args.js
domain/domain-fn-args.js n=10 args=0: 59,126.81519322643
domain/domain-fn-args.js n=10 args=1: 55,949.690038717185
domain/domain-fn-args.js n=10 args=2: 49,182.34353866962
domain/domain-fn-args.js n=10 args=3: 58,395.18356525954

benchmark@benchmark-S2600WFT:~/luc/node_new$ /home/benchmark/.nvm/versions/node/v20.11.1/bin/node benchmark/domain/domain-fn-args.js
domain/domain-fn-args.js n=10 args=0: 38,578.609704063485
domain/domain-fn-args.js n=10 args=1: 38,735.66780291292
domain/domain-fn-args.js n=10 args=2: 39,472.645456698505
domain/domain-fn-args.js n=10 args=3: 39,021.49694266571

@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. domain Issues and PRs related to the domain subsystem. labels Jan 9, 2024
@lucshi
Copy link
Contributor Author

lucshi commented Mar 1, 2024

Reason is the repetition is too small and the data variance is caused by GC activities.
After increasing the repetition, node 20 downgration is fixed.

Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

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

For reference, the original intention of the benchmark case was to verify the performance of fn.call between the size of the sliced argument array: 8c69d7b.

benchmark/domain/domain-fn-args.js Outdated Show resolved Hide resolved
Co-authored-by: Chengzhong Wu <legendecas@gmail.com>
@legendecas legendecas added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Mar 4, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 4, 2024
@nodejs-github-bot nodejs-github-bot merged commit fde5534 into nodejs:main Mar 4, 2024
22 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in fde5534

@lucshi lucshi deleted the benchmark/domain/domain-fn-args.js branch March 5, 2024 01:20
targos pushed a commit that referenced this pull request Mar 7, 2024
Fixes: #50571
PR-URL: #51408
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@targos targos mentioned this pull request Mar 7, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Fixes: #50571
PR-URL: #51408
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Fixes: #50571
PR-URL: #51408
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
Fixes: nodejs#50571
PR-URL: nodejs#51408
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Issues and PRs related to the benchmark subsystem. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. domain Issues and PRs related to the domain subsystem.
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