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.

Sign up for GitHub

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

Jump to bottom

benchmark: increase the iteration number to an appropriate value #50766

Merged
merged 1 commit into from Nov 19, 2023
Merged

benchmark: increase the iteration number to an appropriate value #50766

merged 1 commit into from Nov 19, 2023

Conversation

lucshi
Copy link
Contributor

@lucshi lucshi commented Nov 17, 2023

Current iteration number is too small that fwrite occupies large portion of execution time which made crypo execution time measured inaccurate. The iteration above 1e5 makes about 50% to 100X higher and stable score.

Before (n == 1e3)

crypto/webcrypto-digest.js n=1000 method="SHA-1" data=10 sync="createHash": 222,039.26187044097
crypto/webcrypto-digest.js n=1000 method="SHA-256" data=10 sync="createHash": 219,216.61626876658
crypto/webcrypto-digest.js n=1000 method="SHA-384" data=10 sync="createHash": 206,463.2920654505
crypto/webcrypto-digest.js n=1000 method="SHA-512" data=10 sync="createHash": 207,605.45994055425
crypto/webcrypto-digest.js n=1000 method="SHA-1" data=20 sync="createHash": 223,194.50709390262
crypto/webcrypto-digest.js n=1000 method="SHA-256" data=20 sync="createHash": 223,021.1334826088
crypto/webcrypto-digest.js n=1000 method="SHA-384" data=20 sync="createHash": 209,489.84822669983
crypto/webcrypto-digest.js n=1000 method="SHA-512" data=20 sync="createHash": 209,359.23721218077
crypto/webcrypto-digest.js n=1000 method="SHA-1" data=50 sync="createHash": 224,466.94151895432
crypto/webcrypto-digest.js n=1000 method="SHA-256" data=50 sync="createHash": 219,220.26857990422
crypto/webcrypto-digest.js n=1000 method="SHA-384" data=50 sync="createHash": 210,305.60559076423
crypto/webcrypto-digest.js n=1000 method="SHA-512" data=50 sync="createHash": 209,618.3374121568
crypto/webcrypto-digest.js n=1000 method="SHA-1" data=100 sync="createHash": 223,763.10468622597
crypto/webcrypto-digest.js n=1000 method="SHA-256" data=100 sync="createHash": 222,396.92736405155
crypto/webcrypto-digest.js n=1000 method="SHA-384" data=100 sync="createHash": 207,965.92686254284
crypto/webcrypto-digest.js n=1000 method="SHA-512" data=100 sync="createHash": 208,727.2625565625
crypto/webcrypto-digest.js n=1000 method="SHA-1" data=10 sync="subtle": 98,612.53154245087
crypto/webcrypto-digest.js n=1000 method="SHA-256" data=10 sync="subtle": 96,922.1686878008
crypto/webcrypto-digest.js n=1000 method="SHA-384" data=10 sync="subtle": 96,870.00423225049
crypto/webcrypto-digest.js n=1000 method="SHA-512" data=10 sync="subtle": 96,007.99708212496
crypto/webcrypto-digest.js n=1000 method="SHA-1" data=20 sync="subtle": 98,065.63572225145
crypto/webcrypto-digest.js n=1000 method="SHA-256" data=20 sync="subtle": 98,296.24108293361
crypto/webcrypto-digest.js n=1000 method="SHA-384" data=20 sync="subtle": 96,885.06756958383
crypto/webcrypto-digest.js n=1000 method="SHA-512" data=20 sync="subtle": 96,270.33325708723
crypto/webcrypto-digest.js n=1000 method="SHA-1" data=50 sync="subtle": 95,954.52830049273
crypto/webcrypto-digest.js n=1000 method="SHA-256" data=50 sync="subtle": 97,504.15684596673
crypto/webcrypto-digest.js n=1000 method="SHA-384" data=50 sync="subtle": 95,558.87272255732
crypto/webcrypto-digest.js n=1000 method="SHA-512" data=50 sync="subtle": 96,579.1099771436
crypto/webcrypto-digest.js n=1000 method="SHA-1" data=100 sync="subtle": 95,271.41301224582
crypto/webcrypto-digest.js n=1000 method="SHA-256" data=100 sync="subtle": 95,621.24501347159
crypto/webcrypto-digest.js n=1000 method="SHA-384" data=100 sync="subtle": 95,226.87660510853
crypto/webcrypto-digest.js n=1000 method="SHA-512" data=100 sync="subtle": 93,765.51233194642

