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

Tweak Cadence bench to be more stable #1290

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

janezpodhostnik
Copy link
Contributor

@janezpodhostnik janezpodhostnik commented Dec 2, 2021

Description

Related to onflow/flow-go#1722

Bench tests were being run grouped. Given test A and B with shuffle on and count 3 they would either run like this AAA BBB or like this BBB AAA.

This fix changes the use of the -count flag into a for loop. Which means all test are ran once in a shuffled order, and then this repeats N times.

This tweak causes the uncertainty in the results to go up, which in turn causes the prediction of how much the performance changes to be more conservative.

Example:

name                                              old time/op    new time/op    delta
RuntimeResourceDictionaryValues-8                   17.8ms ±34%    14.5ms ± 7%   ~     (p=1.000 n=3+3)
RuntimeFungibleTokenTransfer-8                      1.11ms ±21%    1.03ms ±11%   ~     (p=0.700 n=3+3)
ParseFungibleToken-8                                 578µs ±14%     534µs ±14%   ~     (p=0.400 n=3+3)
ParseDeploy/byte_array-8                            42.7ms ± 9%    40.2ms ±19%   ~     (p=0.700 n=3+3)
ParseDeploy/decode_hex-8                            1.28ms ±21%    1.09ms ±12%   ~     (p=0.400 n=3+3)
ParseArray-8                                        26.5ms ±11%    25.0ms ± 7%   ~     (p=1.000 n=3+3)
ParseInfix-8                                        28.6µs ± 4%    25.8µs ± 9%   ~     (p=0.100 n=3+3)
QualifiedIdentifierCreation/One_level-8             2.91ns ±18%    2.56ns ± 3%   ~     (p=0.400 n=3+3)
QualifiedIdentifierCreation/Three_levels-8           121ns ± 8%     114ns ± 8%   ~     (p=0.400 n=3+3)
CheckContractInterfaceFungibleTokenConformance-8     176µs ±13%     145µs ±12%   ~     (p=0.100 n=3+3)
ContractInterfaceFungibleToken-8                    51.3µs ±14%    44.4µs ± 6%   ~     (p=0.200 n=3+3)
NewInterpreter/new_interpreter-8                    1.22µs ±16%    1.07µs ± 6%   ~     (p=0.400 n=3+3)
NewInterpreter/new_sub-interpreter-8                2.19µs ±15%    1.94µs ± 1%   ~     (p=0.200 n=3+3)
InterpretRecursionFib-8                             2.82ms ±12%    2.55ms ± 3%   ~     (p=0.700 n=3+3)

name                                              old alloc/op   new alloc/op   delta
RuntimeResourceDictionaryValues-8                   4.34MB ± 0%    4.34MB ± 0%   ~     (p=0.400 n=3+3)
RuntimeFungibleTokenTransfer-8                       239kB ± 0%     239kB ± 0%   ~     (p=0.700 n=3+3)
QualifiedIdentifierCreation/One_level-8              0.00B          0.00B        ~     (all equal)
QualifiedIdentifierCreation/Three_levels-8           64.0B ± 0%     64.0B ± 0%   ~     (all equal)
CheckContractInterfaceFungibleTokenConformance-8    65.7kB ± 0%    65.7kB ± 0%   ~     (p=1.000 n=3+3)
ContractInterfaceFungibleToken-8                    26.5kB ± 0%    26.5kB ± 0%   ~     (all equal)
NewInterpreter/new_interpreter-8                      720B ± 0%      720B ± 0%   ~     (all equal)
NewInterpreter/new_sub-interpreter-8                1.11kB ± 0%    1.11kB ± 0%   ~     (all equal)
InterpretRecursionFib-8                             1.24MB ± 0%    1.24MB ± 0%   ~     (p=1.000 n=3+3)

name                                              old allocs/op  new allocs/op  delta
RuntimeResourceDictionaryValues-8                     108k ± 0%      108k ± 0%   ~     (p=1.000 n=3+3)
RuntimeFungibleTokenTransfer-8                       4.54k ± 0%     4.54k ± 0%   ~     (p=1.000 n=3+3)
QualifiedIdentifierCreation/One_level-8               0.00           0.00        ~     (all equal)
QualifiedIdentifierCreation/Three_levels-8            2.00 ± 0%      2.00 ± 0%   ~     (all equal)
CheckContractInterfaceFungibleTokenConformance-8     1.07k ± 0%     1.07k ± 0%   ~     (all equal)
ContractInterfaceFungibleToken-8                       457 ± 0%       457 ± 0%   ~     (all equal)
NewInterpreter/new_interpreter-8                      11.0 ± 0%      11.0 ± 0%   ~     (all equal)
NewInterpreter/new_sub-interpreter-8                  32.0 ± 0%      32.0 ± 0%   ~     (all equal)
InterpretRecursionFib-8                              25.0k ± 0%     25.0k ± 0%   ~     (all equal)

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@github-actions
Copy link

