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

Per runtime benchmarks #490

Merged
merged 54 commits into from
Apr 18, 2024
Merged

Per runtime benchmarks #490

merged 54 commits into from
Apr 18, 2024

Conversation

girazoki
Copy link
Collaborator

@girazoki girazoki commented Apr 9, 2024

Adds specific benchmarks for each of the runtimes we host: flashbox, dancebox, simple-template and frontier-template. In order to do this:

  • I added a new benchmarking template for runtimes. This one is different from the pallet one.
  • I added new arguments to our benchmarking tool. I will write a guide explaining how to use it.
  • I had to fix several parts of the benchmarking code that was broken

As I advanced on this, I realized that our frontier template was not applying the 1e6 multiplier to all deposits/fees. I also fixed this plus adapted tests.

MIssing:

  • Add complete guide on how to benchmark pallets & runtimes
  • Fix request_delegate benchmark (One db read missing)
  • Wire frame-system
  • Benchmark again in benchmarking server
  • Fix XCM
  • Cleanup

Copy link
Contributor

github-actions bot commented Apr 9, 2024

Coverage Report

(master)

@@                       Coverage Diff                       @@
##           master   girazoki-per-runtime-benchs      +/-   ##
===============================================================
- Coverage   65.38%                        65.36%   -0.02%     
  Files          68                            68              
+ Lines       10000                         10001       +1     
===============================================================
- Hits         6538                          6537       -1     
+ Misses       3462                          3464       +2     
Files Changed Coverage
/pallets/invulnerables/src/lib.rs 67.89% (-0.63%) 🔽
/pallets/invulnerables/src/weights.rs 25.56% (-1.11%) 🔽

Coverage generated Thu Apr 18 11:26:16 UTC 2024

@girazoki girazoki added D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes labels Apr 10, 2024
@girazoki girazoki requested a review from ParthDesai April 12, 2024 12:02
Copy link
Contributor

@fgamundi fgamundi left a comment

Choose a reason for hiding this comment

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

Looks good. Nice doc!

docs/benchmarking.md Outdated Show resolved Hide resolved
@tmpolaczyk
Copy link
Contributor

tmpolaczyk commented Apr 17, 2024

Using rg 'weights::', I can see some pallets are not using the per runtime weights, is this expected?

pallet_session and pallet_treasury in dancebox, many pallets in flashbox, and some pallets in templates:

container-chains/templates/simple/runtime/src/lib.rs
415:    type WeightInfo = cumulus_pallet_parachain_system::weights::SubstrateWeight<Runtime>;
614:    type TimestampWeights = pallet_timestamp::weights::SubstrateWeight<Runtime>;
615:    type AuthorInherentWeights = pallet_author_inherent::weights::SubstrateWeight<Runtime>;
616:    type AuthoritiesNotingWeights = pallet_cc_authorities_noting::weights::SubstrateWeight<Runtime>;

container-chains/templates/frontier/runtime/src/lib.rs
807:    type WeightInfo = pallet_hotfix_sufficients::weights::SubstrateWeight<Runtime>;
826:    type TimestampWeights = pallet_timestamp::weights::SubstrateWeight<Runtime>;
827:    type AuthorInherentWeights = pallet_author_inherent::weights::SubstrateWeight<Runtime>;
828:    type AuthoritiesNotingWeights = pallet_cc_authorities_noting::weights::SubstrateWeight<Runtime>;

runtime/dancebox/src/lib.rs
708:    type WeightInfo = pallet_session::weights::SubstrateWeight<Runtime>;
1602:    type WeightInfo = pallet_treasury::weights::SubstrateWeight<Runtime>;


