Exchange pallet benchmarks #158
Conversation
svyatonik
force-pushed
the
exchange-benchmarks
branch
from
50f2268
to
74d04a2
Compare
svyatonik
force-pushed
the
exchange-benchmarks
branch
from
74d04a2
to
35939fb
Compare
|
(rebased on master. will leave it open until monday) |
modules/ethereum/src/test_utils.rs
Outdated
| @@ -206,6 +223,20 @@ where | |||
| custom_header.sign_by(author) | |||
| } | |||
|
|
|||
| /// Insert unverified header into storage. | |||
| pub fn insert_header<S: Storage>(storage: &mut S, header: Header) { | |||
There was a problem hiding this comment.
I made the same move in my PR, lol
| @@ -117,9 +117,9 @@ pub struct UnsignedTransaction { | |||
| pub gas: U256, | |||
| /// Transaction destination address. None if it is contract creation transaction. | |||
| pub to: Option<Address>, | |||
| /// Transaction value. | |||
| /// Value. | |||
There was a problem hiding this comment.
This doesn't really add much value in terms of describing what "value" is. I'd beef up the description a bit
| pub value: U256, | ||
| /// Transaction payload. | ||
| /// Associated data. |
| add_benchmark!(params, batches, b"bridge-eth-poa", BridgeEthPoA); | ||
| add_benchmark!(params, batches, b"bridge-currency-exchange", BridgeCurrencyExchangeBench::<Runtime>); |
There was a problem hiding this comment.
Not relevant for this PR, but there was recently a change introduced in Substrate that changed the arguments to this macro. We'll need to update that next time we bump Substrate
|
|
||
| const SEED: u32 = 0; | ||
| const WORST_TX_SIZE_FACTOR: u32 = 1000; | ||
| const WORST_PROOF_SIZE_FACTOR: u32 = 1000; |
There was a problem hiding this comment.
I think 1000 is fine, we just need to see how the time to run the extrinsic scales as this increases anyways
|
There do seem to be some problems with the CI (it doesn't look like they're caused by this PR). Should we wait until those are addressed to merge this? |
| // * Transaction has large size. | ||
| // * Recipient account does not exists. | ||
| import_peer_transaction_worst_case { | ||
| let i in 1..100; |
There was a problem hiding this comment.
Can we name the one-letter variables to something more descriptive? The code now is pretty small, but if it grows over time it might get harder to read.
* exchange benchmarks: framework * updated comment about tx size Co-authored-by: Tomasz Drwięga <tomasz@parity.io>
* exchange benchmarks: framework * updated comment about tx size Co-authored-by: Tomasz Drwięga <tomasz@parity.io>
WIP: should be rebased on master once #155 is reviewed + merged
part of #78
The main difficulty with benchmarking this pallet is that it is too generic && I was unable to write any runtime specific code there. So there's extended
Traitandfn Trait::make_proof(), which asks runtime to prepare environment+proof for executing import extrinsic.There are now 5 benchmarks there:
import_peer_transaction_best_case- imports minimal proof (transaction itself), transaction has minimal size (see below) and recipient account exists;import_peer_transaction_when_recipient_does_not_exists- the same as (1), but recipient account is created during import;import_peer_transaction_when_transaction_size_increases- test runtime only accepts transactions of minimal size (where data field is 32-bytes pub key of Substrate' recipient), so this is the same as (1);import_peer_transaction_when_proof_size_increases- the same as (1), but proof size changes from 1 to 1000. Proof size here = number of transactions in PoA block. So worst case is 1000 txs/block. If you think we should have another value for that - please share your thoughts;import_peer_transaction_worst_case- recipient account does not exists, proof size is 1000 transactions, every transaction has maximal size (though in test runtime it it the same minimal size).