github-actions bot commented Dec 2, 2021

Cadence Benchstat comparison

This branch with compared with the base branch onflow:master commit cf11440
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Results

old.txtnew.txt
time/opdelta
ParseArray-219.7ms ± 0%19.9ms ± 1%+1.19%(p=0.004 n=7+7)
RuntimeResourceDictionaryValues-215.3ms ± 2%15.3ms ± 2%~(p=0.535 n=7+7)
RuntimeFungibleTokenTransfer-21.26ms ±21%1.32ms ±17%~(p=0.620 n=7+7)
ParseDeploy/byte_array-230.0ms ± 3%29.9ms ± 1%~(p=0.836 n=7+6)
ParseDeploy/decode_hex-21.20ms ± 3%1.19ms ± 1%~(p=0.620 n=7+7)
ParseFungibleToken-2405µs ± 0%405µs ± 1%~(p=0.731 n=6+7)
ParseInfix-221.3µs ± 0%21.3µs ± 0%~(p=0.165 n=7+7)
QualifiedIdentifierCreation/One_level-22.68ns ± 0%2.68ns ± 0%~(p=0.544 n=7+7)
QualifiedIdentifierCreation/Three_levels-2138ns ± 2%138ns ± 1%~(p=0.808 n=7+6)
ContractInterfaceFungibleToken-240.1µs ± 1%40.2µs ± 2%~(p=0.836 n=7+6)
CheckContractInterfaceFungibleTokenConformance-2136µs ± 5%134µs ± 2%~(p=0.620 n=7+7)
NewInterpreter/new_interpreter-21.01µs ± 3%1.01µs ± 2%~(p=0.916 n=7+6)
NewInterpreter/new_sub-interpreter-21.88µs ± 3%1.88µs ± 1%~(p=0.731 n=7+6)
InterpretRecursionFib-22.45ms ± 1%2.48ms ± 4%~(p=0.945 n=6+7)
 
alloc/opdelta
RuntimeResourceDictionaryValues-24.34MB ± 0%4.34MB ± 0%~(p=1.000 n=7+7)
RuntimeFungibleTokenTransfer-2238kB ± 0%238kB ± 0%~(p=0.056 n=7+7)
QualifiedIdentifierCreation/One_level-20.00B 0.00B ~(all equal)
QualifiedIdentifierCreation/Three_levels-264.0B ± 0%64.0B ± 0%~(all equal)
ContractInterfaceFungibleToken-226.5kB ± 0%26.5kB ± 0%~(all equal)
CheckContractInterfaceFungibleTokenConformance-265.7kB ± 0%65.7kB ± 0%~(p=1.000 n=7+7)
NewInterpreter/new_interpreter-2720B ± 0%720B ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-21.11kB ± 0%1.11kB ± 0%~(all equal)
InterpretRecursionFib-21.24MB ± 0%1.24MB ± 0%~(all equal)
 
allocs/opdelta
RuntimeResourceDictionaryValues-2108k ± 0%108k ± 0%~(p=1.000 n=7+7)
RuntimeFungibleTokenTransfer-24.53k ± 0%4.53k ± 0%~(p=0.735 n=7+7)
QualifiedIdentifierCreation/One_level-20.00 0.00 ~(all equal)
QualifiedIdentifierCreation/Three_levels-22.00 ± 0%2.00 ± 0%~(all equal)
ContractInterfaceFungibleToken-2457 ± 0%457 ± 0%~(all equal)
CheckContractInterfaceFungibleTokenConformance-21.07k ± 0%1.07k ± 0%~(all equal)
NewInterpreter/new_interpreter-211.0 ± 0%11.0 ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-232.0 ± 0%32.0 ± 0%~(all equal)
InterpretRecursionFib-225.0k ± 0%25.0k ± 0%~(all equal)
 

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

@janezpodhostnik janezpodhostnik merged commit 141b10c into master Dec 2, 2021
@janezpodhostnik janezpodhostnik deleted the janez/cadence-bench-tweak branch May 11, 2022 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants