From bda08b5c07403b75cf1dbf9fd7be34d3e0b250d4 Mon Sep 17 00:00:00 2001 From: philoniare Date: Thu, 22 Feb 2024 23:53:46 +0800 Subject: [PATCH 1/5] feat: cleanup String::from_utf8 --- .../benchmarking-cli/src/pallet/command.rs | 54 +++++++++++-------- .../benchmarking-cli/src/pallet/writer.rs | 39 ++++++-------- 2 files changed, 48 insertions(+), 45 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index dce49db15f7d..b5f4bc04bc04 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -291,16 +291,22 @@ impl PalletCmd { // Convert `Vec` to `String` for better readability. let benchmarks_to_run: Vec<_> = benchmarks_to_run .into_iter() - .map(|b| { + .map(|(pallet, extrinsic, components, pov_modes)| { + let pallet_str = String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); + let extrinsic_str = String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); + let pov_modes_str = pov_modes.into_iter() + .map(|(p, s)| { + (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) + }) + .collect(); + ( - b.0, - b.1, - b.2, - b.3.into_iter() - .map(|(p, s)| { - (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) - }) - .collect(), + pallet, + extrinsic, + components, + pov_modes_str, + pallet_str, + extrinsic_str, ) }) .collect(); @@ -323,12 +329,12 @@ impl PalletCmd { let mut component_ranges = HashMap::<(Vec, Vec), Vec>::new(); let pov_modes = Self::parse_pov_modes(&benchmarks_to_run)?; - for (pallet, extrinsic, components, _) in benchmarks_to_run.clone() { + for (pallet, extrinsic, components, _, pallet_str, extrinsic_str) in benchmarks_to_run.clone() { log::info!( target: LOG_TARGET, "Starting benchmark: {}::{}", - String::from_utf8(pallet.clone()).expect("Encoded from String; qed"), - String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"), + pallet_str, + extrinsic_str, ); let all_components = if components.is_empty() { vec![Default::default()] @@ -411,8 +417,8 @@ impl PalletCmd { .map_err(|e| { format!( "Benchmark {}::{} failed: {}", - String::from_utf8_lossy(&pallet), - String::from_utf8_lossy(&extrinsic), + pallet_str, + extrinsic_str, e ) })?; @@ -489,10 +495,8 @@ impl PalletCmd { log::info!( target: LOG_TARGET, "Running benchmark: {}.{}({} args) {}/{} {}/{}", - String::from_utf8(pallet.clone()) - .expect("Encoded from String; qed"), - String::from_utf8(extrinsic.clone()) - .expect("Encoded from String; qed"), + pallet_str, + extrinsic_str, components.len(), s + 1, // s starts at 0. all_components.len(), @@ -701,12 +705,14 @@ impl PalletCmd { Vec, Vec<(BenchmarkParameter, u32, u32)>, Vec<(String, String)>, + String, + String, )>, ) -> Result { use std::collections::hash_map::Entry; let mut parsed = PovModesMap::new(); - for (pallet, call, _components, pov_modes) in benchmarks { + for (pallet, call, _components, pov_modes, _, _) in benchmarks { for (pallet_storage, mode) in pov_modes { let mode = PovEstimationMode::from_str(&mode)?; let splits = pallet_storage.split("::").collect::>(); @@ -760,14 +766,16 @@ fn list_benchmark( Vec, Vec<(BenchmarkParameter, u32, u32)>, Vec<(String, String)>, + String, + String, )>, ) { // Sort and de-dub by pallet and function name. - benchmarks_to_run.sort_by(|(pa, sa, _, _), (pb, sb, _, _)| (pa, sa).cmp(&(pb, sb))); - benchmarks_to_run.dedup_by(|(pa, sa, _, _), (pb, sb, _, _)| (pa, sa) == (pb, sb)); + benchmarks_to_run.sort_by(|(pa, sa, _, _, _, _), (pb, sb, _, _, _, _)| (pa, sa).cmp(&(pb, sb))); + benchmarks_to_run.dedup_by(|(pa, sa, _, _, _, _), (pb, sb, _, _, _, _)| (pa, sa) == (pb, sb)); println!("pallet, benchmark"); - for (pallet, extrinsic, _, _) in benchmarks_to_run { - println!("{}, {}", String::from_utf8_lossy(&pallet), String::from_utf8_lossy(&extrinsic)); + for (_, _, _, _, pallet, extrinsic) in benchmarks_to_run { + println!("{}, {}", pallet, extrinsic); } } diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs index 9493a693bbed..1cc19d2d7b70 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs @@ -571,19 +571,22 @@ pub(crate) fn process_storage_results( let mut prefix_result = result.clone(); let key_info = storage_info_map.get(&prefix); + let pallet_name = match key_info { + Some(k) => String::from_utf8(k.pallet_name.clone()).expect("encoded from string"), + None => "".to_string(), + }; + let storage_name = match key_info { + Some(k) => String::from_utf8(k.storage_name.clone()).expect("encoded from string"), + None => "".to_string(), + }; let max_size = key_info.and_then(|k| k.max_size); let override_pov_mode = match key_info { - Some(StorageInfo { pallet_name, storage_name, .. }) => { - let pallet_name = - String::from_utf8(pallet_name.clone()).expect("encoded from string"); - let storage_name = - String::from_utf8(storage_name.clone()).expect("encoded from string"); - + Some(_) => { // Is there an override for the storage key? - pov_modes.get(&(pallet_name.clone(), storage_name)).or( + pov_modes.get(&(pallet_name.clone(), storage_name.clone())).or( // .. or for the storage prefix? - pov_modes.get(&(pallet_name, "ALL".to_string())).or( + pov_modes.get(&(pallet_name.clone(), "ALL".to_string())).or( // .. or for the benchmark? pov_modes.get(&("ALL".to_string(), "ALL".to_string())), ), @@ -662,13 +665,11 @@ pub(crate) fn process_storage_results( // writes. if !is_prefix_identified { match key_info { - Some(key_info) => { + Some(_) => { let comment = format!( "Storage: `{}::{}` (r:{} w:{})", - String::from_utf8(key_info.pallet_name.clone()) - .expect("encoded from string"), - String::from_utf8(key_info.storage_name.clone()) - .expect("encoded from string"), + pallet_name, + storage_name, reads, writes, ); @@ -699,10 +700,8 @@ pub(crate) fn process_storage_results( Some(new_pov) => { let comment = format!( "Proof: `{}::{}` (`max_values`: {:?}, `max_size`: {:?}, added: {}, mode: `{:?}`)", - String::from_utf8(key_info.pallet_name.clone()) - .expect("encoded from string"), - String::from_utf8(key_info.storage_name.clone()) - .expect("encoded from string"), + pallet_name, + storage_name, key_info.max_values, key_info.max_size, new_pov, @@ -711,13 +710,9 @@ pub(crate) fn process_storage_results( comments.push(comment) }, None => { - let pallet = String::from_utf8(key_info.pallet_name.clone()) - .expect("encoded from string"); - let item = String::from_utf8(key_info.storage_name.clone()) - .expect("encoded from string"); let comment = format!( "Proof: `{}::{}` (`max_values`: {:?}, `max_size`: {:?}, mode: `{:?}`)", - pallet, item, key_info.max_values, key_info.max_size, + pallet_name, storage_name, key_info.max_values, key_info.max_size, used_pov_mode, ); comments.push(comment); From 1b945d17ecb811c121caee85a039aa15513030b2 Mon Sep 17 00:00:00 2001 From: philoniare Date: Fri, 23 Feb 2024 00:02:45 +0800 Subject: [PATCH 2/5] feat: refactor new updates --- .../utils/frame/benchmarking-cli/src/pallet/command.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 87edc6b13e89..fb78fde4bfa4 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -781,11 +781,11 @@ fn list_benchmark( let mut benchmarks = BTreeMap::new(); // Sort and de-dub by pallet and function name. - benchmarks_to_run.iter().for_each(|(pallet, extrinsic, _, _, _, _)| { + benchmarks_to_run.iter().for_each(|(_, _, _, _, pallet_str, extrinsic_str)| { benchmarks - .entry(String::from_utf8_lossy(pallet).to_string()) + .entry(pallet_str) .or_insert_with(BTreeSet::new) - .insert(String::from_utf8_lossy(extrinsic).to_string()); + .insert(extrinsic_str); }); match list_output { From 96ec6355b540956a25d7fc08c5116f3edaddaf63 Mon Sep 17 00:00:00 2001 From: philoniare Date: Mon, 26 Feb 2024 18:13:57 +0800 Subject: [PATCH 3/5] feat: refactor due to review --- .../benchmarking-cli/src/pallet/command.rs | 33 +++++++------------ .../benchmarking-cli/src/pallet/writer.rs | 10 +++--- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index fb78fde4bfa4..b8b09bff09d6 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -299,21 +299,19 @@ impl PalletCmd { let benchmarks_to_run: Vec<_> = benchmarks_to_run .into_iter() .map(|(pallet, extrinsic, components, pov_modes)| { - let pallet_str = String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); - let extrinsic_str = String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); - let pov_modes_str = pov_modes.into_iter() - .map(|(p, s)| { - (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) - }) - .collect(); - + let pallet_name = String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); + let extrinsic_name = String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); ( pallet, extrinsic, components, - pov_modes_str, - pallet_str, - extrinsic_str, + pov_modes.into_iter() + .map(|(p, s)| { + (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) + }) + .collect(), + pallet_name, + extrinsic_name, ) }) .collect(); @@ -338,9 +336,7 @@ impl PalletCmd { for (pallet, extrinsic, components, _, pallet_str, extrinsic_str) in benchmarks_to_run.clone() { log::info!( target: LOG_TARGET, - "Starting benchmark: {}::{}", - pallet_str, - extrinsic_str, + "Starting benchmark: {pallet_str}::{extrinsic_str}" ); let all_components = if components.is_empty() { vec![Default::default()] @@ -422,10 +418,7 @@ impl PalletCmd { .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))? .map_err(|e| { format!( - "Benchmark {}::{} failed: {}", - pallet_str, - extrinsic_str, - e + "Benchmark {pallet_str}::{extrinsic_str} failed: {e}", ) })?; } @@ -500,9 +493,7 @@ impl PalletCmd { log::info!( target: LOG_TARGET, - "Running benchmark: {}.{}({} args) {}/{} {}/{}", - pallet_str, - extrinsic_str, + "Running benchmark: {pallet_str}.{extrinsic_str}({} args) {}/{} {}/{}", components.len(), s + 1, // s starts at 0. all_components.len(), diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs index 1cc19d2d7b70..55960137ea77 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs @@ -153,8 +153,8 @@ fn map_results( continue } - let pallet_string = String::from_utf8(batch.pallet.clone()).unwrap(); - let instance_string = String::from_utf8(batch.instance.clone()).unwrap(); + let pallet_name = String::from_utf8(batch.pallet.clone()).unwrap(); + let instance_name = String::from_utf8(batch.instance.clone()).unwrap(); let benchmark_data = get_benchmark_data( batch, storage_info, @@ -166,7 +166,7 @@ fn map_results( worst_case_map_values, additional_trie_layers, ); - let pallet_benchmarks = all_benchmarks.entry((pallet_string, instance_string)).or_default(); + let pallet_benchmarks = all_benchmarks.entry((pallet_name, instance_name)).or_default(); pallet_benchmarks.push(benchmark_data); } Ok(all_benchmarks) @@ -699,9 +699,7 @@ pub(crate) fn process_storage_results( ) { Some(new_pov) => { let comment = format!( - "Proof: `{}::{}` (`max_values`: {:?}, `max_size`: {:?}, added: {}, mode: `{:?}`)", - pallet_name, - storage_name, + "Proof: `{pallet_name}::{storage_name}` (`max_values`: {:?}, `max_size`: {:?}, added: {}, mode: `{:?}`)", key_info.max_values, key_info.max_size, new_pov, From 3e9899ae5c2bb6c8bdb463a484b1c3fe2761e72a Mon Sep 17 00:00:00 2001 From: philoniare Date: Mon, 26 Feb 2024 20:27:23 +0800 Subject: [PATCH 4/5] feat: rename vars to exclude type --- .../frame/benchmarking-cli/src/pallet/command.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index b8b09bff09d6..7afdca649e56 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -333,10 +333,10 @@ impl PalletCmd { let mut component_ranges = HashMap::<(Vec, Vec), Vec>::new(); let pov_modes = Self::parse_pov_modes(&benchmarks_to_run)?; - for (pallet, extrinsic, components, _, pallet_str, extrinsic_str) in benchmarks_to_run.clone() { + for (pallet, extrinsic, components, _, pallet_name, extrinsic_name) in benchmarks_to_run.clone() { log::info!( target: LOG_TARGET, - "Starting benchmark: {pallet_str}::{extrinsic_str}" + "Starting benchmark: {pallet_name}::{extrinsic_name}" ); let all_components = if components.is_empty() { vec![Default::default()] @@ -418,7 +418,7 @@ impl PalletCmd { .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))? .map_err(|e| { format!( - "Benchmark {pallet_str}::{extrinsic_str} failed: {e}", + "Benchmark {pallet_name}::{extrinsic_name} failed: {e}", ) })?; } @@ -493,7 +493,7 @@ impl PalletCmd { log::info!( target: LOG_TARGET, - "Running benchmark: {pallet_str}.{extrinsic_str}({} args) {}/{} {}/{}", + "Running benchmark: {pallet_name}::{extrinsic_name}({} args) {}/{} {}/{}", components.len(), s + 1, // s starts at 0. all_components.len(), @@ -772,11 +772,11 @@ fn list_benchmark( let mut benchmarks = BTreeMap::new(); // Sort and de-dub by pallet and function name. - benchmarks_to_run.iter().for_each(|(_, _, _, _, pallet_str, extrinsic_str)| { + benchmarks_to_run.iter().for_each(|(_, _, _, _, pallet_name, extrinsic_name)| { benchmarks - .entry(pallet_str) + .entry(pallet_name) .or_insert_with(BTreeSet::new) - .insert(extrinsic_str); + .insert(extrinsic_name); }); match list_output { From 855e81f503dd9f33df7f98c380a8c68d7b4a6dc8 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Mon, 26 Feb 2024 14:20:24 +0000 Subject: [PATCH 5/5] ".git/.scripts/commands/fmt/fmt.sh" --- .../benchmarking-cli/src/pallet/command.rs | 17 ++++++++++------- .../frame/benchmarking-cli/src/pallet/writer.rs | 5 +---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 7afdca649e56..c6ba28240167 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -299,13 +299,16 @@ impl PalletCmd { let benchmarks_to_run: Vec<_> = benchmarks_to_run .into_iter() .map(|(pallet, extrinsic, components, pov_modes)| { - let pallet_name = String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); - let extrinsic_name = String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); + let pallet_name = + String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); + let extrinsic_name = + String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); ( pallet, extrinsic, components, - pov_modes.into_iter() + pov_modes + .into_iter() .map(|(p, s)| { (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) }) @@ -333,7 +336,9 @@ impl PalletCmd { let mut component_ranges = HashMap::<(Vec, Vec), Vec>::new(); let pov_modes = Self::parse_pov_modes(&benchmarks_to_run)?; - for (pallet, extrinsic, components, _, pallet_name, extrinsic_name) in benchmarks_to_run.clone() { + for (pallet, extrinsic, components, _, pallet_name, extrinsic_name) in + benchmarks_to_run.clone() + { log::info!( target: LOG_TARGET, "Starting benchmark: {pallet_name}::{extrinsic_name}" @@ -417,9 +422,7 @@ impl PalletCmd { ) .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))? .map_err(|e| { - format!( - "Benchmark {pallet_name}::{extrinsic_name} failed: {e}", - ) + format!("Benchmark {pallet_name}::{extrinsic_name} failed: {e}",) })?; } // Do one loop of DB tracking. diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs index 55960137ea77..bd4b65d8a2e3 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs @@ -668,10 +668,7 @@ pub(crate) fn process_storage_results( Some(_) => { let comment = format!( "Storage: `{}::{}` (r:{} w:{})", - pallet_name, - storage_name, - reads, - writes, + pallet_name, storage_name, reads, writes, ); comments.push(comment) },