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

Use BTreeSet as the internal type of ParachainsCache #6795

Merged
merged 3 commits into from
Feb 27, 2023

Conversation

antonva
Copy link
Contributor

@antonva antonva commented Feb 27, 2023

Reduces the complexity of batch operations on the parachains in storage as BTreeSet insert/remove is O(log(n)).

@antonva antonva requested a review from eskimor February 27, 2023 16:15
Copy link
Member

@eskimor eskimor left a comment

Choose a reason for hiding this comment

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

Nice work!

@ordian ordian 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Feb 27, 2023
@ordian
Copy link
Member

ordian commented Feb 27, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit c9f0ed2 into master Feb 27, 2023
@paritytech-processbot paritytech-processbot bot deleted the antonva/parachainscache-btreeset branch February 27, 2023 17:22
@@ -2127,7 +2127,7 @@ impl<T: Config> Pallet<T> {
/// or removing parachains in bulk.
pub(crate) struct ParachainsCache<T: Config> {
// `None` here means the parachains list has not been accessed yet, nevermind modified.
parachains: Option<Vec<ParaId>>,
parachains: Option<BTreeSet<ParaId>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this 100% have the same SCALE encoding format as the Vec? If not, this will break things, badly.

}
}

impl<T: Config> Drop for ParachainsCache<T> {
fn drop(&mut self) {
if let Some(parachains) = self.parachains.take() {
Parachains::<T>::put(&parachains);
Parachains::<T>::put(parachains.into_iter().collect::<Vec<ParaId>>());
Copy link
Member

Choose a reason for hiding this comment

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

@rphmeier its being encode as Vec here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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.

5 participants