-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Cleanup String::from_utf8 #3446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
bda08b5
c4ada60
1b945d1
cebbaa6
96ec635
48c0cc6
6256d3b
3e9899a
ac489d4
855e81f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -298,16 +298,20 @@ impl PalletCmd { | |||||
| // Convert `Vec<u8>` 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_name = String::from_utf8(pallet.clone()).expect("Encoded from String; qed"); | ||||||
| let extrinsic_name = String::from_utf8(extrinsic.clone()).expect("Encoded from String; qed"); | ||||||
| ( | ||||||
| b.0, | ||||||
| b.1, | ||||||
| b.2, | ||||||
| b.3.into_iter() | ||||||
| pallet, | ||||||
| extrinsic, | ||||||
| components, | ||||||
| pov_modes.into_iter() | ||||||
| .map(|(p, s)| { | ||||||
| (String::from_utf8(p).unwrap(), String::from_utf8(s).unwrap()) | ||||||
| }) | ||||||
| .collect(), | ||||||
| pallet_name, | ||||||
| extrinsic_name, | ||||||
| ) | ||||||
| }) | ||||||
| .collect(); | ||||||
|
|
@@ -329,12 +333,10 @@ impl PalletCmd { | |||||
| let mut component_ranges = HashMap::<(Vec<u8>, Vec<u8>), Vec<ComponentRange>>::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"), | ||||||
| "Starting benchmark: {pallet_str}::{extrinsic_str}" | ||||||
| ); | ||||||
| let all_components = if components.is_empty() { | ||||||
| vec![Default::default()] | ||||||
|
|
@@ -416,10 +418,7 @@ impl PalletCmd { | |||||
| .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))? | ||||||
| .map_err(|e| { | ||||||
| format!( | ||||||
| "Benchmark {}::{} failed: {}", | ||||||
| String::from_utf8_lossy(&pallet), | ||||||
| String::from_utf8_lossy(&extrinsic), | ||||||
| e | ||||||
| "Benchmark {pallet_str}::{extrinsic_str} failed: {e}", | ||||||
| ) | ||||||
| })?; | ||||||
| } | ||||||
|
|
@@ -494,11 +493,7 @@ 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"), | ||||||
| "Running benchmark: {pallet_str}.{extrinsic_str}({} args) {}/{} {}/{}", | ||||||
|
||||||
| "Running benchmark: {pallet_str}.{extrinsic_str}({} args) {}/{} {}/{}", | |
| "Running benchmark: {pallet_str}::{extrinsic_str}({} args) {}/{} {}/{}", |
Noticed recently that it should be consistent with the print above.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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(); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Originally i thought that already the types in |
||||||
| 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) | ||||||
|
|
@@ -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, | ||||||
| ); | ||||||
|
|
@@ -698,11 +699,7 @@ 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"), | ||||||
| "Proof: `{pallet_name}::{storage_name}` (`max_values`: {:?}, `max_size`: {:?}, added: {}, mode: `{:?}`)", | ||||||
| key_info.max_values, | ||||||
| key_info.max_size, | ||||||
| new_pov, | ||||||
|
|
@@ -711,13 +708,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: `{:?}`)", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| pallet, item, key_info.max_values, key_info.max_size, | ||||||
| pallet_name, storage_name, key_info.max_values, key_info.max_size, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| used_pov_mode, | ||||||
| ); | ||||||
| comments.push(comment); | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Types dont need to be in the var name.