Fix small issues on benchmark overhead command#11893
Fix small issues on benchmark overhead command#11893omadoyeabraham wants to merge 6 commits intoparitytech:masterfrom
Conversation
ggwpez
left a comment
ggwpez
left a comment
There was a problem hiding this comment.
Thanks for the work!
I think the JSON storage part can go into a second MR, as it is also not really important right now.
|
|
||
| /// Add a header file to your outputted benchmarks. | ||
| #[clap(long)] | ||
| pub header: Option<PathBuf>, |
There was a problem hiding this comment.
Is there a reason not to put it into the OverheadParams?
I think its cleaner to only pass around the OverheadParams instead of the whole OverheadCmd.
| pub weight_add: u64, | ||
|
|
||
| /// The block weight key | ||
| #[clap(long = "key", default_value = "26aa394eea5630e07c48ae0c9558cef734abf5cb34d6244378cddbf18e849d96")] |
There was a problem hiding this comment.
This key is needed here.
You can probably add it to the BenchmarkParams there.
| @@ -1,3 +1,4 @@ | |||
| {{header}} | |||
| // This file is part of Substrate. | |||
There was a problem hiding this comment.
Line 2 to 17 can now be removed as they are already in the root file HEADER-APACHE2. Then we can actually use the correct copyright header in Polkadot and Cumulus 😆
Same as in utils/frame/benchmarking-cli/src/pallet/template.hbs.
| //! WARMUPS: `{{params.bench.warmup}}`, REPEAT: `{{params.bench.repeat}}` | ||
| //! WEIGHT-PATH: `{{params.weight.weight_path}}` | ||
| //! WEIGHT-METRIC: `{{params.weight.weight_metric}}`, WEIGHT-MUL: `{{params.weight.weight_mul}}`, WEIGHT-ADD: `{{params.weight.weight_add}}` | ||
| //! WASM-EXECUTION-METHOD: `{{execution_strategy}}` |
There was a problem hiding this comment.
I just noticed that this always prints something, even when --execution native is used.
I'm not sure if its possible to print the executor name so Native or WASM instead of the WASM strategy, but would be nice.
| BA: ClientBackend<Block>, | ||
| C: BlockBuilderProvider<BA, Block, C> + ProvideRuntimeApi<Block>, | ||
| C::Api: ApiExt<Block, StateBackend = BA::State> + BlockBuilderApi<Block>, | ||
| <<<Block as BlockT>::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug, |
There was a problem hiding this comment.
I dont see where this is needed, maybe stale code?
ggwpez
added
A3-in_progress
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
2 participants
Fixes paritytech/polkadot-sdk#391