-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Running this 1000 times with a full block takes ~33 minutes 🙈. Reducing it to ~3 minutes per default. Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
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.
Nice addition. Looking forward to trying it out.
utils/frame/benchmarking-cli/src/extrinsic/extrinsic_factory.rs
Outdated
Show resolved
Hide resolved
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
The .to_lowercase() on the builder is actually not needes since its already documented to only return lower case. Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
let record = self.measure_block(&block, num_ext, bench_type)?; | ||
pub fn bench(&self, ext_builder: Option<&dyn ExtrinsicBuilder>) -> Result<Stats> { | ||
let (block, num_ext) = self.build_block(ext_builder)?; | ||
let record = self.measure_block(&block, num_ext)?; |
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.
You don't remove the block execution time of an empty block. Meaning you don't really measure the extrinsic execution time.
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.
Yes this is a simplification in the code. An empty block executes in about 5.3ms, which is only 0.265% of a 2s block and therefore negligible.
I could still change it to measure an empty block first, do you think its worth it?
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.
How do you know for every runtime what it takes to execute an empty block? :P
Yes, please do this.
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.
I updated it to first measure an empty block and then subtract the average of that from the full block to account for the inherent timings and execution overhead. @bkchr
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
This should already be the case since --dev is passed but somehow not... Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
bot merge |
* Benchmark extrinsic Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Reduce warmup and repeat Running this 1000 times with a full block takes ~33 minutes 🙈. Reducing it to ~3 minutes per default. Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Make ExistentialDeposit public Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add 'bechmark extrinsic' command Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * fmt Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add --list and cleanup Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Unrelated Clippy Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Clippy Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix tests and doc Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Move implementations up + fmt Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Dont use parameter_types macro Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Cache to_lowercase() call The .to_lowercase() on the builder is actually not needes since its already documented to only return lower case. Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Spelling Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Use correct nightly for fmt... Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Rename ED Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix compile Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Subtract block base weight Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Use tmp folder for test This should already be the case since --dev is passed but somehow not... Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix test Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
* Benchmark extrinsic Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Reduce warmup and repeat Running this 1000 times with a full block takes ~33 minutes 🙈. Reducing it to ~3 minutes per default. Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Make ExistentialDeposit public Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add 'bechmark extrinsic' command Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * fmt Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Add --list and cleanup Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Unrelated Clippy Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Clippy Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix tests and doc Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Move implementations up + fmt Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Dont use parameter_types macro Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Cache to_lowercase() call The .to_lowercase() on the builder is actually not needes since its already documented to only return lower case. Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Spelling Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Use correct nightly for fmt... Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Rename ED Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix compile Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Subtract block base weight Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fixes Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Use tmp folder for test This should already be the case since --dev is passed but somehow not... Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix test Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Makes it possible to benchmark the execution speed of an extrinsic. It does so by filling a block with these extrinsics and measures its execution time.
The results is then divided by the number of extrinsics and printed to console.
Example invocation:
Changes:
benchmark extrinsic <pallet> <extrinsic>
commandSystem::Remark
andBalances::TransferKeepAlive
benchmark overhead
to re-use the logicWIP:
Document that it is non-endowing, non-reaping and currently always the same sender and receiver accountDo some more lenient input transformation (eg. lower-case, trim-
and_
)Polkadot companion: paritytech/polkadot#5620
Cumulus companion: paritytech/cumulus#1350