-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix algorithmic complexity of on-demand scheduler with regards to number of cores. #3190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 32 commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
fd41678
max -> min
9ee7aa1
Reduce default queue size on-demand
0b3c03d
Introduce max max queue size.
edad3a4
Use max max queue size.
ee74614
Implementation complete
16afcfa
Revert min/max fix.
c750146
Fixes + EncodeableBinaryHeap.
b0edbcf
Remove EncodeableBinaryHeap
cac2752
Patch scale-info for now.
a9b13f7
Revert default value.
f9ea4e4
Fixes + tests.
457b225
Bring back copyright.
ce7c8e8
Fix benchmark.
1bab42e
binary heap got merged
c21540d
Calculate on demand traffic on idle blocks
antonva 999b7bb
Add migration for on demand provider
antonva 4df8590
Merge branch 'master' into rk-on-demand-perf-proper-fix
antonva ecbae81
Readd missing export
antonva 079c78f
Add storage version to on demand pallet
antonva ee91d8b
Merge branch 'master' into rk-on-demand-perf-proper-fix
antonva 5308f3a
Address comments, add new scale-info
antonva 2253a14
Merge branch 'master' into rk-on-demand-perf-proper-fix
antonva 1ae8b7b
Bump scale-info version again
antonva 3e60c09
Add test to migration to preserve queue ordering
antonva 2dd2512
Merge branch 'master' of https://github.com/paritytech/polkadot-sdk i…
600fd5e
".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…
ee29cc2
Fix post_upgrade in migration
antonva b344bc6
Add prdoc
antonva 76e4e76
Remove unused on-demand max size import
antonva ab51045
Remove unused mut from test
antonva 9a705cb
Remove benchmark todo
antonva f2ad19f
Simplify PartialOrd for EnqueuedOrder
antonva 70f468e
".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…
d8f1543
Address nits
antonva be3a3c1
Type sums in post migration
antonva File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
181 changes: 181 additions & 0 deletions
181
polkadot/runtime/parachains/src/assigner_on_demand/migration.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| // Copyright (C) Parity Technologies (UK) Ltd. | ||
| // This file is part of Polkadot. | ||
|
|
||
| // Polkadot is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
|
|
||
| // Polkadot is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU General Public License for more details. | ||
|
|
||
| // You should have received a copy of the GNU General Public License | ||
| // along with Polkadot. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| //! A module that is responsible for migration of storage. | ||
| use super::*; | ||
| use frame_support::{ | ||
| migrations::VersionedMigration, pallet_prelude::ValueQuery, storage_alias, | ||
| traits::OnRuntimeUpgrade, weights::Weight, | ||
| }; | ||
|
|
||
| mod v0 { | ||
| use super::*; | ||
| use sp_std::collections::vec_deque::VecDeque; | ||
|
|
||
| #[derive(Encode, Decode, TypeInfo, Debug, PartialEq, Clone)] | ||
| pub(super) struct EnqueuedOrder { | ||
| pub para_id: ParaId, | ||
| } | ||
|
|
||
| /// Keeps track of the multiplier used to calculate the current spot price for the on demand | ||
| /// assigner. | ||
| /// NOTE: Ignoring the `OnEmpty` field for the migration. | ||
| #[storage_alias] | ||
| pub(super) type SpotTraffic<T: Config> = StorageValue<Pallet<T>, FixedU128, ValueQuery>; | ||
|
|
||
| /// The order storage entry. Uses a VecDeque to be able to push to the front of the | ||
| /// queue from the scheduler on session boundaries. | ||
| /// NOTE: Ignoring the `OnEmpty` field for the migration. | ||
| #[storage_alias] | ||
| pub(super) type OnDemandQueue<T: Config> = | ||
| StorageValue<Pallet<T>, VecDeque<EnqueuedOrder>, ValueQuery>; | ||
| } | ||
|
|
||
| mod v1 { | ||
| use super::*; | ||
|
|
||
| use crate::assigner_on_demand::LOG_TARGET; | ||
|
|
||
| /// Migration to V1 | ||
| pub struct UncheckedMigrateToV1<T>(sp_std::marker::PhantomData<T>); | ||
| impl<T: Config> OnRuntimeUpgrade for UncheckedMigrateToV1<T> { | ||
| fn on_runtime_upgrade() -> Weight { | ||
| let mut weight: Weight = Weight::zero(); | ||
|
|
||
| // Migrate the current traffic value | ||
| let config = <configuration::Pallet<T>>::config(); | ||
| QueueStatus::<T>::mutate(|mut queue_status| { | ||
| Pallet::<T>::update_spot_traffic(&config, &mut queue_status); | ||
|
|
||
| let v0_queue = v0::OnDemandQueue::<T>::take(); | ||
| // Process the v0 queue into v1. | ||
| v0_queue.into_iter().for_each(|enqueued_order| { | ||
| // Readding the old orders will use the new systems. | ||
| Pallet::<T>::add_on_demand_order( | ||
| queue_status, | ||
| enqueued_order.para_id, | ||
| QueuePushDirection::Back, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| // Remove the old storage. | ||
| v0::OnDemandQueue::<T>::kill(); // 1 write | ||
| v0::SpotTraffic::<T>::kill(); // 1 write | ||
|
|
||
| // Config read | ||
| weight.saturating_accrue(T::DbWeight::get().reads(1)); | ||
| // QueueStatus read write (update_spot_traffic) | ||
| weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); | ||
| // Kill x 2 | ||
| weight.saturating_accrue(T::DbWeight::get().writes(2)); | ||
|
|
||
| log::info!(target: LOG_TARGET, "Migrated on demand assigner storage to v1"); | ||
| weight | ||
| } | ||
|
|
||
| #[cfg(feature = "try-runtime")] | ||
| fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> { | ||
| let n: u32 = v0::OnDemandQueue::<T>::get().len() as u32; | ||
|
|
||
| log::info!( | ||
| target: LOG_TARGET, | ||
| "Number of orders waiting in the queue before: {n}", | ||
| ); | ||
|
|
||
| Ok(n.encode()) | ||
| } | ||
|
|
||
| #[cfg(feature = "try-runtime")] | ||
| fn post_upgrade(state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> { | ||
| log::info!(target: LOG_TARGET, "Running post_upgrade()"); | ||
|
|
||
| ensure!( | ||
| v0::OnDemandQueue::<T>::get().is_empty(), | ||
| "OnDemandQueue should be empty after the migration" | ||
| ); | ||
|
|
||
| let expected_len = u32::decode(&mut &state[..]).unwrap(); | ||
| let queue_status_size = QueueStatus::<T>::get().size(); | ||
| ensure!( | ||
| expected_len == queue_status_size, | ||
| "Number of orders should be the same before and after migration" | ||
| ); | ||
|
|
||
| let n_affinity_entries = | ||
| AffinityEntries::<T>::iter().map(|(_index, heap)| heap.len() as u32).count(); | ||
|
antonva marked this conversation as resolved.
Outdated
|
||
| let n_para_id_affinity = ParaIdAffinity::<T>::iter() | ||
| .map(|(_para_id, affinity)| affinity.count as u32) | ||
| .count(); | ||
|
antonva marked this conversation as resolved.
Outdated
|
||
| ensure!( | ||
| n_para_id_affinity == n_affinity_entries, | ||
|
antonva marked this conversation as resolved.
|
||
| "Number of affinity entries should be the same as the counts in ParaIdAffinity" | ||
| ); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Migrate `V0` to `V1` of the storage format. | ||
| pub type MigrateV0ToV1<T> = VersionedMigration< | ||
| 0, | ||
| 1, | ||
| v1::UncheckedMigrateToV1<T>, | ||
| Pallet<T>, | ||
| <T as frame_system::Config>::DbWeight, | ||
| >; | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
|
eskimor marked this conversation as resolved.
|
||
| use super::{v0, v1, OnRuntimeUpgrade, Weight}; | ||
| use crate::mock::{new_test_ext, MockGenesisConfig, OnDemandAssigner, Test}; | ||
| use primitives::Id as ParaId; | ||
|
|
||
| #[test] | ||
| fn migration_to_v1_preserves_queue_ordering() { | ||
| new_test_ext(MockGenesisConfig::default()).execute_with(|| { | ||
| // Place orders for paraids 1..5 | ||
| for i in 1..=5 { | ||
| v0::OnDemandQueue::<Test>::mutate(|queue| { | ||
| queue.push_back(v0::EnqueuedOrder { para_id: ParaId::new(i) }) | ||
| }); | ||
| } | ||
|
|
||
| // Queue has 5 orders | ||
| let old_queue = v0::OnDemandQueue::<Test>::get(); | ||
| assert_eq!(old_queue.len(), 5); | ||
| // New queue has 0 orders | ||
| assert_eq!(OnDemandAssigner::get_queue_status().size(), 0); | ||
|
|
||
| // For tests, db weight is zero. | ||
| assert_eq!( | ||
| <v1::UncheckedMigrateToV1<Test> as OnRuntimeUpgrade>::on_runtime_upgrade(), | ||
| Weight::zero() | ||
| ); | ||
|
|
||
| // New queue has 5 orders | ||
| assert_eq!(OnDemandAssigner::get_queue_status().size(), 5); | ||
|
|
||
| // Compare each entry from the old queue with the entry in the new queue. | ||
| old_queue.iter().zip(OnDemandAssigner::get_free_entries().iter()).for_each( | ||
| |(old_enq, new_enq)| { | ||
| assert_eq!(old_enq.para_id, new_enq.para_id); | ||
| }, | ||
| ); | ||
| }); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.