Constant Weight Signature in Benchmarking CLI (#7233)

* Don't remove unused components

* add more metadata to generated file

* unused code

* proposed format

* Revert "proposed format"

This reverts commit ce522c3480157ab6670bcbd9b17e1398168cabf4.

* Update weight signatures and unused components in benchmarks

* Keep timestamp constant time

* remove component from as_derivative

substrate/bin/node/runtime/src/weights/frame_system.rs

@@ -24,15 +24,13 @@ use sp_std::marker::PhantomData;
 
 pub struct WeightInfo<T>(PhantomData<T>);
 impl<T: frame_system::Trait> frame_system::WeightInfo for WeightInfo<T> {
-	// WARNING! Some components were not used: ["b"]
-	fn remark() -> Weight {
+	fn remark(_b: u32) -> Weight {
 		(1305000 as Weight)
 	}
 	fn set_heap_pages() -> Weight {
 		(2023000 as Weight)
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
-	// WARNING! Some components were not used: ["d"]
 	fn set_changes_trie_config() -> Weight {
 		(10026000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))

substrate/bin/node/runtime/src/weights/pallet_timestamp.rs

@@ -22,13 +22,11 @@ use sp_std::marker::PhantomData;
 
 pub struct WeightInfo<T>(PhantomData<T>);
 impl<T: frame_system::Trait> pallet_timestamp::WeightInfo for WeightInfo<T> {
-	// WARNING! Some components were not used: ["t"]
 	fn set() -> Weight {
 		(9133000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(2 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
-	// WARNING! Some components were not used: ["t"]
 	fn on_finalize() -> Weight {
 		(5915000 as Weight)
 	}

substrate/bin/node/runtime/src/weights/pallet_treasury.rs

@@ -46,7 +46,6 @@ impl<T: frame_system::Trait> pallet_treasury::WeightInfo for WeightInfo<T> {
 			.saturating_add(T::DbWeight::get().reads(2 as Weight))
 			.saturating_add(T::DbWeight::get().writes(2 as Weight))
 	}
-	// WARNING! Some components were not used: ["r"]
 	fn retract_tip() -> Weight {
 		(82970000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))

substrate/bin/node/runtime/src/weights/pallet_utility.rs

@@ -29,7 +29,6 @@ impl<T: frame_system::Trait> pallet_utility::WeightInfo for WeightInfo<T> {
 		(16461000 as Weight)
 			.saturating_add((1982000 as Weight).saturating_mul(c as Weight))
 	}
-	// WARNING! Some components were not used: ["u"]
 	fn as_derivative() -> Weight {
 		(4086000 as Weight)
 	}

substrate/frame/system/benchmarking/src/lib.rs

@@ -61,7 +61,7 @@ benchmarks! {
 	}
 
 	set_changes_trie_config {
-		let d in 0 .. 1000;
+		let d = 1000;
 
 		let digest_item = DigestItemOf::<T>::Other(vec![]);
 

substrate/frame/system/src/default_weights.rs

@@ -22,15 +22,13 @@
 use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight};
 
 impl crate::WeightInfo for () {
-	// WARNING! Some components were not used: ["b"]
-	fn remark() -> Weight {
+	fn remark(_b: u32) -> Weight {
 		(1305000 as Weight)
 	}
 	fn set_heap_pages() -> Weight {
 		(2023000 as Weight)
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
-	// WARNING! Some components were not used: ["d"]
 	fn set_changes_trie_config() -> Weight {
 		(10026000 as Weight)
 			.saturating_add(DbWeight::get().reads(1 as Weight))

substrate/frame/system/src/lib.rs

@@ -160,7 +160,7 @@ pub fn extrinsics_data_root<H: Hash>(xts: Vec<Vec<u8>>) -> H::Output {
 }
 
 pub trait WeightInfo {
-	fn remark() -> Weight;
+	fn remark(b: u32) -> Weight;
 	fn set_heap_pages() -> Weight;
 	fn set_changes_trie_config() -> Weight;
 	fn set_storage(i: u32, ) -> Weight;
@@ -570,7 +570,7 @@ decl_module! {
 		/// - Base Weight: 0.665 µs, independent of remark length.
 		/// - No DB operations.
 		/// # </weight>
-		#[weight = T::SystemWeightInfo::remark()]
+		#[weight = T::SystemWeightInfo::remark(_remark.len() as u32)]
 		fn remark(origin, _remark: Vec<u8>) {
 			ensure_signed(origin)?;
 		}

substrate/frame/timestamp/src/benchmarking.rs

@@ -33,7 +33,7 @@ benchmarks! {
 	_ { }
 
 	set {
-		let t in 1 .. MAX_TIME;
+		let t = MAX_TIME;
 		// Ignore write to `DidUpdate` since it transient.
 		let did_update_key = crate::DidUpdate::hashed_key().to_vec();
 		frame_benchmarking::benchmarking::add_to_whitelist(TrackedStorageKey {
@@ -47,7 +47,7 @@ benchmarks! {
 	}
 
 	on_finalize {
-		let t in 1 .. MAX_TIME;
+		let t = MAX_TIME;
 		Timestamp::<T>::set(RawOrigin::None.into(), t.into())?;
 		ensure!(DidUpdate::exists(), "Time was not set.");
 		// Ignore read/write to `DidUpdate` since it is transient.

substrate/frame/timestamp/src/default_weights.rs

@@ -22,13 +22,11 @@
 use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight};
 
 impl crate::WeightInfo for () {
-	// WARNING! Some components were not used: ["t"]
 	fn set() -> Weight {
 		(9133000 as Weight)
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(1 as Weight))
 	}
-	// WARNING! Some components were not used: ["t"]
 	fn on_finalize() -> Weight {
 		(5915000 as Weight)
 	}

substrate/frame/timestamp/src/lib.rs

@@ -107,7 +107,7 @@ use frame_support::{
 use sp_runtime::{
 	RuntimeString,
 	traits::{
-		AtLeast32Bit, Zero, SaturatedConversion, Scale
+		AtLeast32Bit, Zero, SaturatedConversion, Scale,
 	}
 };
 use frame_system::ensure_none;
@@ -159,9 +159,9 @@ decl_module! {
 		/// The dispatch origin for this call must be `Inherent`.
 		///
 		/// # <weight>
-		/// - `O(T)` where `T` complexity of `on_timestamp_set`
+		/// - `O(1)` (Note that implementations of `OnTimestampSet` must also be `O(1)`)
 		/// - 1 storage read and 1 storage mutation (codec `O(1)`). (because of `DidUpdate::take` in `on_finalize`)
-		/// - 1 event handler `on_timestamp_set` `O(T)`.
+		/// - 1 event handler `on_timestamp_set`. Must be `O(1)`.
 		/// # </weight>
 		#[weight = (
 			T::WeightInfo::set(),

substrate/frame/treasury/src/benchmarking.rs

@@ -203,7 +203,7 @@ benchmarks_instance! {
 	}: _(RawOrigin::Signed(caller), reason, awesome_person)
 
 	retract_tip {
-		let r in 0 .. MAX_BYTES;
+		let r = MAX_BYTES;
 		let (caller, reason, awesome_person) = setup_awesome::<T, _>(r);
 		Treasury::<T, _>::report_awesome(
 			RawOrigin::Signed(caller.clone()).into(),

substrate/frame/treasury/src/default_weights.rs

@@ -44,7 +44,6 @@ impl crate::WeightInfo for () {
 			.saturating_add(DbWeight::get().reads(2 as Weight))
 			.saturating_add(DbWeight::get().writes(2 as Weight))
 	}
-	// WARNING! Some components were not used: ["r"]
 	fn retract_tip() -> Weight {
 		(82970000 as Weight)
 			.saturating_add(DbWeight::get().reads(1 as Weight))

substrate/frame/utility/src/benchmarking.rs

@@ -50,13 +50,12 @@ benchmarks! {
 	}
 
 	as_derivative {
-		let u in 0 .. 1000;
-		let caller = account("caller", u, SEED);
+		let caller = account("caller", SEED, SEED);
 		let call = Box::new(frame_system::Call::remark(vec![]).into());
 		// Whitelist caller account from further DB operations.
 		let caller_key = frame_system::Account::<T>::hashed_key_for(&caller);
 		frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into());
-	}: _(RawOrigin::Signed(caller), u as u16, call)
+	}: _(RawOrigin::Signed(caller), SEED as u16, call)
 }
 
 #[cfg(test)]

substrate/frame/utility/src/default_weights.rs

@@ -27,7 +27,6 @@ impl crate::WeightInfo for () {
 		(16461000 as Weight)
 			.saturating_add((1982000 as Weight).saturating_mul(c as Weight))
 	}
-	// WARNING! Some components were not used: ["u"]
 	fn as_derivative() -> Weight {
 		(4086000 as Weight)
 	}

substrate/utils/frame/benchmarking-cli/src/command.rs

@@ -102,20 +102,9 @@ impl BenchmarkCmd {
 				// If we are going to output results to a file...
 				if let Some(output_path) = &self.output {
 					if self.trait_def {
-						crate::writer::write_trait(&batches, output_path, &self.r#trait, self.spaces)?;
+						crate::writer::write_trait(&batches, output_path, self)?;
 					} else {
-						crate::writer::write_results(
-							&batches,
-							output_path,
-							&self.lowest_range_values,
-							&self.highest_range_values,
-							&self.steps,
-							self.repeat,
-							&self.header,
-							&self.r#struct,
-							&self.r#trait,
-							self.spaces
-						)?;
+						crate::writer::write_results(&batches, output_path, self)?;
 					}
 				}
 

substrate/utils/frame/benchmarking-cli/src/writer.rs

@@ -17,6 +17,7 @@
 
 // Outputs benchmark results to Rust files that can be ingested by the runtime.
 
+use crate::BenchmarkCmd;
 use std::fs::{self, File, OpenOptions};
 use std::io::prelude::*;
 use std::path::PathBuf;
@@ -51,15 +52,14 @@ fn underscore<Number>(i: Number) -> String
 pub fn write_trait(
 	batches: &[BenchmarkBatch],
 	path: &PathBuf,
-	trait_name: &String,
-	spaces: bool,
+	cmd: &BenchmarkCmd,
 ) -> Result<(), std::io::Error> {
 	let mut file_path = path.clone();
 	file_path.push("trait");
 	file_path.set_extension("rs");
 	let mut file = crate::writer::open_file(file_path)?;
 
-	let indent = if spaces {"    "} else {"\t"};
+	let indent = if cmd.spaces {"    "} else {"\t"};
 
 	let mut current_pallet = Vec::<u8>::new();
 
@@ -82,7 +82,7 @@ pub fn write_trait(
 
 			// trait wrapper
 			write!(file, "// {}\n", pallet_string)?;
-			write!(file, "pub trait {} {{\n", trait_name)?;
+			write!(file, "pub trait {} {{\n", cmd.r#trait)?;
 
 			current_pallet = batch.pallet.clone()
 		}
@@ -108,17 +108,10 @@ pub fn write_trait(
 pub fn write_results(
 	batches: &[BenchmarkBatch],
 	path: &PathBuf,
-	lowest_range_values: &[u32],
-	highest_range_values: &[u32],
-	steps: &[u32],
-	repeat: u32,
-	header: &Option<PathBuf>,
-	struct_name: &String,
-	trait_name: &String,
-	spaces: bool,
+	cmd: &BenchmarkCmd,
 ) -> Result<(), std::io::Error> {
 
-	let header_text = match header {
+	let header_text = match &cmd.header {
 		Some(header_file) => {
 			let text = fs::read_to_string(header_file)?;
 			Some(text)
@@ -126,7 +119,7 @@ pub fn write_results(
 		None => None,
 	};
 
-	let indent = if spaces {"    "} else {"\t"};
+	let indent = if cmd.spaces {"    "} else {"\t"};
 	let date = chrono::Utc::now();
 
 	let mut current_pallet = Vec::<u8>::new();
@@ -170,15 +163,25 @@ pub fn write_results(
 				VERSION,
 			)?;
 
-			// date of generation
+			// date of generation + some settings
 			write!(
 				file,
-				"//! DATE: {}, STEPS: {:?}, REPEAT: {}, LOW RANGE: {:?}, HIGH RANGE: {:?}\n\n",
+				"//! DATE: {}, STEPS: {:?}, REPEAT: {}, LOW RANGE: {:?}, HIGH RANGE: {:?}\n",
 				date.format("%Y-%m-%d"),
-				steps,
-				repeat,
-				lowest_range_values,
-				highest_range_values,
+				cmd.steps,
+				cmd.repeat,
+				cmd.lowest_range_values,
+				cmd.highest_range_values,
+			)?;
+
+			// more settings
+			write!(
+				file,
+				"//! EXECUTION: {:?}, WASM-EXECUTION: {}, CHAIN: {:?}, DB CACHE: {}\n",
+				cmd.execution,
+				cmd.wasm_method,
+				cmd.shared_params.chain,
+				cmd.database_cache_size,
 			)?;
 
 			// allow statements
@@ -194,15 +197,15 @@ pub fn write_results(
 			)?;
 
 			// struct for weights
-			write!(file, "pub struct {}<T>(PhantomData<T>);\n", struct_name)?;
+			write!(file, "pub struct {}<T>(PhantomData<T>);\n", cmd.r#struct)?;
 
 			// trait wrapper
 			write!(
 				file,
 				"impl<T: frame_system::Trait> {}::{} for {}<T> {{\n",
 				pallet_string,
-				trait_name,
-				struct_name,
+				cmd.r#trait,
+				cmd.r#struct,
 			)?;
 
 			current_pallet = batch.pallet.clone()
@@ -241,17 +244,16 @@ pub fn write_results(
 			.iter()
 			.map(|(name, _)| -> String { return name.to_string() })
 			.collect::<Vec<String>>();
-		if all_components.len() != used_components.len() {
-			let mut unused_components = all_components;
-			unused_components.retain(|x| !used_components.contains(&x));
-			write!(file, "{}// WARNING! Some components were not used: {:?}\n", indent, unused_components)?;
-		}
 
 		// function name
 		write!(file, "{}fn {}(", indent, benchmark_string)?;
 		// params
-		for component in used_components {
-			write!(file, "{}: u32, ", component)?;
+		for component in all_components {
+			if used_components.contains(&&component) {
+				write!(file, "{}: u32, ", component)?;
+			} else {
+				write!(file, "_{}: u32, ", component)?;
+			}
 		}
 		// return value
 		write!(file, ") -> Weight {{\n")?;