Skip to content

Commit dcc83ae

Browse files
shaunxwshaunxw
authored
More benchmarking update (#470)
* Show benchmark debug info. * Use current crate dependencies. * Make clippy happy.
1 parent 45d55b7 commit dcc83ae

File tree

2 files changed

+93
-33
lines changed

2 files changed

+93
-33
lines changed

‎benchmarking/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ sp-runtime-interface = { git = "https://github.com/paritytech/substrate", branch
1616
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1", default-features = false }
1717
sp-std = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1", default-features = false }
1818
sp-io = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1", default-features = false }
19+
sp-storage = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1", default-features = false }
1920
frame-support = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1", default-features = false }
2021
frame-system = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1", default-features = false }
2122
frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1", default-features = false }

‎benchmarking/src/lib.rs

+92-33
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,22 @@ mod tests;
77

88
pub use frame_benchmarking::{
99
benchmarking, whitelisted_caller, BenchmarkBatch, BenchmarkConfig, BenchmarkParameter, BenchmarkResults,
10-
Benchmarking, BenchmarkingSetup, TrackedStorageKey,
10+
Benchmarking, BenchmarkingSetup,
1111
};
1212
#[cfg(feature = "std")]
1313
pub use frame_benchmarking::{Analysis, BenchmarkSelector};
14+
#[doc(hidden)]
1415
pub use frame_support;
16+
#[doc(hidden)]
1517
pub use paste;
1618
#[doc(hidden)]
1719
pub use sp_io::storage::root as storage_root;
20+
#[doc(hidden)]
1821
pub use sp_runtime::traits::Zero;
19-
22+
#[doc(hidden)]
23+
pub use sp_std::{self, boxed::Box, prelude::Vec, vec};
24+
#[doc(hidden)]
25+
pub use sp_storage::TrackedStorageKey;
2026
/// Construct pallet benchmarks for weighing dispatchables.
2127
///
2228
/// Works around the idea of complexity parameters, named by a single letter
@@ -551,8 +557,8 @@ macro_rules! benchmark_backend {
551557
struct $name;
552558
#[allow(unused_variables)]
553559
impl $crate::BenchmarkingSetup<$runtime $(, $instance)?> for $name {
554-
fn components(&self) -> Vec<($crate::BenchmarkParameter, u32, u32)> {
555-
vec! [
560+
fn components(&self) -> $crate::Vec<($crate::BenchmarkParameter, u32, u32)> {
561+
$crate::vec! [
556562
$(
557563
($crate::BenchmarkParameter::$param, $param_from, $param_to)
558564
),*
@@ -563,7 +569,7 @@ macro_rules! benchmark_backend {
563569
&self,
564570
components: &[($crate::BenchmarkParameter, u32)],
565571
verify: bool
566-
) -> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str> {
572+
) -> Result<$crate::Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str> {
567573
$(
568574
// Prepare instance
569575
let $param = components.iter()
@@ -577,7 +583,7 @@ macro_rules! benchmark_backend {
577583
$( $param_instancer ; )*
578584
$( $post )*
579585

580-
Ok(Box::new(move || -> Result<(), &'static str> {
586+
Ok($crate::Box::new(move || -> Result<(), &'static str> {
581587
$eval;
582588
if verify {
583589
$postcode;
@@ -620,7 +626,7 @@ macro_rules! selected_benchmark {
620626

621627
// Allow us to select a benchmark from the list of available benchmarks.
622628
impl $crate::BenchmarkingSetup<$runtime $(, $instance)?> for SelectedBenchmark {
623-
fn components(&self) -> Vec<($crate::BenchmarkParameter, u32, u32)> {
629+
fn components(&self) -> $crate::Vec<($crate::BenchmarkParameter, u32, u32)> {
624630
match self {
625631
$(
626632
Self::$bench => <
@@ -634,9 +640,7 @@ macro_rules! selected_benchmark {
634640
&self,
635641
components: &[($crate::BenchmarkParameter, u32)],
636642
verify: bool
637-
)
638-
-> Result<Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str>
639-
{
643+
) -> Result<$crate::Box<dyn FnOnce() -> Result<(), &'static str>>, &'static str> {
640644
match self {
641645
$(
642646
Self::$bench => <
@@ -662,8 +666,8 @@ macro_rules! impl_benchmark {
662666
pub struct Benchmark;
663667

664668
impl $crate::Benchmarking<$crate::BenchmarkResults> for Benchmark {
665-
fn benchmarks(extra: bool) -> Vec<&'static [u8]> {
666-
let mut all = vec![ $( stringify!($name).as_ref() ),* ];
669+
fn benchmarks(extra: bool) -> $crate::Vec<&'static [u8]> {
670+
let mut all = $crate::vec![ $( stringify!($name).as_ref() ),* ];
667671
if !extra {
668672
let extra = [ $( stringify!($name_extra).as_ref() ),* ];
669673
all.retain(|x| !extra.contains(x));
@@ -679,23 +683,23 @@ macro_rules! impl_benchmark {
679683
repeat: u32,
680684
whitelist: &[$crate::TrackedStorageKey],
681685
verify: bool,
682-
) -> Result<Vec<$crate::BenchmarkResults>, &'static str> {
686+
) -> Result<$crate::Vec<$crate::BenchmarkResults>, &'static str> {
683687
// Map the input to the selected benchmark.
684-
let extrinsic = sp_std::str::from_utf8(extrinsic)
688+
let extrinsic = $crate::sp_std::str::from_utf8(extrinsic)
685689
.map_err(|_| "`extrinsic` is not a valid utf8 string!")?;
686690
let selected_benchmark = match extrinsic {
687691
$( stringify!($name) => SelectedBenchmark::$name, )*
688692
_ => return Err("Could not find extrinsic."),
689693
};
690-
let mut results: Vec<$crate::BenchmarkResults> = Vec::new();
694+
let mut results: $crate::Vec<$crate::BenchmarkResults> = $crate::Vec::new();
691695
if repeat == 0 {
692696
return Ok(results);
693697
}
694698

695699
// Add whitelist to DB including whitelisted caller
696700
let mut whitelist = whitelist.to_vec();
697701
let whitelisted_caller_key =
698-
<frame_system::Account::<$runtime> as frame_support::storage::StorageMap<_,_>>::hashed_key_for(
702+
<frame_system::Account::<$runtime> as $crate::frame_support::storage::StorageMap<_,_>>::hashed_key_for(
699703
$crate::whitelisted_caller::<<$runtime as frame_system::Config>::AccountId>()
700704
);
701705
whitelist.push(whitelisted_caller_key.into());
@@ -715,7 +719,7 @@ macro_rules! impl_benchmark {
715719
let repeat_benchmark = |
716720
repeat: u32,
717721
c: &[($crate::BenchmarkParameter, u32)],
718-
results: &mut Vec<$crate::BenchmarkResults>,
722+
results: &mut $crate::Vec<$crate::BenchmarkResults>,
719723
verify: bool,
720724
| -> Result<(), &'static str> {
721725
// Run the benchmark `repeat` times.
@@ -742,7 +746,7 @@ macro_rules! impl_benchmark {
742746
closure_to_benchmark()?;
743747
} else {
744748
// Time the extrinsic logic.
745-
frame_support::log::trace!(
749+
$crate::frame_support::log::trace!(
746750
target: "benchmark",
747751
"Start Benchmark: {:?}", c
748752
);
@@ -764,12 +768,12 @@ macro_rules! impl_benchmark {
764768

765769
// Commit the changes to get proper write count
766770
$crate::benchmarking::commit_db();
767-
frame_support::log::trace!(
771+
$crate::frame_support::log::trace!(
768772
target: "benchmark",
769773
"End Benchmark: {} ns", elapsed_extrinsic
770774
);
771775
let read_write_count = $crate::benchmarking::read_write_count();
772-
frame_support::log::trace!(
776+
$crate::frame_support::log::trace!(
773777
target: "benchmark",
774778
"Read/Write Count {:?}", read_write_count
775779
);
@@ -802,7 +806,7 @@ macro_rules! impl_benchmark {
802806
if components.is_empty() {
803807
if verify {
804808
// If `--verify` is used, run the benchmark once to verify it would complete.
805-
repeat_benchmark(1, Default::default(), &mut Vec::new(), true)?;
809+
repeat_benchmark(1, Default::default(), &mut $crate::Vec::new(), true)?;
806810
}
807811
repeat_benchmark(repeat, Default::default(), &mut results, false)?;
808812
} else {
@@ -829,7 +833,7 @@ macro_rules! impl_benchmark {
829833
let component_value = lowest + step_size * s;
830834

831835
// Select the max value for all the other components.
832-
let c: Vec<($crate::BenchmarkParameter, u32)> = components.iter()
836+
let c: $crate::Vec<($crate::BenchmarkParameter, u32)> = components.iter()
833837
.enumerate()
834838
.map(|(idx, (n, _, h))|
835839
if n == name {
@@ -842,7 +846,7 @@ macro_rules! impl_benchmark {
842846

843847
if verify {
844848
// If `--verify` is used, run the benchmark once to verify it would complete.
845-
repeat_benchmark(1, &c, &mut Vec::new(), true)?;
849+
repeat_benchmark(1, &c, &mut $crate::Vec::new(), true)?;
846850
}
847851
repeat_benchmark(repeat, &c, &mut results, false)?;
848852
}
@@ -875,7 +879,7 @@ macro_rules! impl_benchmark_test {
875879
>::components(&selected_benchmark);
876880

877881
let execute_benchmark = |
878-
c: Vec<($crate::BenchmarkParameter, u32)>
882+
c: $crate::Vec<($crate::BenchmarkParameter, u32)>
879883
| -> Result<(), &'static str> {
880884
// Set up the benchmark, return execution + verification function.
881885
let closure_to_verify = <
@@ -901,9 +905,9 @@ macro_rules! impl_benchmark_test {
901905
} else {
902906
for (_, (name, low, high)) in components.iter().enumerate() {
903907
// Test only the low and high value, assuming values in the middle won't break
904-
for component_value in vec![low, high] {
908+
for component_value in $crate::vec![low, high] {
905909
// Select the max value for all the other components.
906-
let c: Vec<($crate::BenchmarkParameter, u32)> = components.iter()
910+
let c: $crate::Vec<($crate::BenchmarkParameter, u32)> = components.iter()
907911
.enumerate()
908912
.map(|(_, (n, _, h))|
909913
if n == name {
@@ -924,6 +928,39 @@ macro_rules! impl_benchmark_test {
924928
};
925929
}
926930

931+
/// show error message and debugging info for the case of an error happening
932+
/// during a benchmark
933+
#[allow(clippy::too_many_arguments)]
934+
pub fn show_benchmark_debug_info(
935+
instance_string: &[u8],
936+
benchmark: &[u8],
937+
lowest_range_values: &[u32],
938+
highest_range_values: &[u32],
939+
steps: &[u32],
940+
repeat: &u32,
941+
verify: &bool,
942+
error_message: &str,
943+
) -> sp_runtime::RuntimeString {
944+
sp_runtime::format_runtime_string!(
945+
"\n* Pallet: {}\n\
946+
* Benchmark: {}\n\
947+
* Lowest_range_values: {:?}\n\
948+
* Highest_range_values: {:?}\n\
949+
* Steps: {:?}\n\
950+
* Repeat: {:?}\n\
951+
* Verify: {:?}\n\
952+
* Error message: {}",
953+
sp_std::str::from_utf8(instance_string).expect("it's all just strings ran through the wasm interface. qed"),
954+
sp_std::str::from_utf8(benchmark).expect("it's all just strings ran through the wasm interface. qed"),
955+
lowest_range_values,
956+
highest_range_values,
957+
steps,
958+
repeat,
959+
verify,
960+
error_message,
961+
)
962+
}
963+
927964
/// This macro adds pallet benchmarks to a `Vec<BenchmarkBatch>` object.
928965
///
929966
/// First create an object that holds in the input parameters for the benchmark:
@@ -989,7 +1026,9 @@ macro_rules! add_benchmark {
9891026
if &pallet[..] == &b"*"[..] || &benchmark[..] == &b"*"[..] {
9901027
for benchmark in $( $location )*::Benchmark::benchmarks(*extra).into_iter() {
9911028
$batches.push($crate::BenchmarkBatch {
1029+
pallet: name_string.to_vec(),
9921030
instance: instance_string.to_vec(),
1031+
benchmark: benchmark.to_vec(),
9931032
results: $( $location )*::Benchmark::run_benchmark(
9941033
benchmark,
9951034
&lowest_range_values[..],
@@ -998,14 +1037,25 @@ macro_rules! add_benchmark {
9981037
*repeat,
9991038
whitelist,
10001039
*verify,
1001-
)?,
1002-
pallet: name_string.to_vec(),
1003-
benchmark: benchmark.to_vec(),
1004-
});
1040+
).map_err(|e| {
1041+
$crate::show_benchmark_debug_info(
1042+
instance_string,
1043+
benchmark,
1044+
lowest_range_values,
1045+
highest_range_values,
1046+
steps,
1047+
repeat,
1048+
verify,
1049+
e,
1050+
)
1051+
})?,
1052+
});
10051053
}
10061054
} else {
10071055
$batches.push($crate::BenchmarkBatch {
1056+
pallet: name_string.to_vec(),
10081057
instance: instance_string.to_vec(),
1058+
benchmark: benchmark.clone(),
10091059
results: $( $location )*::Benchmark::run_benchmark(
10101060
&benchmark[..],
10111061
&lowest_range_values[..],
@@ -1014,9 +1064,18 @@ macro_rules! add_benchmark {
10141064
*repeat,
10151065
whitelist,
10161066
*verify,
1017-
)?,
1018-
pallet: name_string.to_vec(),
1019-
benchmark: benchmark.clone(),
1067+
).map_err(|e| {
1068+
$crate::show_benchmark_debug_info(
1069+
instance_string,
1070+
benchmark,
1071+
lowest_range_values,
1072+
highest_range_values,
1073+
steps,
1074+
repeat,
1075+
verify,
1076+
e,
1077+
)
1078+
})?,
10201079
});
10211080
}
10221081
}

0 commit comments

Comments
 (0)