After (n == 1e5)

crypto/webcrypto-digest.js n=100000 method="SHA-1" data=10 sync="createHash": 415,274.05377886206
crypto/webcrypto-digest.js n=100000 method="SHA-256" data=10 sync="createHash": 417,149.6738452703
crypto/webcrypto-digest.js n=100000 method="SHA-384" data=10 sync="createHash": 376,980.2172355287
crypto/webcrypto-digest.js n=100000 method="SHA-512" data=10 sync="createHash": 372,715.0975820119
crypto/webcrypto-digest.js n=100000 method="SHA-1" data=20 sync="createHash": 430,181.83639962785
crypto/webcrypto-digest.js n=100000 method="SHA-256" data=20 sync="createHash": 419,084.5224332802
crypto/webcrypto-digest.js n=100000 method="SHA-384" data=20 sync="createHash": 376,547.25573734415
crypto/webcrypto-digest.js n=100000 method="SHA-512" data=20 sync="createHash": 370,234.4655255085
crypto/webcrypto-digest.js n=100000 method="SHA-1" data=50 sync="createHash": 423,742.27455861174
crypto/webcrypto-digest.js n=100000 method="SHA-256" data=50 sync="createHash": 395,833.20006948936
crypto/webcrypto-digest.js n=100000 method="SHA-384" data=50 sync="createHash": 379,050.974623447
crypto/webcrypto-digest.js n=100000 method="SHA-512" data=50 sync="createHash": 375,675.6197304685
crypto/webcrypto-digest.js n=100000 method="SHA-1" data=100 sync="createHash": 421,568.2032789735
crypto/webcrypto-digest.js n=100000 method="SHA-256" data=100 sync="createHash": 407,834.3543901419
crypto/webcrypto-digest.js n=100000 method="SHA-384" data=100 sync="createHash": 372,268.2060939776
crypto/webcrypto-digest.js n=100000 method="SHA-512" data=100 sync="createHash": 379,056.31670779944
crypto/webcrypto-digest.js n=100000 method="SHA-1" data=10 sync="subtle": 148,302.19260164732
crypto/webcrypto-digest.js n=100000 method="SHA-256" data=10 sync="subtle": 147,238.89019628844
crypto/webcrypto-digest.js n=100000 method="SHA-384" data=10 sync="subtle": 146,129.28974494382
crypto/webcrypto-digest.js n=100000 method="SHA-512" data=10 sync="subtle": 131,636.1514168684
crypto/webcrypto-digest.js n=100000 method="SHA-1" data=20 sync="subtle": 126,594.01903468177
crypto/webcrypto-digest.js n=100000 method="SHA-256" data=20 sync="subtle": 145,318.21012920764
crypto/webcrypto-digest.js n=100000 method="SHA-384" data=20 sync="subtle": 144,951.76649521643
crypto/webcrypto-digest.js n=100000 method="SHA-512" data=20 sync="subtle": 144,832.58315076123
crypto/webcrypto-digest.js n=100000 method="SHA-1" data=50 sync="subtle": 143,897.2475802149
crypto/webcrypto-digest.js n=100000 method="SHA-256" data=50 sync="subtle": 143,150.88827538266
crypto/webcrypto-digest.js n=100000 method="SHA-384" data=50 sync="subtle": 144,435.63972808863
crypto/webcrypto-digest.js n=100000 method="SHA-512" data=50 sync="subtle": 144,699.0298307714
crypto/webcrypto-digest.js n=100000 method="SHA-1" data=100 sync="subtle": 143,925.53210068002
crypto/webcrypto-digest.js n=100000 method="SHA-256" data=100 sync="subtle": 144,552.87612665596
crypto/webcrypto-digest.js n=100000 method="SHA-384" data=100 sync="subtle": 143,831.10380643434
crypto/webcrypto-digest.js n=100000 method="SHA-512" data=100 sync="subtle": 144,501.80167302856