runtime/flashbox/src/lib.rs
52:        weights::{
339:    type SystemWeightInfo = weights::frame_system::SubstrateWeight<Runtime>;
360:    type WeightInfo = weights::pallet_timestamp::SubstrateWeight<Runtime>;
392:    type WeightInfo = weights::pallet_author_inherent::SubstrateWeight<Runtime>;
415:    type WeightInfo = weights::pallet_balances::SubstrateWeight<Runtime>;
481:    type WeightInfo = weights::cumulus_pallet_parachain_system::SubstrateWeight<Runtime>;
602:    type WeightInfo = pallet_session::weights::SubstrateWeight<Runtime>;
733:    type WeightInfo = weights::pallet_collator_assignment::SubstrateWeight<Runtime>;
783:    type WeightInfo = pallet_services_payment::weights::SubstrateWeight<Runtime>;
792:    type WeightInfo = pallet_data_preservers::weights::SubstrateWeight<Runtime>;
807:    type WeightInfo = pallet_author_noting::weights::SubstrateWeight<Runtime>;
826:    type WeightInfo = pallet_invulnerables::weights::SubstrateWeight<Runtime>;
852:    type WeightInfo = pallet_configuration::weights::SubstrateWeight<Runtime>;
917:    type WeightInfo = pallet_registrar::weights::SubstrateWeight<Runtime>;
929:    type WeightInfo = pallet_sudo::weights::SubstrateWeight<Runtime>;
936:    type WeightInfo = pallet_utility::weights::SubstrateWeight<Runtime>;
1046:    type WeightInfo = pallet_proxy::weights::SubstrateWeight<Runtime>;
1164:    type WeightInfo = weights::pallet_tx_pause::SubstrateWeight<Runtime>;
1305:    type WeightInfo = weights::pallet_stream_payment::SubstrateWeight<Runtime>;
1339:    type WeightInfo = weights::pallet_identity::SubstrateWeight<Runtime>;
1364:    type WeightInfo = pallet_treasury::weights::SubstrateWeight<Runtime>;
1397:    type WeightInfo = weights::pallet_multisig::SubstrateWeight<Runtime>;

// inject it into pallet-foreign-asset-creator.
let asset_location = MultiLocation::new(
0,
X2(PalletInstance(50), GeneralIndex(u32::from(asset_id).into()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we define some constant and use it? That would ensure nobody accidentally modify this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will look into this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

would you want this to be somewhere common to all runtimes? If so, I think this would leave in dancekit, and I would need to cherry-pick the change here. Maybe it's a cleanup we can do in a follow up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My idea actually is that we generate in foreign asset creator a benchmarking code that is called: register_foreign_asset_for_benchmarking which would abstract all this. But again I think we can leave this for future PR

@ParthDesai
Copy link
Contributor

How does benchmark of a pallet as part of runtime and standalone differs? Apart from the configuration difference actual code is more or less same.

@ParthDesai
Copy link
Contributor

Using rg 'weights::', I can see some pallets are not using the per runtime weights, is this expected?

pallet_session and pallet_treasury in dancebox, many pallets in flashbox, and some pallets in templates:

container-chains/templates/simple/runtime/src/lib.rs
415:    type WeightInfo = cumulus_pallet_parachain_system::weights::SubstrateWeight<Runtime>;
614:    type TimestampWeights = pallet_timestamp::weights::SubstrateWeight<Runtime>;
615:    type AuthorInherentWeights = pallet_author_inherent::weights::SubstrateWeight<Runtime>;
616:    type AuthoritiesNotingWeights = pallet_cc_authorities_noting::weights::SubstrateWeight<Runtime>;

container-chains/templates/frontier/runtime/src/lib.rs
807:    type WeightInfo = pallet_hotfix_sufficients::weights::SubstrateWeight<Runtime>;
826:    type TimestampWeights = pallet_timestamp::weights::SubstrateWeight<Runtime>;
827:    type AuthorInherentWeights = pallet_author_inherent::weights::SubstrateWeight<Runtime>;
828:    type AuthoritiesNotingWeights = pallet_cc_authorities_noting::weights::SubstrateWeight<Runtime>;

runtime/dancebox/src/lib.rs
708:    type WeightInfo = pallet_session::weights::SubstrateWeight<Runtime>;
1602:    type WeightInfo = pallet_treasury::weights::SubstrateWeight<Runtime>;


runtime/flashbox/src/lib.rs
52:        weights::{
339:    type SystemWeightInfo = weights::frame_system::SubstrateWeight<Runtime>;
360:    type WeightInfo = weights::pallet_timestamp::SubstrateWeight<Runtime>;
392:    type WeightInfo = weights::pallet_author_inherent::SubstrateWeight<Runtime>;
415:    type WeightInfo = weights::pallet_balances::SubstrateWeight<Runtime>;
481:    type WeightInfo = weights::cumulus_pallet_parachain_system::SubstrateWeight<Runtime>;
602:    type WeightInfo = pallet_session::weights::SubstrateWeight<Runtime>;
733:    type WeightInfo = weights::pallet_collator_assignment::SubstrateWeight<Runtime>;
783:    type WeightInfo = pallet_services_payment::weights::SubstrateWeight<Runtime>;
792:    type WeightInfo = pallet_data_preservers::weights::SubstrateWeight<Runtime>;
807:    type WeightInfo = pallet_author_noting::weights::SubstrateWeight<Runtime>;
826:    type WeightInfo = pallet_invulnerables::weights::SubstrateWeight<Runtime>;
852:    type WeightInfo = pallet_configuration::weights::SubstrateWeight<Runtime>;
917:    type WeightInfo = pallet_registrar::weights::SubstrateWeight<Runtime>;
929:    type WeightInfo = pallet_sudo::weights::SubstrateWeight<Runtime>;
936:    type WeightInfo = pallet_utility::weights::SubstrateWeight<Runtime>;
1046:    type WeightInfo = pallet_proxy::weights::SubstrateWeight<Runtime>;
1164:    type WeightInfo = weights::pallet_tx_pause::SubstrateWeight<Runtime>;
1305:    type WeightInfo = weights::pallet_stream_payment::SubstrateWeight<Runtime>;
1339:    type WeightInfo = weights::pallet_identity::SubstrateWeight<Runtime>;
1364:    type WeightInfo = pallet_treasury::weights::SubstrateWeight<Runtime>;
1397:    type WeightInfo = weights::pallet_multisig::SubstrateWeight<Runtime>;

+1

@girazoki
Copy link
Collaborator Author

girazoki commented Apr 18, 2024

Using rg 'weights::', I can see some pallets are not using the per runtime weights, is this expected?

pallet_session and pallet_treasury in dancebox, many pallets in flashbox, and some pallets in templates:

container-chains/templates/simple/runtime/src/lib.rs
415:    type WeightInfo = cumulus_pallet_parachain_system::weights::SubstrateWeight<Runtime>;
614:    type TimestampWeights = pallet_timestamp::weights::SubstrateWeight<Runtime>;
615:    type AuthorInherentWeights = pallet_author_inherent::weights::SubstrateWeight<Runtime>;
616:    type AuthoritiesNotingWeights = pallet_cc_authorities_noting::weights::SubstrateWeight<Runtime>;

container-chains/templates/frontier/runtime/src/lib.rs
807:    type WeightInfo = pallet_hotfix_sufficients::weights::SubstrateWeight<Runtime>;
826:    type TimestampWeights = pallet_timestamp::weights::SubstrateWeight<Runtime>;
827:    type AuthorInherentWeights = pallet_author_inherent::weights::SubstrateWeight<Runtime>;
828:    type AuthoritiesNotingWeights = pallet_cc_authorities_noting::weights::SubstrateWeight<Runtime>;

runtime/dancebox/src/lib.rs
708:    type WeightInfo = pallet_session::weights::SubstrateWeight<Runtime>;
1602:    type WeightInfo = pallet_treasury::weights::SubstrateWeight<Runtime>;


runtime/flashbox/src/lib.rs
52:        weights::{
339:    type SystemWeightInfo = weights::frame_system::SubstrateWeight<Runtime>;
360:    type WeightInfo = weights::pallet_timestamp::SubstrateWeight<Runtime>;
392:    type WeightInfo = weights::pallet_author_inherent::SubstrateWeight<Runtime>;
415:    type WeightInfo = weights::pallet_balances::SubstrateWeight<Runtime>;
481:    type WeightInfo = weights::cumulus_pallet_parachain_system::SubstrateWeight<Runtime>;
602:    type WeightInfo = pallet_session::weights::SubstrateWeight<Runtime>;
733:    type WeightInfo = weights::pallet_collator_assignment::SubstrateWeight<Runtime>;
783:    type WeightInfo = pallet_services_payment::weights::SubstrateWeight<Runtime>;
792:    type WeightInfo = pallet_data_preservers::weights::SubstrateWeight<Runtime>;
807:    type WeightInfo = pallet_author_noting::weights::SubstrateWeight<Runtime>;
826:    type WeightInfo = pallet_invulnerables::weights::SubstrateWeight<Runtime>;
852:    type WeightInfo = pallet_configuration::weights::SubstrateWeight<Runtime>;
917:    type WeightInfo = pallet_registrar::weights::SubstrateWeight<Runtime>;
929:    type WeightInfo = pallet_sudo::weights::SubstrateWeight<Runtime>;
936:    type WeightInfo = pallet_utility::weights::SubstrateWeight<Runtime>;
1046:    type WeightInfo = pallet_proxy::weights::SubstrateWeight<Runtime>;
1164:    type WeightInfo = weights::pallet_tx_pause::SubstrateWeight<Runtime>;
1305:    type WeightInfo = weights::pallet_stream_payment::SubstrateWeight<Runtime>;
1339:    type WeightInfo = weights::pallet_identity::SubstrateWeight<Runtime>;
1364:    type WeightInfo = pallet_treasury::weights::SubstrateWeight<Runtime>;
1397:    type WeightInfo = weights::pallet_multisig::SubstrateWeight<Runtime>;

For pallet-session, there is really no benchmark we can do for now. The benchmarking code depends on pallet-session-historical and pallet-staking that we dont have. For the rest that you mention, I will take a look, it's probably that I forgot to add them.

For pallet-hotfix-sufficients, I believe this is a pallet we should get rid of as it was introduced in moonbeam to fix a bug a long time ago. So it does not really matter if it uses official weights or not I would say, as it is going to dissaper

@girazoki
Copy link
Collaborator Author

How does benchmark of a pallet as part of runtime and standalone differs? Apart from the configuration difference actual code is more or less same.

Different runtimes might have different hooks for the same pallet. The benchmarking code is the same, but the surrounding conditions are different. And for each runtime, we can have different weights associated.

What we call the "standalone" pallet benchmarking is basically a benchmarking that we dont trully care (as long as it's not generated against a mocked runtime obviously) against which runtime is used, nor if it is generated in the benchmarking server. It's only to generate the WeightInfo trait and some reference weights. Tipically I benchmark "standalone pallets" against dancebox, but this is just a choice.

@girazoki girazoki merged commit faf9e75 into master Apr 18, 2024
32 checks passed
@girazoki girazoki deleted the girazoki-per-runtime-benchs branch April 18, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants