Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix weights for backed candidates #6929

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

eskimor
Copy link
Member

@eskimor eskimor commented Mar 21, 2023

In particular take into account ump and hrmp messages.

Update:

This does not work. Benchmarks are all over the place. This PR would result in a massive overestimate of the actual weight. In reality there is not much computation done based on these parameters, it is mostly block size that is affected - which is not measured by weight.

In particular the constant term gets massively over estimated and the linear terms also fluctuate a lot from run to run.

Anyhow, given that the constant term changes by a factor of 41 based on the upper bound of those linear parameters, some significant effect on weight seems to be there regardless - it likely is not linear though and the current benchmarking system has trouble providing meaningful/useful results.

For now limiting candidates based on size (to be done) will need to suffice.

Can hopefully be revived once paritytech/polkadot-sdk#194 is done.
With this in place as well we can also properly track size via v2 weights.

Another Update and Plan forward

Benchmarks are still bonkers, but I managed to get acceptable values (not too crazy high for the base case). Constant factors are mostly ignored now for CPU time, this should be acceptable if we limit by size.

Plan: Get size limiting, then merge this.

@eskimor eskimor added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Mar 21, 2023
@eskimor
Copy link
Member Author

eskimor commented Mar 21, 2023

bot bench $ runtime kusama runtime_parachains::paras_inherent

@command-bot
Copy link

command-bot bot commented Mar 21, 2023

@eskimor "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent (https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2562026) was cancelled in #6929 (comment)

@eskimor
Copy link
Member Author

eskimor commented Mar 21, 2023

bot cancel

@command-bot
Copy link

command-bot bot commented Mar 21, 2023

@eskimor Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2562026 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2562026/artifacts/download.

@paritytech paritytech deleted a comment from command-bot bot Mar 21, 2023
@eskimor
Copy link
Member Author

eskimor commented Mar 22, 2023

bot bench-all $ polkadot-all kusama

@command-bot
Copy link

command-bot bot commented Mar 22, 2023

@eskimor https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2567452 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" polkadot-all kusama. Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 5-3e09b7e0-145e-4c7f-bcd2-77f3d8d25eb4 to cancel this command or bot cancel to cancel all commands in this pull request.

@paritytech paritytech deleted a comment from command-bot bot Mar 22, 2023
@@ -116,7 +123,6 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
dispute_statements: BTreeMap::new(),
dispute_sessions: Default::default(),
backed_and_concluding_cores: Default::default(),
code_upgrade: None,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is now part of the candidates themselves.

@@ -221,8 +220,9 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {

/// Get the minimum number of validity votes in order for a backed candidate to be included.
#[cfg(feature = "runtime-benchmarks")]
pub(crate) fn fallback_min_validity_votes() -> u32 {
(Self::fallback_max_validators() / 2) + 1
pub(crate) fn fallback_min_backing_votes() -> u32 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed name, because backing votes is less ambiguous than validity_votes.

@@ -895,7 +895,7 @@ fn apply_weight_limit<T: Config + inclusion::Config>(
let (acc_candidate_weight, indices) =
random_sel::<BackedCandidate<<T as frame_system::Config>::Hash>, _>(
rng,
candidates.clone(),
&candidates,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this was cloned. Can be quite expensive for candidates.

@command-bot
Copy link

command-bot bot commented Mar 22, 2023

@eskimor Command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" polkadot-all kusama has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2567452 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2567452/artifacts/download.

runtime/parachains/src/builder.rs Show resolved Hide resolved
runtime/parachains/src/builder.rs Show resolved Hide resolved
@eskimor eskimor force-pushed the rk-fix-weights-backed-candidates branch from ef02aab to 44b7162 Compare March 23, 2023 13:51
@eskimor
Copy link
Member Author

eskimor commented Mar 24, 2023

bot bench $ runtime kusama runtime_parachains::paras_inherent

@command-bot
Copy link

command-bot bot commented Mar 24, 2023

"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent was queued.

Comment bot cancel 41-8ede39f0-18e3-413d-ab7d-c0f1b5966091 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Mar 24, 2023

@eskimor Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent has finished. Result:

ValidationError: "id" is required
ValidationError: "id" is required
{"message":{"base":["Reference not found"]}}

@command-bot
Copy link

command-bot bot commented Apr 25, 2023

@eskimor Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2728288 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2728288/artifacts/download.

@eskimor
Copy link
Member Author

eskimor commented Apr 26, 2023

bot bench $ runtime westend runtime_parachains::paras_inherent
bot bench $ runtime rococo runtime_parachains::paras_inherent
bot bench $ runtime kusama runtime_parachains::paras_inherent

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@eskimor Exception caught in webhook handler
EEXIST: file already exists, link '/app/generated/docs/d29a3d012fc468d6511e2e3e0c7f9c6e04bdf893.html' -> '/app/generated/docs/latest.html'

@eskimor
Copy link
Member Author

eskimor commented Apr 26, 2023

bot bench $ runtime westend runtime_parachains::paras_inherent
bot bench $ runtime rococo runtime_parachains::paras_inherent
bot bench $ runtime kusama runtime_parachains::paras_inherent

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@eskimor Exception caught in webhook handler
EEXIST: file already exists, link '/app/generated/docs/d29a3d012fc468d6511e2e3e0c7f9c6e04bdf893.html' -> '/app/generated/docs/latest.html'

@eskimor
Copy link
Member Author

eskimor commented Apr 26, 2023

bot bench $ runtime westend runtime_parachains::paras_inherent
bot bench $ runtime rococo runtime_parachains::paras_inherent
bot bench $ runtime kusama runtime_parachains::paras_inherent

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@eskimor Exception caught in webhook handler
EEXIST: file already exists, link '/app/generated/docs/d29a3d012fc468d6511e2e3e0c7f9c6e04bdf893.html' -> '/app/generated/docs/latest.html'

@eskimor
Copy link
Member Author

eskimor commented Apr 26, 2023

bot bench $ runtime westend runtime_parachains::paras_inherent
bot bench $ runtime rococo runtime_parachains::paras_inherent
bot bench $ runtime kusama runtime_parachains::paras_inherent

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@eskimor Exception caught in webhook handler
EEXIST: file already exists, link '/app/generated/docs/d29a3d012fc468d6511e2e3e0c7f9c6e04bdf893.html' -> '/app/generated/docs/latest.html'

@mordamax
Copy link
Contributor

bot bench $ runtime westend runtime_parachains::paras_inherent
bot bench $ runtime rococo runtime_parachains::paras_inherent
bot bench $ runtime kusama runtime_parachains::paras_inherent

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@mordamax https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737646 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime westend runtime_parachains::paras_inherent. Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-e21fcb6c-d07d-4720-b23e-50116246fbee to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@mordamax https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737647 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime rococo runtime_parachains::paras_inherent. Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 2-5e5ba5e0-7a8b-4942-9cdd-7ed1600f4284 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@mordamax https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737648 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent. Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 3-3888cc18-0a5a-430c-8740-14e2b286697f to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@mordamax Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime westend runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737646 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737646/artifacts/download.

@command-bot
Copy link

command-bot bot commented Apr 26, 2023

@mordamax Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime rococo runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737647 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737647/artifacts/download.

@command-bot
Copy link

command-bot bot commented Apr 27, 2023

@mordamax Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737648 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2737648/artifacts/download.

@eskimor
Copy link
Member Author

eskimor commented Apr 27, 2023

bot bench $ runtime polkadot runtime_parachains::paras_inherent
bot bench $ runtime rococo runtime_parachains::paras_inherent

@command-bot
Copy link

command-bot bot commented Apr 27, 2023

@eskimor https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2741604 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime polkadot runtime_parachains::paras_inherent. Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 9-2c948476-b633-4e9f-893b-e6f9d1c51efe to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 27, 2023

@eskimor https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2741605 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime rococo runtime_parachains::paras_inherent. Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 10-d37a8df2-d918-476d-815b-8f8c60884791 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 27, 2023

@eskimor Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime polkadot runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2741604 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2741604/artifacts/download.

@command-bot
Copy link

command-bot bot commented Apr 27, 2023

@eskimor Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" runtime rococo runtime_parachains::paras_inherent has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2741605 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2741605/artifacts/download.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants