Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upConstant Weight Signature in Benchmarking CLI #7233
Conversation
|
@shawntabrizi please merge master |
|
PR is good to me, |
| @@ -159,9 +159,9 @@ decl_module! { | |||
| /// The dispatch origin for this call must be `Inherent`. | |||
| /// | |||
| /// # <weight> | |||
| /// - `O(T)` where `T` complexity of `on_timestamp_set` | |||
| /// - `O(1)` (Note that implementations of `OnTimestampSet` must also be `O(1)`) | |||
thiolliere
Oct 14, 2020
Contributor
Even if OnTimestampSet is O(1) the benchmark might miss some read/writes and base cost.
benchmark only call on_timestamp_set(100), but maybe on_timestamp_set(100) is 1 storage read whereas on_timestamp_set(200) will trigger some storage writes because some period are over or something.
like if a module do 100 storage writes every minutes. (obviously in this case such a pallet should be refactored to use on_initialize, but my point is that such implementation is O(1)).
maybe we can document that on_timestamp_set should always cost its worst case, although this is an annoying constraint.
or maybe I miss something in the picture here.
(note aside, I don't think this is that much of an issue here, I just wonder about general usage of handler)
shawntabrizi
Oct 15, 2020
•
Author
Member
O(1) means constant time right? i.e. this function should always have the same weight independent of the input.
thiolliere
Oct 15, 2020
Contributor
I think constant time means there is an upper bound independant from input.
the following code is constant time:
fn foo(a: u32) {
if a > 10 {
do 2 writes
}
}
|
bot merge |
|
Trying merge. |
parity-processbot
bot
merged commit 900d034
into
master
polkadot companion: paritytech/polkadot#1791
This PR fixes a dumb choice I made about hiding variables when they are not used in the weight formula at the end of the benchmarking pipeline.
What I realize now is that our fitting algorithm is not perfect, and may associate some weight to a component depending on the results of the benchmark. If it does this one time, and not another time, then the entire signature of the weight formula would change, making it incompatible at the trait definition layer.
Instead of removing the variable from the function definition, I simply omit usage of the variable by adding a
_.This will keep the function signature the same, but also be a clear indicator that the variable is not being used.
If a variable does not need to be included in the formula at all, it is on the benchmark writer to omit this as one of the components.