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

fix(cmd bench-omni): build omni-bencher with production profile #7299

Merged
merged 7 commits into from
Jan 24, 2025

Conversation

iulianbarbu
Copy link
Contributor

@iulianbarbu iulianbarbu commented Jan 22, 2025

Description

This PR builds frame-omni-bencher with production profile when calling /cmd bench-omni to compute benchmarks for pallets.
Fix proposed by @bkchr , thanks!

Closes #6797.

Integration

N/A

Review Notes

More info on #6797, and related to how the fix was tested: #6797 (comment).

@iulianbarbu iulianbarbu added the R0-silent Changes should not be mentioned in any release notes label Jan 22, 2025
@iulianbarbu
Copy link
Contributor Author

/cmd bench --runtime westend --pallets pallet_balances

Copy link
Contributor

Command "bench --runtime westend --pallets pallet_balances" has started 🚀 See logs here

Copy link
Contributor

Command "bench --runtime westend --pallets pallet_balances" has finished ✅ See logs here

@iulianbarbu
Copy link
Contributor Author

/cmd bench --runtime westend --pallet pallet_balances

Copy link
Contributor

Command "bench --runtime westend --pallet pallet_balances" has started 🚀 See logs here

Copy link
Contributor

Command "bench --runtime westend --pallet pallet_balances" has finished ✅ See logs here

Subweight results:
File Extrinsic Old New Change [%]
cumulus/pallets/collator-selection/src/weights.rs leave_intent - - ERROR
cumulus/pallets/collator-selection/src/weights.rs new_session - - ERROR
cumulus/pallets/collator-selection/src/weights.rs register_as_candidate - - ERROR
cumulus/pallets/collator-selection/src/weights.rs set_invulnerables - - ERROR
cumulus/pallets/collator-selection/src/weights.rs take_candidate_slot - - ERROR
cumulus/pallets/collator-selection/src/weights.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
polkadot/runtime/westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmen - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmms - - ERROR
polkadot/runtime/westend/src/weights/pallet_balances.rs force_adjust_total_issuance 7.25us 6.86us -5.27
Command output:

✅ Successful benchmarks of runtimes/pallets:
-- westend: ['pallet_balances']

@iulianbarbu
Copy link
Contributor Author

/cmd bench-omni --runtime westend --pallet pallet_balances

Copy link
Contributor

Command "bench-omni --runtime westend --pallet pallet_balances" has started 🚀 See logs here

Copy link
Contributor

Command "bench-omni --runtime westend --pallet pallet_balances" has finished ✅ See logs here

Subweight results:
File Extrinsic Old New Change [%]
cumulus/pallets/collator-selection/src/weights.rs leave_intent - - ERROR
cumulus/pallets/collator-selection/src/weights.rs new_session - - ERROR
cumulus/pallets/collator-selection/src/weights.rs register_as_candidate - - ERROR
cumulus/pallets/collator-selection/src/weights.rs set_invulnerables - - ERROR
cumulus/pallets/collator-selection/src/weights.rs take_candidate_slot - - ERROR
cumulus/pallets/collator-selection/src/weights.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
polkadot/runtime/westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmen - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmms - - ERROR
polkadot/runtime/westend/src/weights/pallet_balances.rs burn_keep_alive 21.76us 29.60us +36.02
polkadot/runtime/westend/src/weights/pallet_balances.rs burn_allow_death 32.25us 42.37us +31.36
polkadot/runtime/westend/src/weights/pallet_balances.rs force_adjust_total_issuance 7.25us 9.19us +26.81
polkadot/runtime/westend/src/weights/pallet_balances.rs transfer_all 176.42us 192.32us +9.02
polkadot/runtime/westend/src/weights/pallet_balances.rs transfer_allow_death 177.84us 193.19us +8.63
polkadot/runtime/westend/src/weights/pallet_balances.rs transfer_keep_alive 166.41us 178.96us +7.54
polkadot/runtime/westend/src/weights/pallet_balances.rs force_transfer 304.81us 321.03us +5.32
Command output:

✅ Successful benchmarks of runtimes/pallets:
-- westend: ['pallet_balances']

@iulianbarbu iulianbarbu self-assigned this Jan 24, 2025
@iulianbarbu iulianbarbu marked this pull request as ready for review January 24, 2025 08:25
@iulianbarbu iulianbarbu requested review from a team as code owners January 24, 2025 08:25
@iulianbarbu iulianbarbu changed the title Test benchmarks fix: build omni-bencher with production profile Jan 24, 2025
@iulianbarbu
Copy link
Contributor Author

@iulianbarbu if I compare 2 commits b/w each other - i see that bench-omni is still 30% worse as it was before

https://weights.tasty.limo/compare?repo=polkadot-sdk&threshold=10&path_pattern=substrate%2Fframe%2F**%2Fsrc%2Fweights.rs%2Cpolkadot%2Fruntime%2F*%2Fsrc%2Fweights%2F**%2F*.rs%2Cpolkadot%2Fbridges%2Fmodules%2F*%2Fsrc%2Fweights.rs%2Ccumulus%2F**%2Fweights%2F*.rs%2Ccumulus%2F**%2Fweights%2Fxcm%2F*.rs%2Ccumulus%2F**%2Fsrc%2Fweights.rs&method=asymptotic&ignore_errors=true&unit=time&old=f4104066615364858ac17f19de168b6b50c236cc&new=c6c03daf63cc000584aeee3648c848dd6b5ea26b

with threshold 2 it shows all of them degraded

Looks like the commits are from paritytech/polkadot-sdk. Wouldn't the comparisons use the omni-bencher built w/o the prod profile, so the results make sense? In any case, seems like the commits refer to my previous attempts to reproduce your issue on this branch, on this repo, on this org, while my actual fixing/testing happened on paritytech-stg.

Fixing master on paritytech is based on the results of the tests from paritytech-stg.

@mordamax
Copy link
Contributor

mordamax commented Jan 24, 2025

@iulianbarbu you're right! I didn't think that it's a context of paritytech and this change is still pulling from master
so technically didn't make sense to compare it in this PR

@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 24, 2025 11:22
@iulianbarbu iulianbarbu changed the title fix: build omni-bencher with production profile fix(cmd bench-omni): build omni-bencher with production profile Jan 24, 2025
@iulianbarbu iulianbarbu added this pull request to the merge queue Jan 24, 2025
Merged via the queue into master with commit a2c63e8 Jan 24, 2025
273 of 347 checks passed
@iulianbarbu iulianbarbu deleted the test-benchmarks branch January 24, 2025 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frame-omni-bencher weights are worse than old bench
4 participants