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/async_hooks/async-local- #51420

Conversation

lucshi
Copy link
Contributor

@lucshi lucshi commented Jan 10, 2024

storage-getstore-nested-resources.js

Before applying this PR, the real logic code of "aync hooks" is not measured accurately.
After increasing the iteration value, the test case behavior made sense.

<style> </style>
    after PR before PR benefit
async_hooks/async-local-storage-getstore-nested-resources.js resourceCount=10: 247429210.5 12782754.53 1936%
async_hooks/async-local-storage-getstore-nested-resources.js resourceCount=100: 241176206.7 12875431.49 1873%
async_hooks/async-local-storage-getstore-nested-resources.js resourceCount=1000: 265273098.7 16031987.02 1655%

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

lucshi commented Mar 1, 2024

benchmark@benchmark-S2600WFT: v16.20.2/bin/node benchmark/async_hooks/async-local-storage-getstore-nested-resources.js

async_hooks/async-local-storage-getstore-nested-resources.js n=10000 resourceCount=10: 21,332,514.164789405
async_hooks/async-local-storage-getstore-nested-resources.js n=10000 resourceCount=100: 20,428,967.458697733
async_hooks/async-local-storage-getstore-nested-resources.js n=10000 resourceCount=1000: 19,337,123.4095216

benchmark@benchmark-S2600WFT: v20.10.0/bin/node  benchmark/async_hooks/async-local-storage-getstore-nested-resources.js
async_hooks/async-local-storage-getstore-nested-resources.js n=10000 resourceCount=10: 13,379,304.62402147
async_hooks/async-local-storage-getstore-nested-resources.js n=10000 resourceCount=100: 13,310,421.660847794
async_hooks/async-local-storage-getstore-nested-resources.js n=10000 resourceCount=1000: 13,387,149.67502694

compared with node16, node21 performance downgraded by about 40%. Root cause is repetition is too small and only occupies about 1% of scoring period while 99% is occupied by GC. When the repetition is enlarged a lot, node21 downgration was fixed.

benchmark@benchmark-S2600WFT:~/luc/node_new$ /home/benchmark/.nvm/versions/node/v20.10.0/bin/node  benchmark/async_hooks/async-local-storage-getstore-nested-resources.js
async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=10: 186,648,648.85043097
async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=100: 196,515,698.066993
async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=1000: 200,298,043.4887112

benchmark@benchmark-S2600WFT:~/luc/node_new$ /home/benchmark/.nvm/versions/node/v16.20.2/bin/node  benchmark/async_hooks/async-local-storage-getstore-nested-resources.js                                           async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=10: 172,352,994.0989782
async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=100: 175,277,771.4483027
async_hooks/async-local-storage-getstore-nested-resources.js n=500000 resourceCount=1000: 164,489,213.29086003

@legendecas
Copy link
Member

Would you mind rebasing on top of the main branch? One of the GitHub actions failed but I can not restart it.

@lucshi
Copy link
Contributor Author

lucshi commented Mar 1, 2024

sure I will rebase

@lucshi lucshi force-pushed the async_hooks/async-local-storage-getstore-nested-resources.js branch from 5156af6 to 13b615c Compare March 1, 2024 13:25
@legendecas legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label 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 bf4a63e into nodejs:main Mar 4, 2024
19 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in bf4a63e

@lucshi lucshi deleted the async_hooks/async-local-storage-getstore-nested-resources.js branch March 5, 2024 01:19
targos pushed a commit that referenced this pull request Mar 7, 2024
storage-getstore-nested-resources.js

Fixes: #50571
PR-URL: #51420
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
storage-getstore-nested-resources.js

Fixes: #50571
PR-URL: #51420
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
storage-getstore-nested-resources.js

Fixes: #50571
PR-URL: #51420
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
storage-getstore-nested-resources.js

Fixes: nodejs#50571
PR-URL: nodejs#51420
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. benchmark Issues and PRs related to the benchmark subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants