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

Weights for enacting candidates in paras inherent #4172

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

Conversation

emostov
Copy link
Contributor

@emostov emostov commented Oct 28, 2021

This PR

  • bubbles up the weight from the enact_candidate call and adds it to the returned weight of paras inherent enter. Previously we where not tracking the weight from enacting candidates.
  • creates enact_candidate_weight, which gives us the worst case weight which we can then use prior to calling enter.

cc @pepyakin

Notes for reviewers:

  • for people familiar with the FRAME storage apis: please focus on making sure the hardcoded storage read / write numbers look correct
  • for people familiar with the parachains runtime: please focus on making sure the way the weights from enact_candidate are used is correct

relates to #4044

skip check-dependent-cumulus

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Oct 28, 2021
primitives/src/v1/mod.rs Outdated Show resolved Hide resolved
primitives/src/v1/mod.rs Outdated Show resolved Hide resolved
/// Worst case weight for queue outbound hrmp.
pub(crate) fn queue_outbound_hrmp_weight(hrmp_max_message_num_per_candidate: u32) -> Weight {
let read_writes = (2 * hrmp_max_message_num_per_candidate).into();
T::DbWeight::get().reads_writes(read_writes, read_writes)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

	// - in loop
	//	- get 1r,
	// 	- insert 1w
	// 	- append 1w
	// 	- get 1r
	// 	- insert 1r

@@ -1002,6 +1002,11 @@ impl<T: Config> Pallet<T> {
})
}

/// Worst case weight for `schedule_code_upgrade`.
pub(crate) fn schedule_code_upgrade_weight() -> Weight {
T::DbWeight::get().reads_writes(2 + 1, 3 + 2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

	// - mutate 1r,1w
	// - insert 1r
	// - mutate 1r, 1w
	// - write 1w
	// - mutate, insert  1r,2w
	// - insert 1w

@@ -203,6 +203,10 @@ impl<T: Config> Pallet<T> {
T::DbWeight::get().reads_writes(1, 1)
}

pub(crate) fn prune_dmq_weight() -> Weight {
T::DbWeight::get().reads_writes(1, 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

	// - 1 mutate.

@@ -1040,6 +1045,11 @@ impl<T: Config> Pallet<T> {
}
}

/// Worst case weight for `note_new_head`.
pub(crate) fn note_new_head_weight() -> Weight {
T::DbWeight::get().reads_writes(2 + 3, 3 + 1 + 3)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

	// - insert 1w
	// - remove 1r
	// - remove 1r
	// - take 1r,1w
	// - get 1r
	// - insert 1w
	// - deposit_log (append) 1w
	// - note_past_code
	// 	- mutate 1r,1w
	//	- insert 1w
	//	- mutate 1r, 1w

.max(hrmp_max_parathread_inbound_channels).into();

T::DbWeight::get()
.reads_writes(1 + 2 * max_pruneable_channels, 1 + 1 + 2 * max_pruneable_channels)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

	// - mutate 1r,1w
	// - max_pruneable_channels * {insert || remove, mutate, get} r2, w2
	// - insert w1

} else {
0
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

	// - mutate
	// - mutate
	// - mutate

}
})
}

/// Worst case weight for `schedule_code_upgrade`.
pub(crate) fn schedule_code_upgrade_weight() -> Weight {
T::DbWeight::get().reads_writes(4, 5)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

	// - mutate 1r, 1w
	// - mutate 1r, 1w
	// - insert 1w
	// - mutate 1r, 1w
	// - deposit_log (append) 1w
	// - increase_code_ref 1r, 2w
	// - insert 1w

Copy link
Contributor Author

Choose a reason for hiding this comment

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

^ these are notes about how I counted reads and writes

@emostov emostov requested a review from pepyakin October 29, 2021 17:10
@emostov emostov added 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. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Oct 29, 2021
Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

Previous review was moot, only saw part of the diff.

Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

Two follow up issues needed, other than that, LGTM! 👍 Thank you!

@drahnr drahnr self-assigned this Feb 23, 2022
@emostov
Copy link
Contributor Author

emostov commented Apr 22, 2022

@drahnr Can this be closed?

@drahnr
Copy link
Contributor

drahnr commented Apr 22, 2022

I think it's still desirable to account for the enacted candidates, but it depends how much work that'd entail to update vs reimpl?

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. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
Development

Successfully merging this pull request may close these issues.

5 participants