From ea48d6b3e7accc8d1cf8733b1e2ee38a22d7a441 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Fri, 11 Dec 2020 01:20:10 -0800 Subject: [PATCH] Support Multiple Instances with Benchmarks (#7669) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Support multiple instances with benchmarks * fix tests * Apply suggestions from code review Co-authored-by: Bastian Köcher * docs * fix output * Update lib.rs Co-authored-by: Bastian Köcher --- Cargo.lock | 1 + frame/benchmarking/src/lib.rs | 29 ++++++++++++--- frame/benchmarking/src/utils.rs | 2 ++ utils/frame/benchmarking-cli/Cargo.toml | 3 +- utils/frame/benchmarking-cli/src/writer.rs | 42 +++++++++++++++------- 5 files changed, 60 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1555957f80e33..dbb640806c698 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1568,6 +1568,7 @@ dependencies = [ name = "frame-benchmarking-cli" version = "2.0.0" dependencies = [ + "Inflector", "chrono", "frame-benchmarking", "handlebars", diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index 97b58ae19ec70..6296c000e289f 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -1052,10 +1052,29 @@ macro_rules! impl_benchmark_test { /// ``` /// /// At the end of `dispatch_benchmark`, you should return this batches object. +/// +/// In the case where you have multiple instances of a pallet that you need to separately benchmark, +/// the name of your module struct will be used as a suffix to your outputted weight file. For +/// example: +/// +/// ```ignore +/// add_benchmark!(params, batches, pallet_balances, Balances); // pallet_balances.rs +/// add_benchmark!(params, batches, pallet_collective, Council); // pallet_collective_council.rs +/// add_benchmark!(params, batches, pallet_collective, TechnicalCommittee); // pallet_collective_technical_committee.rs +/// ``` +/// +/// You can manipulate this suffixed string by using a type alias if needed. For example: +/// +/// ```ignore +/// type Council2 = TechnicalCommittee; +/// add_benchmark!(params, batches, pallet_collective, Council2); // pallet_collective_council_2.rs +/// ``` + #[macro_export] macro_rules! add_benchmark { ( $params:ident, $batches:ident, $name:ident, $( $location:tt )* ) => ( let name_string = stringify!($name).as_bytes(); + let instance_string = stringify!( $( $location )* ).as_bytes(); let (config, whitelist) = $params; let $crate::BenchmarkConfig { pallet, @@ -1071,6 +1090,9 @@ macro_rules! add_benchmark { if &pallet[..] == &b"*"[..] || &benchmark[..] == &b"*"[..] { for benchmark in $( $location )*::benchmarks(*extra).into_iter() { $batches.push($crate::BenchmarkBatch { + pallet: name_string.to_vec(), + instance: instance_string.to_vec(), + benchmark: benchmark.to_vec(), results: $( $location )*::run_benchmark( benchmark, &lowest_range_values[..], @@ -1080,12 +1102,13 @@ macro_rules! add_benchmark { whitelist, *verify, )?, - pallet: name_string.to_vec(), - benchmark: benchmark.to_vec(), }); } } else { $batches.push($crate::BenchmarkBatch { + pallet: name_string.to_vec(), + instance: instance_string.to_vec(), + benchmark: benchmark.clone(), results: $( $location )*::run_benchmark( &benchmark[..], &lowest_range_values[..], @@ -1095,8 +1118,6 @@ macro_rules! add_benchmark { whitelist, *verify, )?, - pallet: name_string.to_vec(), - benchmark: benchmark.clone(), }); } } diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index 042f4b707aef4..2c2aee910e364 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -43,6 +43,8 @@ impl std::fmt::Display for BenchmarkParameter { pub struct BenchmarkBatch { /// The pallet containing this benchmark. pub pallet: Vec, + /// The instance of this pallet being benchmarked. + pub instance: Vec, /// The extrinsic (or benchmark name) of this benchmark. pub benchmark: Vec, /// The results from this benchmark. diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 4ee2454e708e9..83f93799691d3 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -23,11 +23,12 @@ sp-externalities = { version = "0.8.0", path = "../../../primitives/externalitie sp-keystore = { version = "0.8.0", path = "../../../primitives/keystore" } sp-runtime = { version = "2.0.0", path = "../../../primitives/runtime" } sp-state-machine = { version = "0.8.0", path = "../../../primitives/state-machine" } -structopt = "0.3.8" codec = { version = "1.3.1", package = "parity-scale-codec" } +structopt = "0.3.8" chrono = "0.4" serde = "1.0.116" handlebars = "3.5.0" +Inflector = "0.11.4" [features] default = ["db"] diff --git a/utils/frame/benchmarking-cli/src/writer.rs b/utils/frame/benchmarking-cli/src/writer.rs index fd72e003b417a..4afc810730678 100644 --- a/utils/frame/benchmarking-cli/src/writer.rs +++ b/utils/frame/benchmarking-cli/src/writer.rs @@ -22,6 +22,7 @@ use std::fs; use std::path::PathBuf; use serde::Serialize; +use inflector::Inflector; use crate::BenchmarkCmd; use frame_benchmarking::{BenchmarkBatch, BenchmarkSelector, Analysis, RegressionModel}; @@ -37,6 +38,7 @@ struct TemplateData { date: String, version: String, pallet: String, + instance: String, header: String, cmd: CmdData, benchmarks: Vec, @@ -102,7 +104,7 @@ fn io_error(s: &str) -> std::io::Error { // p1 -> [b1, b2, b3] // p2 -> [b1, b2] // ``` -fn map_results(batches: &[BenchmarkBatch]) -> Result>, std::io::Error> { +fn map_results(batches: &[BenchmarkBatch]) -> Result>, std::io::Error> { // Skip if batches is empty. if batches.is_empty() { return Err(io_error("empty batches")) } @@ -115,6 +117,7 @@ fn map_results(batches: &[BenchmarkBatch]) -> Result Result