Tips Benchmarking to be Runtime Agnostic#10368
Tips Benchmarking to be Runtime Agnostic#10368paritytech-processbot[bot] merged 4 commits intoparitytech:masterfrom
Conversation
bkchr
added
A0-please_review
kianenigma
left a comment
kianenigma
left a comment
There was a problem hiding this comment.
A bit worried that if the configs are too small, then the benchmark won't be able to capture the coefficients accurately. For example if Tippers::max_len is just a few dozens (because the council has a small size), running that with the default --step 50 might be a bit of an issue. Code looks correct to me.
gui1117
left a comment
gui1117
left a comment
There was a problem hiding this comment.
looks good to me, maybe better to improve doc so we know that constant mustn't be changed without running the benchmark.
A bit worried that if the configs are too small, then the benchmark won't be able to capture the coefficients accurately. For example if
Tippers::max_lenis just a few dozens (because the council has a small size), running that with the default--step 50might be a bit of an issue.
I think we should consider the benchmark only valid for the value given at benchmark. And people should run benchmark again when changing the values.
| for i in 0..n { | ||
| let (caller, _curator, _fee, value, reason) = setup_bounty::<T>(i, MAX_BYTES); | ||
| let (caller, _curator, _fee, value, reason) = | ||
| setup_bounty::<T>(i, T::MaximumReasonLength::get()); |
There was a problem hiding this comment.
maybe we should add in the doc of MaximumReasonLength that this value is used in benchmark, so changing it requires running the benchmark again.
(so it can't be changed without runtime upgrade).
| benchmarks! { | ||
| report_awesome { | ||
| let r in 0 .. MAX_BYTES; | ||
| let r in 0 .. T::MaximumReasonLength::get(); |
There was a problem hiding this comment.
similarly I think we should say something in the doc
| let r in 0 .. MAX_BYTES; | ||
| let t in 1 .. MAX_TIPPERS; | ||
| let r in 0 .. T::MaximumReasonLength::get(); | ||
| let t in 1 .. T::Tippers::max_len() as u32; |
There was a problem hiding this comment.
maybe it also worth mentionning it that ContainsLengthBound::max_len implementation of Tippers is used in benchmark.
|
bot merge |
|
Waiting for commit status. |
* use config traits instead of constants * bounties too * do bounties too * update docs
* use config traits instead of constants * bounties too * do bounties too * update docs
* use config traits instead of constants * bounties too * do bounties too * update docs
5 participants
Use the generic config traits in benchmarking instead of hard-coded constants so any runtime configuration can benchmark
pallet_tipsandpallet bounties