bench: Use _ instead of :: in auto-generated file names (parityte…

…ch#12332)

* Replace :: with _ in auto-generated file names

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* fmt

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

frame/benchmarking/README.md

@@ -185,7 +185,7 @@ Then you can run a benchmark like so:
 ```
 
 This will output a file `pallet_name.rs` which implements the `WeightInfo` trait you should include
-in your pallet. Each blockchain should generate their own benchmark file with their custom
+in your pallet. Double colons `::` will be replaced with a `_` in the output name if you specify a directory. Each blockchain should generate their own benchmark file with their custom
 implementation of the `WeightInfo` trait. This means that you will be able to use these modular
 Substrate pallets while still keeping your network safe for your specific configuration and
 requirements.

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

@@ -228,7 +228,7 @@ impl PalletCmd {
 		let mut component_ranges = HashMap::<(Vec<u8>, Vec<u8>), Vec<ComponentRange>>::new();
 
 		for (pallet, extrinsic, components) in benchmarks_to_run {
-			log::info!(
+			println!(
 				"Starting benchmark: {}::{}",
 				String::from_utf8(pallet.clone()).expect("Encoded from String; qed"),
 				String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"),
@@ -376,7 +376,7 @@ impl PalletCmd {
 					if let Ok(elapsed) = timer.elapsed() {
 						if elapsed >= time::Duration::from_secs(5) {
 							timer = time::SystemTime::now();
-							log::info!(
+							println!(
 								"Running Benchmark: {}.{}({} args) {}/{} {}/{}",
 								String::from_utf8(pallet.clone())
 									.expect("Encoded from String; qed"),
@@ -398,16 +398,17 @@ impl PalletCmd {
 		// are together.
 		let batches: Vec<BenchmarkBatchSplitResults> = combine_batches(batches, batches_db);
 
-		// Create the weights.rs file.
-		if let Some(output_path) = &self.output {
-			writer::write_results(&batches, &storage_info, &component_ranges, output_path, self)?;
-		}
-
 		// Jsonify the result and write it to a file or stdout if desired.
 		if !self.jsonify(&batches)? {
 			// Print the summary only if `jsonify` did not write to stdout.
 			self.print_summary(&batches, &storage_info)
 		}
+
+		// Create the weights.rs file.
+		if let Some(output_path) = &self.output {
+			writer::write_results(&batches, &storage_info, &component_ranges, output_path, self)?;
+		}
+
 		Ok(())
 	}
 

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

@@ -24,6 +24,7 @@ use std::{
 };
 
 use inflector::Inflector;
+use itertools::Itertools;
 use serde::Serialize;
 
 use crate::{pallet::command::ComponentRange, shared::UnderscoreHelper, PalletCmd};
@@ -314,18 +315,21 @@ pub(crate) fn write_results(
 
 	// Organize results by pallet into a JSON map
 	let all_results = map_results(batches, storage_info, component_ranges, &analysis_choice)?;
+	let mut created_files = Vec::new();
+
 	for ((pallet, instance), results) in all_results.iter() {
 		let mut file_path = path.clone();
 		// If a user only specified a directory...
 		if file_path.is_dir() {
+			// Start with "path/to/pallet_name".
+			let mut file_name = pallet.clone();
 			// Check if there might be multiple instances benchmarked.
 			if all_results.keys().any(|(p, i)| p == pallet && i != instance) {
-				// Create new file: "path/to/pallet_name_instance_name.rs".
-				file_path.push(pallet.clone() + "_" + instance.to_snake_case().as_str());
-			} else {
-				// Create new file: "path/to/pallet_name.rs".
-				file_path.push(pallet.clone());
+				// Append "_instance_name".
+				file_name = format!("{}_{}", file_name, instance.to_snake_case());
 			}
+			// "mod::pallet_name.rs" becomes "mod_pallet_name.rs".
+			file_path.push(file_name.replace("::", "_"));
 			file_path.set_extension("rs");
 		}
 
@@ -342,10 +346,18 @@ pub(crate) fn write_results(
 			benchmarks: results.clone(),
 		};
 
-		let mut output_file = fs::File::create(file_path)?;
+		let mut output_file = fs::File::create(&file_path)?;
 		handlebars
 			.render_template_to_write(&template, &hbs_data, &mut output_file)
 			.map_err(|e| io_error(&e.to_string()))?;
+		println!("Created file: {:?}", &file_path);
+		created_files.push(file_path);
+	}
+
+	for file in created_files.iter().duplicates() {
+		// This can happen when there are multiple instances of a pallet deployed
+		// and `--output` forces the output of all instances into the same file.
+		println!("Multiple benchmarks were written to the same file: {:?}.", file);
 	}
 	Ok(())
 }