diff --git a/substrate/bin/node/cli/tests/benchmark_pallet_works.rs b/substrate/bin/node/cli/tests/benchmark_pallet_works.rs new file mode 100644 index 0000000000000000000000000000000000000000..bf29c0e308bcb3380909a03bc567ca55eb03469d --- /dev/null +++ b/substrate/bin/node/cli/tests/benchmark_pallet_works.rs @@ -0,0 +1,49 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see <https://www.gnu.org/licenses/>. + +use assert_cmd::cargo::cargo_bin; +use std::process::Command; + +pub mod common; + +/// `benchmark pallet` works for the different combinations of `steps` and `repeat`. +#[test] +fn benchmark_pallet_works() { + // Some invalid combinations: + benchmark_pallet(0, 10, false); + benchmark_pallet(1, 10, false); + // ... and some valid: + benchmark_pallet(2, 1, true); + benchmark_pallet(50, 20, true); + benchmark_pallet(20, 50, true); +} + +fn benchmark_pallet(steps: u32, repeat: u32, should_work: bool) { + let output = Command::new(cargo_bin("substrate")) + .args(["benchmark", "pallet", "--dev"]) + // Use the `addition` benchmark since is the fastest. + .args(["--pallet", "frame-benchmarking", "--extrinsic", "addition"]) + .args(["--steps", &format!("{}", steps), "--repeat", &format!("{}", repeat)]) + .output() + .unwrap(); + + if output.status.success() != should_work { + let log = String::from_utf8_lossy(&output.stderr).to_string(); + panic!("Test failed:\n{}", log); + } +} diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index fae5a4494cdc4192abf4fdbc68d9bead7aecff76..fb6f1393650adb384dd6f8a94a0168ad616cb924 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -236,14 +236,22 @@ impl PalletCmd { let lowest = self.lowest_range_values.get(idx).cloned().unwrap_or(*low); let highest = self.highest_range_values.get(idx).cloned().unwrap_or(*high); - let diff = highest - lowest; + let diff = + highest.checked_sub(lowest).ok_or("`low` cannot be higher than `high`")?; - // Create up to `STEPS` steps for that component between high and low. - let step_size = (diff / self.steps).max(1); - let num_of_steps = diff / step_size + 1; - for s in 0..num_of_steps { + // The slope logic needs at least two points + // to compute a slope. + if self.steps < 2 { + return Err("`steps` must be at least 2.".into()) + } + + let step_size = (diff as f32 / (self.steps - 1) as f32).max(0.0); + + for s in 0..self.steps { // This is the value we will be testing for component `name` - let component_value = lowest + step_size * s; + let component_value = ((lowest as f32 + step_size * s as f32) as u32) + .min(highest) + .max(lowest); // Select the max value for all the other components. let c: Vec<(BenchmarkParameter, u32)> = components @@ -364,13 +372,14 @@ impl PalletCmd { if elapsed >= time::Duration::from_secs(5) { timer = time::SystemTime::now(); log::info!( - "Running Benchmark: {}.{} {}/{} {}/{}", + "Running Benchmark: {}.{}({} args) {}/{} {}/{}", String::from_utf8(pallet.clone()) .expect("Encoded from String; qed"), String::from_utf8(extrinsic.clone()) .expect("Encoded from String; qed"), - s + 1, // s starts at 0. todo show step - self.steps, + components.len(), + s + 1, // s starts at 0. + all_components.len(), r + 1, self.external_repeat, );