Update consensus runtime transaction benchmark #2990
Conversation
crates/pallet-domains/src/weights.rs
Outdated
| .saturating_add(T::DbWeight::get().writes(13_u64)) | ||
| // Measured: `1751` | ||
| // Estimated: `10166` | ||
| // Minimum execution time: 150_394_000 picoseconds. |
There was a problem hiding this comment.
Any idea why this has almost doubled in time?
There was a problem hiding this comment.
My guess is:
- CPU changed, the previous benchmarks were running on mac chip
- The benchmark hasn't run for a while and there are maybe some changes in the code that bring more weight.
There was a problem hiding this comment.
The benchmark hasn't run for a while and there are maybe some changes in the code that bring more weight.
This does seem related, I run the benchmark on my machine and it is Weight::from_parts(119_000_000, 10166) now.
There was a problem hiding this comment.
4vcpu (2 cores) and 8GB RAM
Is not a reference machine, it is a very rough high-level guideline. Such description can correspond to machines with 5x discrepancy in performance. We need to have an actual CPU and RAM specs defined here at least for benchmarking purposes. We also want physical machine here, not VM, such that we avoid interference of shared resources in order to get reproducible benchmarking results.
7R13 is OEM-only server CPU, not really representative of consumer chips.
Also, the benchmark of set_pot_slot_iterations in the pallet-subspace seems missing so there is no weight generated for this extrinsic and I simply use the previous set_pot_slot_iterations weight
Why not adding corresponding benchmark then?
| @@ -17,7 +17,7 @@ | |||
|
|
|||
| #![cfg_attr(not(feature = "std"), no_std)] | |||
| #![forbid(unsafe_code)] | |||
| #![warn(rust_2018_idioms, missing_debug_implementations)] | |||
There was a problem hiding this comment.
Why removing lint, was it not possible to fix?
There was a problem hiding this comment.
Simply adding a #[derive(Debug)] can fix it but I don't think this lint is necessary.
I agree, do you have any idea how we can get "We need to have an actual CPU and RAM specs defined here at least for benchmarking purposes" done? cc @nazar-pc @dariolina
I did look at the PR that brings this extrinsic and its weight but didn't see the benchmark there, is it deleted by accident and I need to write one? |
Yes, we need to take a reasonable physical machine and then possibly constrain (with something like
There was likely none added from the very beginning, but the worst-case should be trivial there, so worth adding corresponding benchmark. |
|
Looking at our telemetry, the most basic consumer one ppl use is Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz with 2.8% users |
Yes, among the most popular options. However, we also have 27% of Other+Unknown. |
NingLin-P
force-pushed
the
update-consensus-benchmark
branch
from
4e1fd3f to
90fb06a
Compare
…Core(TM) i7-6700 CPU @ 3.40GHz Signed-off-by: linning <[email protected]>
…fter_message_is_sent Signed-off-by: linning <[email protected]>
NingLin-P
force-pushed
the
update-consensus-benchmark
branch
from
90fb06a to
0f6f9d9
Compare
|
Force pushed to rebase on main and update the benchmark result from the reference CPU |
|
What memory type and speed was it? What SSD (ideally model)? |
Perhaps @DaMandal0rian has more precise info. |
@NingLin-P @nazar-pc here are the hardware details: |
This PR updates the consensus runtime benchmark (the extrinsic weight) with the benchmark result on following hardware:
See the diff in each
weight.rsfile for the change of weight when running on the reference machine.Also, the benchmark of
set_pot_slot_iterationsin thepallet-subspaceseems missing so there is no weight generated for this extrinsic and I simply use the previousset_pot_slot_iterationsweight cc @nazar-pcCode contributor checklist: