Benchmark Ethereum Pallet #149
Conversation
Allows the benchmarking code to use this without having to pull it in from the mock.
|
For receipts I think the complexity could be Dump of my thoughts:
|
| // blocks has on our import time. As such we need to make sure that we keep the number of | ||
| // validators fixed while changing the number blocks finalized (the complixity parameter) by | ||
| // importing the last header. | ||
| import_unsigned_finality { |
There was a problem hiding this comment.
One last thing I forgot to add to my top-level comment - imo weight(import_signed_headers) is actually weight(import_unsigned_header) * len(headers) + some_constant, where some_constant stands for recovering signature + iterating headers + invoking callbacks (which are noop in our runtime - not sure how to measure that in general case?). So we may only introduce benchmarks for unsigned version and use results for signed version.
|
So I read benchmarking doc and it suggests to use worst case estimate in weights computation. Worst case means many unfinalized blocks for us && it may be too much for regular header import. So my suggestion is to implement point 2 from above list (i.e. use this suggestion). The final weight would be: So we need benchmarks that Hernando has originally suggested to get these factors, then everything else should be quite easy to implement. |
Allows unit tests and benchmarks to access the same helper function and const
I don't think its entirely worth it to split out so few lines of code. The benches aren't particularly hard to read anyways.
|
I think this is ready for feedback now. One thing I want to point out is that I'm not sure if my usage of the complexity parameter is correct. For example, in the pruning and receipt benches it's not used at all. I don't see a way to use it meaningfully since both of these are fixed regardless of the number of blocks imported. I also did a first pass for the benchmarks using a
I haven't included any CI changes in this PR to |
As part of this change we have removed the hardcoded TEST_RECEIPT_ROOT and instead chose to calculate the receipt root on the fly. This will make tests and benches less fragile.
|
I've generated some plots. I first got the benchmarking datapoints as so: ./target/release/bridge-node benchmark --pallet bridge-eth-poa --extrinsic $extrinsic --steps 50 --repeat 20 --raw > $extrinsic.txtThen then I pasted the contents of the file into here. Import Unsigned Header
Import Unsigned Finality
Import Unsigned Finality with Cache
Import Unsigned Pruning
Import Unsigned Receipts
The finality cache plot is my personal favourite, lol. |
|
The plots looks good! So it seems that either things are constant (or at least amortized) or linear, which is really good. Can we plot our formulas on top of this to make sure we have an upper bound? Then let's get this merged and deployed :) |
* Add skeleton for worst case import_unsigned_header * Fix a typo * Add benchmark test for best case unsigned header import * Add finality verification to worst case bench * Move `insert_header()` from mock to test_utils Allows the benchmarking code to use this without having to pull it in from the mock. * Add a rough bench to test a finalizing a "long" chain * Try to use complexity parameter for finality bench * Improve long finality bench * Remove stray dot file * Remove old "worst" case bench * Scribble some ideas down for pruning bench * Prune headers during benchmarking * Clean up some comments * Make finality bench work for entire range of complexity parameter * Place initialization code into a function * Add bench for block finalization with caching * First attempt at bench with receipts * Try and trigger validator set change * Perform a validator set change during benchmarking * Move `validators_change_receipt()` to shared location Allows unit tests and benchmarks to access the same helper function and const * Extract a test receipt root into a constant * Clean up description of pruning bench * Fix cache and pruning tests * Remove unecessary `build_custom_header` usage * Get rid of warnings * Remove code duplication comment I don't think its entirely worth it to split out so few lines of code. The benches aren't particularly hard to read anyways. * Increase the range of the complexity parameter * Use dynamic number of receipts while benchmarking As part of this change we have removed the hardcoded TEST_RECEIPT_ROOT and instead chose to calculate the receipt root on the fly. This will make tests and benches less fragile. * Prune a dynamic number of headers
* Add skeleton for worst case import_unsigned_header * Fix a typo * Add benchmark test for best case unsigned header import * Add finality verification to worst case bench * Move `insert_header()` from mock to test_utils Allows the benchmarking code to use this without having to pull it in from the mock. * Add a rough bench to test a finalizing a "long" chain * Try to use complexity parameter for finality bench * Improve long finality bench * Remove stray dot file * Remove old "worst" case bench * Scribble some ideas down for pruning bench * Prune headers during benchmarking * Clean up some comments * Make finality bench work for entire range of complexity parameter * Place initialization code into a function * Add bench for block finalization with caching * First attempt at bench with receipts * Try and trigger validator set change * Perform a validator set change during benchmarking * Move `validators_change_receipt()` to shared location Allows unit tests and benchmarks to access the same helper function and const * Extract a test receipt root into a constant * Clean up description of pruning bench * Fix cache and pruning tests * Remove unecessary `build_custom_header` usage * Get rid of warnings * Remove code duplication comment I don't think its entirely worth it to split out so few lines of code. The benches aren't particularly hard to read anyways. * Increase the range of the complexity parameter * Use dynamic number of receipts while benchmarking As part of this change we have removed the hardcoded TEST_RECEIPT_ROOT and instead chose to calculate the receipt root on the fly. This will make tests and benches less fragile. * Prune a dynamic number of headers
The goal of this PR is to add benchmarks to the Ethereum bridge pallet. There are only two extrinsics we care about here:
import_unsigned_headers()andimport_signed_headers(). For each of these extrinsics we need to make sure that we cover the best and worst case run times.There are four main operations whose performance we should check each extrinsic:
I think we should benchmark each of these individually, and combine them later if needed to find the worst case performance.
If I've understood the benchmarking correctly we want to see how the run time is affected by the complexity parameter. For finalization I think we should use the number of blocks being finalized as the complexity parameter. For pruning it should be the number of blocks, but in practice it doesn't matter because we're limited to the number of blocks we can prune per import anyways (eight in our case). For receipts I'm not sure how the complexity parameter will relate to the bench. For re-orgs it would be the length of the chain being re-org'd.