handle steps in cli

frame/benchmarking/src/lib.rs

@@ -846,17 +846,20 @@ macro_rules! impl_benchmark {
 					Ok(())
 				};
 
+				let (current_step, total_steps) = steps;
+
 				if components.is_empty() {
-					if verify {
-						// If `--verify` is used, run the benchmark once to verify it would complete.
-						do_benchmark(Default::default(), &mut $crate::Vec::new(), true, 1, 1)?;
+					// The CLI could ask to do more steps than is sensible, so we skip those.
+					if current_step == 0 {
+						if verify {
+							// If `--verify` is used, run the benchmark once to verify it would complete.
+							do_benchmark(Default::default(), &mut $crate::Vec::new(), true, 1, 1)?;
+						}
+						do_benchmark(Default::default(), &mut results, false, 1, 1)?;
 					}
-					do_benchmark(Default::default(), &mut results, false, 1, 1)?;
 				} else {
 					// Select the component we will be benchmarking. Each component will be benchmarked.
 					for (idx, (name, low, high)) in components.iter().enumerate() {
-						// Get the number of steps for this component.
-						let (_current_step, total_steps) = steps;
 
 						let lowest = lowest_range_values.get(idx).cloned().unwrap_or(*low);
 						let highest = highest_range_values.get(idx).cloned().unwrap_or(*high);
@@ -867,28 +870,31 @@ macro_rules! impl_benchmark {
 						let step_size = (diff / total_steps).max(1);
 						let num_of_steps = diff / step_size + 1;
 
-						for s in 0..num_of_steps {
-							// This is the value we will be testing for component `name`
-							let component_value = lowest + step_size * s;
-
-							// Select the max value for all the other components.
-							let c: $crate::Vec<($crate::BenchmarkParameter, u32)> = components.iter()
-								.enumerate()
-								.map(|(idx, (n, _, h))|
-									if n == name {
-										(*n, component_value)
-									} else {
-										(*n, *highest_range_values.get(idx).unwrap_or(h))
-									}
-								)
-								.collect();
+						// The CLI could ask to do more steps than is sensible, so we just skip those.
+						if current_step >= num_of_steps {
+							continue;
+						}
 
-							if verify {
-								// If `--verify` is used, run the benchmark once to verify it would complete.
-								do_benchmark(&c, &mut $crate::Vec::new(), true, s, num_of_steps)?;
-							}
-							do_benchmark(&c, &mut results, false, s, num_of_steps)?;
+						// This is the value we will be testing for component `name`
+						let component_value = lowest + step_size * current_step;
+
+						// Select the max value for all the other components.
+						let c: $crate::Vec<($crate::BenchmarkParameter, u32)> = components.iter()
+							.enumerate()
+							.map(|(idx, (n, _, h))|
+								if n == name {
+									(*n, component_value)
+								} else {
+									(*n, *highest_range_values.get(idx).unwrap_or(h))
+								}
+							)
+							.collect();
+
+						if verify {
+							// If `--verify` is used, run the benchmark once to verify it would complete.
+							do_benchmark(&c, &mut $crate::Vec::new(), true, current_step, num_of_steps)?;
 						}
+						do_benchmark(&c, &mut results, false, current_step, num_of_steps)?;
 					}
 				}
 				return Ok(results);

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

@@ -173,39 +173,43 @@ impl BenchmarkCmd {
 
 		// Run the benchmarks
 		for (pallet, extrinsic) in benchmarks_to_run {
-			for r in 0..self.repeat {
-				let result = StateMachine::<_, _, NumberFor<BB>, _>::new(
-					&state,
-					None,
-					&mut changes,
-					&executor,
-					"Benchmark_dispatch_benchmark",
-					&(
-						&pallet,
-						&extrinsic,
-						self.lowest_range_values.clone(),
-						self.highest_range_values.clone(),
-						(self.steps, self.steps),
-						(r, self.repeat),
-						!self.no_verify,
-						self.extra,
+			for s in 0..self.steps {
+				for r in 0..self.repeat {
+					// This should run only a single instance of a benchmark for `pallet` and
+					// `extrinsic`. All loops happen above.
+					let result = StateMachine::<_, _, NumberFor<BB>, _>::new(
+						&state,
+						None,
+						&mut changes,
+						&executor,
+						"Benchmark_dispatch_benchmark",
+						&(
+							&pallet,
+							&extrinsic,
+							self.lowest_range_values.clone(),
+							self.highest_range_values.clone(),
+							(s, self.steps),
+							(r, self.repeat),
+							!self.no_verify,
+							self.extra,
+						)
+							.encode(),
+						extensions(),
+						&sp_state_machine::backend::BackendRuntimeCode::new(&state).runtime_code()?,
+						sp_core::testing::TaskExecutor::new(),
 					)
-						.encode(),
-					extensions(),
-					&sp_state_machine::backend::BackendRuntimeCode::new(&state).runtime_code()?,
-					sp_core::testing::TaskExecutor::new(),
-				)
-				.execute(strategy.into())
-				.map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?;
-
-				let (batch, last_storage_info) = <std::result::Result<
-					(Vec<BenchmarkBatch>, Vec<StorageInfo>),
-					String,
-				> as Decode>::decode(&mut &result[..])
-				.map_err(|e| format!("Failed to decode benchmark results: {:?}", e))??;
-
-				batches.extend(batch);
-				storage_info = last_storage_info;
+					.execute(strategy.into())
+					.map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?;
+
+					let (batch, last_storage_info) = <std::result::Result<
+						(Vec<BenchmarkBatch>, Vec<StorageInfo>),
+						String,
+					> as Decode>::decode(&mut &result[..])
+					.map_err(|e| format!("Failed to decode benchmark results: {:?}", e))??;
+
+					batches.extend(batch);
+					storage_info = last_storage_info;
+				}
 			}
 		}