@lucshi
benchmark: increase the iteration number to an appropriate value
1890810 
Current iteration number is too small that fwrite occupies large
portion of execution time which made crypo execution time measured
inaccurate. The iteration above 1e5 makes 50% higher and stable
score.
@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. crypto Issues and PRs related to the crypto subsystem. labels Nov 17, 2023
debadree25
debadree25 approved these changes Nov 17, 2023
View reviewed changes
Copy link
Member

@debadree25 debadree25 left a comment

Choose a reason for hiding this comment

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

lgtm
does this have any adverse impact on the benchmark running times?

@lucshi
Copy link
Contributor Author

lucshi commented Nov 17, 2023

lgtm does this have any adverse impact on the benchmark running times?

yes, the running duration increased a bit but acceptable. Thanks!

@debadree25 debadree25 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 17, 2023
@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 17, 2023
H4ad
H4ad approved these changes Nov 17, 2023
View reviewed changes
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!

Just to keep the track, ref: #50571

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 19, 2023
@nodejs-github-bot nodejs-github-bot merged commit bae7e38 into nodejs:main Nov 19, 2023
30 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in bae7e38

targos pushed a commit that referenced this pull request Nov 23, 2023
@lucshi @targos
benchmark: increase the iteration number to an appropriate value
c822138 
Current iteration number is too small that fwrite occupies large
portion of execution time which made crypo execution time measured
inaccurate. The iteration above 1e5 makes 50% higher and stable
score.

PR-URL: #50766
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
martenrichter pushed a commit to martenrichter/node that referenced this pull request Nov 26, 2023
@lucshi @martenrichter
benchmark: increase the iteration number to an appropriate value
c79ab13 
Current iteration number is too small that fwrite occupies large
portion of execution time which made crypo execution time measured
inaccurate. The iteration above 1e5 makes 50% higher and stable
score.

PR-URL: nodejs#50766
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
lucshi added a commit to lucshi/node that referenced this pull request Nov 27, 2023
@lucshi
benchmark: increase the iteration number to an appropriate value
a9fa824 
Current iteration number is too small that fwrite occupies large
portion of execution time which made crypo execution time measured
inaccurate. The iteration above 1e5 makes 50% higher and stable
score.

PR-URL: nodejs#50766
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
@RafaelGSS RafaelGSS mentioned this pull request Nov 28, 2023
Merged
RafaelGSS pushed a commit that referenced this pull request Nov 29, 2023
@lucshi @RafaelGSS
benchmark: increase the iteration number to an appropriate value
36380eb 
Current iteration number is too small that fwrite occupies large
portion of execution time which made crypo execution time measured
inaccurate. The iteration above 1e5 makes 50% higher and stable
score.

PR-URL: #50766
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
RafaelGSS pushed a commit that referenced this pull request Nov 30, 2023
@lucshi @RafaelGSS
benchmark: increase the iteration number to an appropriate value
fd3c59c 
Current iteration number is too small that fwrite occupies large
portion of execution time which made crypo execution time measured
inaccurate. The iteration above 1e5 makes 50% higher and stable
score.

PR-URL: #50766
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
@lucshi @UlisesGascon
benchmark: increase the iteration number to an appropriate value
cd1703e 
Current iteration number is too small that fwrite occupies large
portion of execution time which made crypo execution time measured
inaccurate. The iteration above 1e5 makes 50% higher and stable
score.

PR-URL: #50766
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
Merged
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
@lucshi @UlisesGascon
benchmark: increase the iteration number to an appropriate value
00f7a5d 
Current iteration number is too small that fwrite occupies large
portion of execution time which made crypo execution time measured
inaccurate. The iteration above 1e5 makes 50% higher and stable
score.

PR-URL: #50766
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panva panva panva approved these changes

@H4ad H4ad H4ad approved these changes

@debadree25 debadree25 debadree25 approved these changes

Assignees
No one assigned
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. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lucshi @nodejs-github-bot @panva @H4ad @debadree25