-
Notifications
You must be signed in to change notification settings - Fork 798
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
Cleanup String::from_utf8 #3446
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
bda08b5
feat: cleanup String::from_utf8
philoniare c4ada60
Merge branch 'master' into cleanup-from_utf8
philoniare 1b945d1
feat: refactor new updates
philoniare cebbaa6
Merge branch 'master' into cleanup-from_utf8
philoniare 96ec635
feat: refactor due to review
philoniare 48c0cc6
Merge branch 'master' into cleanup-from_utf8
philoniare 6256d3b
Merge branch 'master' into cleanup-from_utf8
philoniare 3e9899a
feat: rename vars to exclude type
philoniare ac489d4
Merge branch 'master' into cleanup-from_utf8
philoniare 855e81f
".git/.scripts/commands/fmt/fmt.sh"
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||||||
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,15 +665,10 @@ 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"), | ||||||
reads, | ||||||
writes, | ||||||
pallet_name, storage_name, reads, writes, | ||||||
); | ||||||
comments.push(comment) | ||||||
}, | ||||||
|
@@ -698,11 +696,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 +705,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: `{:?}`)", | ||||||
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, | ||||||
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); | ||||||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Originally i thought that already the types in
BenchmarkBatchSplitResults
andBenchmarkBatch
should be converted, but it would mean to duplicate or generitize the types.Guess this is a good start for now.