From 397da2753081b3cbe4cbb0893f17b4188f0d5997 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 23 Sep 2022 12:42:36 +0200 Subject: [PATCH] bench: Use `_` instead of `::` in auto-generated file names (#12332) * Replace :: with _ in auto-generated file names Signed-off-by: Oliver Tale-Yazdi * fmt Signed-off-by: Oliver Tale-Yazdi Co-authored-by: Shawn Tabrizi --- frame/benchmarking/README.md | 2 +- .../benchmarking-cli/src/pallet/command.rs | 15 ++++++------ .../benchmarking-cli/src/pallet/writer.rs | 24 ++++++++++++++----- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/frame/benchmarking/README.md b/frame/benchmarking/README.md index 6316cd5903c8b..76673c5f69b33 100644 --- a/frame/benchmarking/README.md +++ b/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. diff --git a/utils/frame/benchmarking-cli/src/pallet/command.rs b/utils/frame/benchmarking-cli/src/pallet/command.rs index 0fc7cc4d783f7..6870ec386d23d 100644 --- a/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -228,7 +228,7 @@ impl PalletCmd { let mut component_ranges = HashMap::<(Vec, Vec), Vec>::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 = 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(()) } diff --git a/utils/frame/benchmarking-cli/src/pallet/writer.rs b/utils/frame/benchmarking-cli/src/pallet/writer.rs index a601c09eb5033..cd52ffc329d1c 100644 --- a/utils/frame/benchmarking-cli/src/pallet/writer.rs +++ b/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(()) }