-
Notifications
You must be signed in to change notification settings - Fork 1.6k
remove Default from CandidateDescriptor #4484
remove Default from CandidateDescriptor #4484
Conversation
Co-authored-by: Andronik Ordian <write@reusable.software>
primitives/test-helpers/src/lib.rs
Outdated
|
||
/// Create meaningless dummy hash. | ||
pub fn dummy_hash() -> Hash { | ||
// TODO make this PRNG based |
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.
@drahnr are you ok with leaving this out for now and leaving for a follow up?
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.
It would make things a little more complicated because we would no longer be able to use this when we are asserting the output
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.
Please remove the TODOs, it's fine for right now, and I think we should introduce a separate fn with _rand
suffix
/benchmark runtime kusama runtime_parachains::paras_inherent |
Benchmark Runtime Kusama Pallet for branch "bernhard-avoid-default-for-bench-and-tests" with command cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_parachains::paras_inherent --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_parachains_paras_inherent.rs Results
|
…bernhard-avoid-default-for-bench-and-tests
…k --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_parachains::paras_inherent --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_parachains_paras_inherent.rs
// Standard Error: 26_000 | ||
.saturating_add((49_051_000 as Weight).saturating_mul(v as Weight)) | ||
.saturating_add(T::DbWeight::get().reads(29 as Weight)) | ||
(1_134_397_000 as Weight) |
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.
3x increase to the static weight here is quite significant
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 don't quite understand where that comes from
/benchmark runtime polkadot runtime_parachains::paras_inherent |
Benchmark Runtime Polkadot Pallet for branch "bernhard-avoid-default-for-bench-and-tests" with command cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=polkadot-dev --steps=50 --repeat=20 --pallet=runtime_parachains::paras_inherent --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/polkadot/src/weights/runtime_parachains_paras_inherent.rs Results
|
…k --chain=polkadot-dev --steps=50 --repeat=20 --pallet=runtime_parachains::paras_inherent --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/polkadot/src/weights/runtime_parachains_paras_inherent.rs
@@ -47,7 +47,7 @@ fn dummy_validation_code() -> ValidationCode { | |||
/// "features = runtime-benchmarks". | |||
fn account<AccountId: Decode + Default>(name: &'static str, index: u32, seed: u32) -> AccountId { | |||
let entropy = (name, index, seed).using_encoded(sp_io::hashing::blake2_256); | |||
AccountId::decode(&mut &entropy[..]).unwrap_or_default() | |||
AccountId::decode(&mut &entropy[..]).expect("256 bit input is valid. qed.") |
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.
This is fine for now, but in general it's best to use TrailZeroInput
to ensure that AccountId::decode
doesn't run out of input data. In the future, we should just have a trait function for fn from_entropy(e: impl Encode) -> Self
.
(201_142_000 as Weight) | ||
// Standard Error: 2_000 | ||
.saturating_add((304_000 as Weight).saturating_mul(v as Weight)) | ||
(404_051_000 as Weight) |
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.
ouch - 2x increase here :(
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.
It's unclear to what caused this, or if other changes on master
caused the change, to be checked.
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.
Overall, looks like a great improvement!
bot merge |
Waiting for commit status. |
bot merge |
Waiting for commit status. |
Introduces a set of helpers for testing to create
Candidate*
-types with ease. The purpose of these types twofold, 1) subsystem unit tests which are in need of unified fillers, that create types which can have arbitrary content 2) for benchmarking where the content has influence on the measured runtime.@emostov please take over and drive this to completion ASAP
TODOs
fn dummy() -> Self
implementations @emostov`sr25519_sign_version_1` called outside of an Externalities-provided environment.'
, needs a fixfn
s into the the test helper crate (i.e.CandidateBuilder
is a good candidate, there are 4 or 5 copies of that. AlsoSessionInfo
) @drahnr , can be defered to a follow up PRdummy_*
calls in there @emostovx: Default::default()
usage as much as possible @drahnrRef paritytech/substrate#10403
skip check-dependent-cumulus