diff --git a/Cargo.lock b/Cargo.lock index 525ea78d9d4e2..e44105b1d907d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1572,7 +1572,6 @@ dependencies = [ name = "frame-benchmarking-cli" version = "2.0.0-rc5" dependencies = [ - "Inflector", "frame-benchmarking", "parity-scale-codec", "sc-cli", diff --git a/Process.json b/Process.json index cd15e137df6ed..540bd644311cf 100644 --- a/Process.json +++ b/Process.json @@ -21,4 +21,9 @@ "project_name": "Smart Contracts", "owner": "pepyakin", "matrix_room_id": "!yBKstWVBkwzUkPslsp:matrix.parity.io" +}, +{ + "project_name": "Benchmarking and Weights", + "owner": "shawntabrizi", + "matrix_room_id": "!pZPWqCRLVtORZTEsEf:matrix.parity.io" }] diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index 7ed9a862a0479..5a2bd55ff79aa 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -30,6 +30,13 @@ pub enum BenchmarkParameter { a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z, } +#[cfg(feature = "std")] +impl std::fmt::Display for BenchmarkParameter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?}", self) + } +} + /// The results of a single of benchmark. #[derive(Encode, Decode, Clone, PartialEq, Debug)] pub struct BenchmarkBatch { diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 4c522337259ef..c34404575e5fc 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -12,7 +12,6 @@ description = "CLI for benchmarking FRAME" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -Inflector = "0.11.4" frame-benchmarking = { version = "2.0.0-rc5", path = "../../../frame/benchmarking" } sp-core = { version = "2.0.0-rc5", path = "../../../primitives/core" } sc-service = { version = "0.8.0-rc5", default-features = false, path = "../../../client/service" } diff --git a/utils/frame/benchmarking-cli/src/command.rs b/utils/frame/benchmarking-cli/src/command.rs index 553b68c453fa7..688e393bd605a 100644 --- a/utils/frame/benchmarking-cli/src/command.rs +++ b/utils/frame/benchmarking-cli/src/command.rs @@ -95,8 +95,7 @@ impl BenchmarkCmd { let mut file = crate::writer::open_file("traits.rs")?; crate::writer::write_trait(&mut file, batches.clone())?; } else { - let mut file = crate::writer::open_file("benchmarks.rs")?; - crate::writer::write_results(&mut file, batches.clone())?; + crate::writer::write_results(&batches)?; } } diff --git a/utils/frame/benchmarking-cli/src/writer.rs b/utils/frame/benchmarking-cli/src/writer.rs index 199dbb795e581..2bc17aa85bddb 100644 --- a/utils/frame/benchmarking-cli/src/writer.rs +++ b/utils/frame/benchmarking-cli/src/writer.rs @@ -20,13 +20,15 @@ use std::fs::{File, OpenOptions}; use std::io::prelude::*; use frame_benchmarking::{BenchmarkBatch, BenchmarkSelector, Analysis}; -use inflector::Inflector; +use sp_runtime::traits::Zero; + +const VERSION: &'static str = env!("CARGO_PKG_VERSION"); pub fn open_file(path: &str) -> Result { OpenOptions::new() .create(true) .write(true) - .append(true) + .truncate(true) .open(path) } @@ -47,81 +49,49 @@ pub fn write_trait(file: &mut File, batches: Vec) -> Result<(), if batch.pallet != current_pallet { if !current_pallet.is_empty() { // close trait - write!(file, "}}\n").unwrap(); + write!(file, "}}\n")?; } // trait wrapper - write!(file, "// {}\n", pallet_string).unwrap(); - write!(file, "pub trait WeightInfo {{\n").unwrap(); - - current_pallet = batch.pallet.clone() - } - - // function name - write!(file, "\tfn {}(", benchmark_string).unwrap(); - - // params - let components = &batch.results[0].components; - for component in components { - write!(file, "{:?}: u32, ", component.0).unwrap(); - } - // return value - write!(file, ") -> Weight;\n").unwrap(); - } - - // final close trait - write!(file, "}}\n").unwrap(); - - // Reset - current_pallet = Vec::::new(); - - for batch in &batches { - if batch.results.is_empty() { continue } - - let benchmark_string = String::from_utf8(batch.benchmark.clone()).unwrap(); - - // only create new trait definitions when we go to a new pallet - if batch.pallet != current_pallet { - if !current_pallet.is_empty() { - // close trait - write!(file, "}}\n").unwrap(); - } - - // impl trait - write!(file, "\n").unwrap(); - write!(file, "impl WeightInfo for () {{\n").unwrap(); + write!(file, "// {}\n", pallet_string)?; + write!(file, "pub trait WeightInfo {{\n")?; current_pallet = batch.pallet.clone() } // function name - write!(file, "\tfn {}(", benchmark_string).unwrap(); + write!(file, "\tfn {}(", benchmark_string)?; // params let components = &batch.results[0].components; for component in components { - write!(file, "_{:?}: u32, ", component.0).unwrap(); + write!(file, "{:?}: u32, ", component.0)?; } // return value - write!(file, ") -> Weight {{ 1_000_000_000 }}\n").unwrap(); + write!(file, ") -> Weight;\n")?; } // final close trait - write!(file, "}}\n").unwrap(); + write!(file, "}}\n")?; Ok(()) } -pub fn write_results(file: &mut File, batches: Vec) -> Result<(), std::io::Error> { +pub fn write_results(batches: &[BenchmarkBatch]) -> Result<(), std::io::Error> { let mut current_pallet = Vec::::new(); // Skip writing if there are no batches if batches.is_empty() { return Ok(()) } - // general imports - write!(file, "use frame_support::weights::{{Weight, constants::RocksDbWeight as DbWeight}};\n").unwrap(); + let mut batches_iter = batches.iter().peekable(); - for batch in &batches { + let first_pallet = String::from_utf8( + batches_iter.peek().expect("we checked that batches is not empty").pallet.clone() + ).unwrap(); + let mut file = open_file(&(first_pallet + ".rs"))?; + + + while let Some(batch) = batches_iter.next() { // Skip writing if there are no results if batch.results.is_empty() { continue } @@ -130,69 +100,120 @@ pub fn write_results(file: &mut File, batches: Vec) -> Result<() // only create new trait definitions when we go to a new pallet if batch.pallet != current_pallet { - if !current_pallet.is_empty() { - // close trait - write!(file, "}}\n").unwrap(); - } + // auto-generation note + write!( + file, + "//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {}\n\n", + VERSION, + )?; + + // general imports + write!( + file, + "use frame_support::weights::{{Weight, constants::RocksDbWeight as DbWeight}};\n\n" + )?; // struct for weights - write!(file, "pub struct WeightFor{};\n", - pallet_string.to_pascal_case(), - ).unwrap(); + write!(file, "pub struct WeightInfo;\n")?; // trait wrapper - write!(file, "impl {}::WeightInfo for WeightFor{} {{\n", - pallet_string, - pallet_string.to_pascal_case(), - ).unwrap(); + write!(file, "impl {}::WeightInfo for WeightInfo {{\n", pallet_string)?; current_pallet = batch.pallet.clone() } - // function name - write!(file, "\tfn {}(", benchmark_string).unwrap(); + // Analysis results + let extrinsic_time = Analysis::min_squares_iqr(&batch.results, BenchmarkSelector::ExtrinsicTime).unwrap(); + let reads = Analysis::min_squares_iqr(&batch.results, BenchmarkSelector::Reads).unwrap(); + let writes = Analysis::min_squares_iqr(&batch.results, BenchmarkSelector::Writes).unwrap(); + // Analysis data may include components that are not used, this filters out anything whose value is zero. + let mut used_components = Vec::new(); + let mut used_extrinsic_time = Vec::new(); + let mut used_reads = Vec::new(); + let mut used_writes = Vec::new(); + extrinsic_time.slopes.iter().zip(extrinsic_time.names.iter()).for_each(|(slope, name)| { + if !slope.is_zero() { + if !used_components.contains(&name) { used_components.push(name); } + used_extrinsic_time.push((slope, name)); + } + }); + reads.slopes.iter().zip(reads.names.iter()).for_each(|(slope, name)| { + if !slope.is_zero() { + if !used_components.contains(&name) { used_components.push(name); } + used_reads.push((slope, name)); + } + }); + writes.slopes.iter().zip(writes.names.iter()).for_each(|(slope, name)| { + if !slope.is_zero() { + if !used_components.contains(&name) { used_components.push(name); } + used_writes.push((slope, name)); + } + }); + + let all_components = batch.results[0].components + .iter() + .map(|(name, _)| -> String { return name.to_string() }) + .collect::>(); + if all_components.len() != used_components.len() { + let mut unused_components = all_components; + unused_components.retain(|x| !used_components.contains(&x)); + write!(file, "\t// WARNING! Some components were not used: {:?}\n", unused_components)?; + } + + // function name + write!(file, "\tfn {}(", benchmark_string)?; // params - let components = &batch.results[0].components; - for component in components { - write!(file, "{:?}: u32, ", component.0).unwrap(); + for component in used_components { + write!(file, "{}: u32, ", component)?; } // return value - write!(file, ") -> Weight {{\n").unwrap(); + write!(file, ") -> Weight {{\n")?; - let extrinsic_time = Analysis::min_squares_iqr(&batch.results, BenchmarkSelector::ExtrinsicTime).unwrap(); - write!(file, "\t\t({} as Weight)\n", extrinsic_time.base.saturating_mul(1000)).unwrap(); - extrinsic_time.slopes.iter().zip(extrinsic_time.names.iter()).for_each(|(slope, name)| { + write!(file, "\t\t({} as Weight)\n", extrinsic_time.base.saturating_mul(1000))?; + used_extrinsic_time.iter().try_for_each(|(slope, name)| -> Result<(), std::io::Error> { write!(file, "\t\t\t.saturating_add(({} as Weight).saturating_mul({} as Weight))\n", slope.saturating_mul(1000), name, - ).unwrap(); - }); + ) + })?; - let reads = Analysis::min_squares_iqr(&batch.results, BenchmarkSelector::Reads).unwrap(); - write!(file, "\t\t\t.saturating_add(DbWeight::get().reads({} as Weight))\n", reads.base).unwrap(); - reads.slopes.iter().zip(reads.names.iter()).for_each(|(slope, name)| { + if !reads.base.is_zero() { + write!(file, "\t\t\t.saturating_add(DbWeight::get().reads({} as Weight))\n", reads.base)?; + } + used_reads.iter().try_for_each(|(slope, name)| -> Result<(), std::io::Error> { write!(file, "\t\t\t.saturating_add(DbWeight::get().reads(({} as Weight).saturating_mul({} as Weight)))\n", slope, name, - ).unwrap(); - }); + ) + })?; - let writes = Analysis::min_squares_iqr(&batch.results, BenchmarkSelector::Writes).unwrap(); - write!(file, "\t\t\t.saturating_add(DbWeight::get().writes({} as Weight))\n", writes.base).unwrap(); - writes.slopes.iter().zip(writes.names.iter()).for_each(|(slope, name)| { + if !writes.base.is_zero() { + write!(file, "\t\t\t.saturating_add(DbWeight::get().writes({} as Weight))\n", writes.base)?; + } + used_writes.iter().try_for_each(|(slope, name)| -> Result<(), std::io::Error> { write!(file, "\t\t\t.saturating_add(DbWeight::get().writes(({} as Weight).saturating_mul({} as Weight)))\n", slope, name, - ).unwrap(); - }); + ) + })?; // close function - write!(file, "\t}}\n").unwrap(); + write!(file, "\t}}\n")?; + + // Check if this is the end of the iterator + if let Some(next) = batches_iter.peek() { + // Next pallet is different than current pallet, so we close up the file and open a new one. + if next.pallet != current_pallet { + write!(file, "}}\n")?; + let next_pallet = String::from_utf8(next.pallet.clone()).unwrap(); + file = open_file(&(next_pallet + ".rs"))?; + } + } else { + // This is the end of the iterator, so we close up the final file. + write!(file, "}}\n")?; + } } - // final close trait - write!(file, "}}\n").unwrap(); - Ok(()) }