Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion utils/frame/benchmarking-cli/src/overhead/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Each time an extrinsic or a block is executed, a fixed weight is charged as "execution overhead".
This is necessary since the weight that is calculated by the pallet benchmarks does not include this overhead.
The exact overhead to can vary per Substrate chain and needs to be calculated per chain.
The exact overhead can vary per Substrate chain and needs to be calculated per chain.
This command calculates the exact values of these overhead weights for any Substrate chain that supports it.

## How does it work?
Expand Down
19 changes: 15 additions & 4 deletions utils/frame/benchmarking-cli/src/overhead/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ use sc_cli::{CliConfiguration, ImportParams, Result, SharedParams};
use sc_client_api::Backend as ClientBackend;
use sc_service::Configuration;
use sp_api::{ApiExt, ProvideRuntimeApi};
use sp_runtime::{traits::Block as BlockT, OpaqueExtrinsic};
use sp_runtime::{traits::{ Block as BlockT, Header as HeaderT}, OpaqueExtrinsic};

use clap::{Args, Parser};
use log::info;
use serde::Serialize;
use std::{fmt::Debug, sync::Arc};
use std::{fmt::Debug, sync::Arc, path::PathBuf};

use crate::{
overhead::{
Expand All @@ -52,6 +52,10 @@ pub struct OverheadCmd {
#[allow(missing_docs)]
#[clap(flatten)]
pub params: OverheadParams,

/// Add a header file to your outputted benchmarks.
#[clap(long)]
pub header: Option<PathBuf>,
Copy link
Member

Choose a reason for hiding this comment

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

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.

}

/// Configures the benchmark, the post-processing and weight generation.
Expand Down Expand Up @@ -96,21 +100,28 @@ impl OverheadCmd {
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,
Copy link
Member

Choose a reason for hiding this comment

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

I dont see where this is needed, maybe stale code?

{
if let Some(header_file) = &self.header {
if !header_file.is_file() {
return Err("Header file is invalid!".into())
};
}

let bench = Benchmark::new(client, self.params.bench.clone(), inherent_data, ext_builder);

// per-block execution overhead
{
let stats = bench.bench(BenchmarkType::Block)?;
info!("Per-block execution overhead [ns]:\n{:?}", stats);
let template = TemplateData::new(BenchmarkType::Block, &cfg, &self.params, &stats)?;
let template = TemplateData::new(BenchmarkType::Block, &cfg, &self, &stats)?;
template.write(&self.params.weight.weight_path)?;
}
// per-extrinsic execution overhead
{
let stats = bench.bench(BenchmarkType::Extrinsic)?;
info!("Per-extrinsic execution overhead [ns]:\n{:?}", stats);
let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self.params, &stats)?;
let template = TemplateData::new(BenchmarkType::Extrinsic, &cfg, &self, &stats)?;
template.write(&self.params.weight.weight_path)?;
}

Expand Down
22 changes: 17 additions & 5 deletions utils/frame/benchmarking-cli/src/overhead/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@ use log::info;
use serde::Serialize;
use std::{env, fs, path::PathBuf};

use crate::{
overhead::{bench::BenchmarkType, cmd::OverheadParams},
shared::{Stats, UnderscoreHelper},
};
use crate::{overhead::{bench::BenchmarkType, cmd::OverheadParams}, OverheadCmd, shared::{Stats, UnderscoreHelper}};

static VERSION: &str = env!("CARGO_PKG_VERSION");
static TEMPLATE: &str = include_str!("./weights.hbs");
Expand Down Expand Up @@ -59,18 +56,31 @@ pub(crate) struct TemplateData {
stats: Stats,
/// The resulting weight in ns.
weight: u64,
/// Contents of the header file provided in the params of the executed command
header: String,
/// The Execution Strategy used
execution_strategy: String,
}

impl TemplateData {
/// Returns a new [`Self`] from the given params.
pub(crate) fn new(
t: BenchmarkType,
cfg: &Configuration,
params: &OverheadParams,
cmd: &OverheadCmd,
stats: &Stats,
) -> Result<Self> {
let params = &cmd.params;
let weight = params.weight.calc_weight(stats)?;

let header_text = match &cmd.header {
Some(header_file) => {
let text = fs::read_to_string(header_file)?;
text
},
None => String::new(),
};

Ok(TemplateData {
short_name: t.short_name().into(),
long_name: t.long_name().into(),
Expand All @@ -83,6 +93,8 @@ impl TemplateData {
params: params.clone(),
stats: stats.clone(),
weight,
header: header_text.clone(),
execution_strategy: cmd.import_params.wasm_method.to_string()
})
}

Expand Down
2 changes: 2 additions & 0 deletions utils/frame/benchmarking-cli/src/overhead/weights.hbs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{{header}}
// This file is part of Substrate.
Copy link
Member

Choose a reason for hiding this comment

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

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.


// Copyright (C) 2022 Parity Technologies (UK) Ltd.
Expand All @@ -23,6 +24,7 @@
//! 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}}`
Copy link
Member

Choose a reason for hiding this comment

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

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.


// Executed Command:
{{#each args as |arg|}}
Expand Down
4 changes: 4 additions & 0 deletions utils/frame/benchmarking-cli/src/shared/weight_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ pub struct WeightParams {
/// Is applied after `weight_mul`.
#[clap(long = "add", default_value = "0")]
pub weight_add: u64,

/// The block weight key
#[clap(long = "key", default_value = "26aa394eea5630e07c48ae0c9558cef734abf5cb34d6244378cddbf18e849d96")]
Copy link
Member

Choose a reason for hiding this comment

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

This key is needed here.
You can probably add it to the BenchmarkParams there.

pub block_weight_key: String,
}

/// Calculates the final weight by multiplying the selected metric with
Expand Down