Simplify registrar benchmarks #566
Conversation
| 191, 27, 227, 32, 0, 0, 0, 0, 0, 0, 0, 0, 128, 254, 108, 203, 37, 75, 132, 240, 210, 32, 46, | ||
| 181, 5, 189, 223, 159, 84, 203, 158, 189, 15, 178, 113, 144, 114, 233, 46, 229, 124, 29, 161, | ||
| 216, 9, | ||
| 0, 55, 133, 53, 79, 12, 10, 168, 38, 54, 66, 50, 252, 54, 227, 253, 113, 215, 29, 209, 8, 221, |
There was a problem hiding this comment.
This has changed because previously I used let account = ALICE; below, but that doesn't work because of an invalid signature error when running:
cargo test -p pallet-registrar --features=runtime-benchmarks
I didn't notice before because this command is not being run in CI
| @@ -64,20 +65,60 @@ mod benchmarks { | |||
| } | |||
| } | |||
|
|
|||
| /// Creates a `ContainerChainGenesisData` with encoded size very near to `max_encoded_size`, and | |||
| /// with the provided number of keys. | |||
| fn max_size_genesis_data<T: Config>( | |||
There was a problem hiding this comment.
This function is needed because setting the storage to T::MaxGenesisDataSize::get() results in an encoded size greater than that, because the encoded size also includes fields other than the storage, such as id, name, extensions, etc. That overhead was around 80 bytes if I remember correctly.
| ) -> Result<(), BenchmarkError> { | ||
| // This extrinsic is disabled in flashbox runtime, return 0 weight there | ||
| let _origin = T::RegisterWithRelayProofOrigin::try_successful_origin() | ||
| .map_err(|_| BenchmarkError::Weightless)?; |
There was a problem hiding this comment.
I can't use this origin below because the signature is only valid for the account created by create_funded_user below, but if this returns an error it means that the extrinsic cannot be called, so we return a weight of zero.
There was a problem hiding this comment.
nice! then the benchmarks dont fail right?
There was a problem hiding this comment.
Correct, I have run them and there is a weight of 0 in flashbox, and non-zero in dancebox
| @@ -375,7 +447,7 @@ mod benchmarks { | |||
| } | |||
|
|
|||
| // Second loop to fill RegisteredParaIds to its maximum | |||
| for k in 1000..(1000 + y) { | |||
| for k in 1000..(1000 + y - 1) { | |||
There was a problem hiding this comment.
This -1 is because if we fill RegisteredParaIds to its maximum, the call below will fail because the list is full, so we need to leave space for 1 chain.
There was a problem hiding this comment.
Maybe it's good to add this as a comment in the code?
tmpolaczyk
added
B7-runtimenoteworthy
Coverage Report@@ Coverage Diff @@
## master tomasz-benchmarks-less-params +/- ##
=================================================================
+ Coverage 69.20% 69.32% +0.12%
Files 224 224
- Lines 39497 39347 -150
=================================================================
- Hits 27331 27274 -57
- Misses 12166 12073 -93
|
tools/dev-benchmark-pallet.sh
Outdated
Remove unused parameters from pallet_registrar benchmarks.
For instance, most extrinsics had a param "y: number of registered chains" that was always hardcoded to
T::MaxLengthParaIds::get(). While making the benchmark parametric provides useful information about whether the weight depends on the number of chains or not, it also increases the time it takes to run benchmarks by a significant factor, so at the end it is not worth it.Also add a script to automatically run benchmarks and update weights for the pallet and both runtimes, previously you had to copy the weights manually. I hope this will improve developer experience when dealing with benchmarks.