From b28d202d8d9fb31c6e1e1ec92afc2a9cddbda2f5 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Fri, 18 Sep 2020 16:15:40 +0200 Subject: [PATCH] WeightInfo for Scheduler (#7138) * initial scheduler stuff * integrate weightinfo * Update pallet_scheduler.rs --- bin/node/runtime/src/lib.rs | 4 +- bin/node/runtime/src/weights/mod.rs | 1 + .../runtime/src/weights/pallet_scheduler.rs | 51 +++++++++++++++++ frame/democracy/src/tests.rs | 1 + frame/scheduler/src/benchmarking.rs | 13 +++-- frame/scheduler/src/default_weights.rs | 50 +++++++++++++++++ frame/scheduler/src/lib.rs | 56 ++++++++++++------- 7 files changed, 148 insertions(+), 28 deletions(-) create mode 100644 bin/node/runtime/src/weights/pallet_scheduler.rs create mode 100644 frame/scheduler/src/default_weights.rs diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 03fe279366d50..0de78464bddec 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -273,6 +273,7 @@ impl pallet_proxy::Trait for Runtime { parameter_types! { pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * MaximumBlockWeight::get(); + pub const MaxScheduledPerBlock: u32 = 50; } impl pallet_scheduler::Trait for Runtime { @@ -282,7 +283,8 @@ impl pallet_scheduler::Trait for Runtime { type Call = Call; type MaximumWeight = MaximumSchedulerWeight; type ScheduleOrigin = EnsureRoot; - type WeightInfo = (); + type MaxScheduledPerBlock = MaxScheduledPerBlock; + type WeightInfo = weights::pallet_scheduler::WeightInfo; } parameter_types! { diff --git a/bin/node/runtime/src/weights/mod.rs b/bin/node/runtime/src/weights/mod.rs index 199e66888e65d..668b9462a7d6a 100644 --- a/bin/node/runtime/src/weights/mod.rs +++ b/bin/node/runtime/src/weights/mod.rs @@ -24,6 +24,7 @@ pub mod pallet_identity; pub mod pallet_indices; pub mod pallet_im_online; pub mod pallet_proxy; +pub mod pallet_scheduler; pub mod pallet_staking; pub mod pallet_timestamp; pub mod pallet_utility; diff --git a/bin/node/runtime/src/weights/pallet_scheduler.rs b/bin/node/runtime/src/weights/pallet_scheduler.rs new file mode 100644 index 0000000000000..110a0545ed839 --- /dev/null +++ b/bin/node/runtime/src/weights/pallet_scheduler.rs @@ -0,0 +1,51 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc6 + +#![allow(unused_parens)] +#![allow(unused_imports)] + +use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight}; + +pub struct WeightInfo; +impl pallet_scheduler::WeightInfo for WeightInfo { + fn schedule(s: u32, ) -> Weight { + (37_835_000 as Weight) + .saturating_add((81_000 as Weight).saturating_mul(s as Weight)) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(1 as Weight)) + } + fn cancel(s: u32, ) -> Weight { + (34_707_000 as Weight) + .saturating_add((3_125_000 as Weight).saturating_mul(s as Weight)) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(2 as Weight)) + } + fn schedule_named(s: u32, ) -> Weight { + (48_065_000 as Weight) + .saturating_add((110_000 as Weight).saturating_mul(s as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().writes(2 as Weight)) + } + fn cancel_named(s: u32, ) -> Weight { + (38_776_000 as Weight) + .saturating_add((3_138_000 as Weight).saturating_mul(s as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().writes(2 as Weight)) + } +} diff --git a/frame/democracy/src/tests.rs b/frame/democracy/src/tests.rs index aed6739c77ebb..4e569c98aee7f 100644 --- a/frame/democracy/src/tests.rs +++ b/frame/democracy/src/tests.rs @@ -128,6 +128,7 @@ impl pallet_scheduler::Trait for Test { type Call = Call; type MaximumWeight = MaximumSchedulerWeight; type ScheduleOrigin = EnsureRoot; + type MaxScheduledPerBlock = (); type WeightInfo = (); } parameter_types! { diff --git a/frame/scheduler/src/benchmarking.rs b/frame/scheduler/src/benchmarking.rs index 847460fe85a50..17b3a298f49ea 100644 --- a/frame/scheduler/src/benchmarking.rs +++ b/frame/scheduler/src/benchmarking.rs @@ -28,7 +28,6 @@ use frame_benchmarking::benchmarks; use crate::Module as Scheduler; use frame_system::Module as System; -const MAX_SCHEDULED: u32 = 50; const BLOCK_NUMBER: u32 = 2; // Add `n` named items to the schedule @@ -56,7 +55,7 @@ benchmarks! { _ { } schedule { - let s in 0 .. MAX_SCHEDULED; + let s in 0 .. T::MaxScheduledPerBlock::get(); let when = BLOCK_NUMBER.into(); let periodic = Some((T::BlockNumber::one(), 100)); let priority = 0; @@ -73,7 +72,7 @@ benchmarks! { } cancel { - let s in 1 .. MAX_SCHEDULED; + let s in 1 .. T::MaxScheduledPerBlock::get(); let when = BLOCK_NUMBER.into(); fill_schedule::(when, s)?; @@ -92,7 +91,7 @@ benchmarks! { } schedule_named { - let s in 0 .. MAX_SCHEDULED; + let s in 0 .. T::MaxScheduledPerBlock::get(); let id = s.encode(); let when = BLOCK_NUMBER.into(); let periodic = Some((T::BlockNumber::one(), 100)); @@ -110,7 +109,7 @@ benchmarks! { } cancel_named { - let s in 1 .. MAX_SCHEDULED; + let s in 1 .. T::MaxScheduledPerBlock::get(); let when = BLOCK_NUMBER.into(); fill_schedule::(when, s)?; @@ -127,8 +126,10 @@ benchmarks! { ); } + // TODO: Make this more complex and flexible so it can be used in automation. + #[extra] on_initialize { - let s in 0 .. MAX_SCHEDULED; + let s in 0 .. T::MaxScheduledPerBlock::get(); let when = BLOCK_NUMBER.into(); fill_schedule::(when, s)?; }: { Scheduler::::on_initialize(BLOCK_NUMBER.into()); } diff --git a/frame/scheduler/src/default_weights.rs b/frame/scheduler/src/default_weights.rs new file mode 100644 index 0000000000000..920de1d37a07c --- /dev/null +++ b/frame/scheduler/src/default_weights.rs @@ -0,0 +1,50 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc6 + +#![allow(unused_parens)] +#![allow(unused_imports)] + +use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight}; + +impl crate::WeightInfo for () { + fn schedule(s: u32, ) -> Weight { + (37_835_000 as Weight) + .saturating_add((81_000 as Weight).saturating_mul(s as Weight)) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(1 as Weight)) + } + fn cancel(s: u32, ) -> Weight { + (34_707_000 as Weight) + .saturating_add((3_125_000 as Weight).saturating_mul(s as Weight)) + .saturating_add(DbWeight::get().reads(1 as Weight)) + .saturating_add(DbWeight::get().writes(2 as Weight)) + } + fn schedule_named(s: u32, ) -> Weight { + (48_065_000 as Weight) + .saturating_add((110_000 as Weight).saturating_mul(s as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().writes(2 as Weight)) + } + fn cancel_named(s: u32, ) -> Weight { + (38_776_000 as Weight) + .saturating_add((3_138_000 as Weight).saturating_mul(s as Weight)) + .saturating_add(DbWeight::get().reads(2 as Weight)) + .saturating_add(DbWeight::get().writes(2 as Weight)) + } +} diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index edd112bd89299..99b9ea4ea5741 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -52,6 +52,7 @@ #![cfg_attr(not(feature = "std"), no_std)] mod benchmarking; +mod default_weights; use sp_std::{prelude::*, marker::PhantomData, borrow::Borrow}; use codec::{Encode, Decode, Codec}; @@ -69,15 +70,6 @@ pub trait WeightInfo { fn cancel(s: u32, ) -> Weight; fn schedule_named(s: u32, ) -> Weight; fn cancel_named(s: u32, ) -> Weight; - fn on_initialize(s: u32, ) -> Weight; -} - -impl WeightInfo for () { - fn schedule(_s: u32, ) -> Weight { 1_000_000_000 } - fn cancel(_s: u32, ) -> Weight { 1_000_000_000 } - fn schedule_named(_s: u32, ) -> Weight { 1_000_000_000 } - fn cancel_named(_s: u32, ) -> Weight { 1_000_000_000 } - fn on_initialize(_s: u32, ) -> Weight { 1_000_000_000 } } /// Our pallet's configuration trait. All our types and constants go in here. If the @@ -106,6 +98,10 @@ pub trait Trait: system::Trait { /// Required origin to schedule or cancel calls. type ScheduleOrigin: EnsureOrigin<::Origin>; + /// The maximum number of scheduled calls in the queue for a single block. + /// Not strictly enforced, but used for weight estimation. + type MaxScheduledPerBlock: Get; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -213,7 +209,7 @@ decl_module! { /// - Write: Agenda /// - Will use base weight of 25 which should be good for up to 30 scheduled calls /// # - #[weight = 25_000_000 + T::DbWeight::get().reads_writes(1, 1)] + #[weight = T::WeightInfo::schedule(T::MaxScheduledPerBlock::get())] fn schedule(origin, when: T::BlockNumber, maybe_periodic: Option>, @@ -235,7 +231,7 @@ decl_module! { /// - Write: Agenda, Lookup /// - Will use base weight of 100 which should be good for up to 30 scheduled calls /// # - #[weight = 100_000_000 + T::DbWeight::get().reads_writes(1, 2)] + #[weight = T::WeightInfo::cancel(T::MaxScheduledPerBlock::get())] fn cancel(origin, when: T::BlockNumber, index: u32) { T::ScheduleOrigin::ensure_origin(origin.clone())?; let origin = ::Origin::from(origin); @@ -252,7 +248,7 @@ decl_module! { /// - Write: Agenda, Lookup /// - Will use base weight of 35 which should be good for more than 30 scheduled calls /// # - #[weight = 35_000_000 + T::DbWeight::get().reads_writes(2, 2)] + #[weight = T::WeightInfo::schedule_named(T::MaxScheduledPerBlock::get())] fn schedule_named(origin, id: Vec, when: T::BlockNumber, @@ -277,7 +273,7 @@ decl_module! { /// - Write: Agenda, Lookup /// - Will use base weight of 100 which should be good for up to 30 scheduled calls /// # - #[weight = 100_000_000 + T::DbWeight::get().reads_writes(2, 2)] + #[weight = T::WeightInfo::cancel_named(T::MaxScheduledPerBlock::get())] fn cancel_named(origin, id: Vec) { T::ScheduleOrigin::ensure_origin(origin.clone())?; let origin = ::Origin::from(origin); @@ -289,7 +285,7 @@ decl_module! { /// # /// Same as [`schedule`]. /// # - #[weight = 25_000_000 + T::DbWeight::get().reads_writes(1, 1)] + #[weight = T::WeightInfo::schedule(T::MaxScheduledPerBlock::get())] fn schedule_after(origin, after: T::BlockNumber, maybe_periodic: Option>, @@ -308,7 +304,7 @@ decl_module! { /// # /// Same as [`schedule_named`]. /// # - #[weight = 35_000_000 + T::DbWeight::get().reads_writes(2, 2)] + #[weight = T::WeightInfo::schedule_named(T::MaxScheduledPerBlock::get())] fn schedule_named_after(origin, id: Vec, after: T::BlockNumber, @@ -340,16 +336,20 @@ decl_module! { .enumerate() .filter_map(|(index, s)| s.map(|inner| (index as u32, inner))) .collect::>(); + if queued.len() as u32 > T::MaxScheduledPerBlock::get() { + frame_support::debug::warn!( + "Warning: This block has more items queued in Scheduler than \ + expected from the runtime configuration. An update might be needed." + ); + } queued.sort_by_key(|(_, s)| s.priority); - let base_weight: Weight = T::DbWeight::get().reads_writes(1, 2) // Agenda + Agenda(next) - .saturating_add(10_000_000); // Base Weight + let base_weight: Weight = T::DbWeight::get().reads_writes(1, 2); // Agenda + Agenda(next) let mut total_weight: Weight = 0; queued.into_iter() .enumerate() .scan(base_weight, |cumulative_weight, (order, (index, s))| { *cumulative_weight = cumulative_weight - .saturating_add(s.call.get_dispatch_info().weight) - .saturating_add(25_000_000); // Base multiplier + .saturating_add(s.call.get_dispatch_info().weight); if s.maybe_id.is_some() { // Remove/Modify Lookup @@ -466,6 +466,12 @@ impl Module { }); Agenda::::append(when, s); let index = Agenda::::decode_len(when).unwrap_or(1) as u32 - 1; + if index > T::MaxScheduledPerBlock::get() { + frame_support::debug::warn!( + "Warning: There are more items queued in the Scheduler than \ + expected from the runtime configuration. An update might be needed." + ); + } Self::deposit_event(RawEvent::Scheduled(when, index)); Ok((when, index)) @@ -535,6 +541,12 @@ impl Module { }; Agenda::::append(when, Some(s)); let index = Agenda::::decode_len(when).unwrap_or(1) as u32 - 1; + if index > T::MaxScheduledPerBlock::get() { + frame_support::debug::warn!( + "Warning: There are more items queued in the Scheduler than \ + expected from the runtime configuration. An update might be needed." + ); + } let address = (when, index); Lookup::::insert(&id, &address); Self::deposit_event(RawEvent::Scheduled(when, index)); @@ -734,6 +746,7 @@ mod tests { } parameter_types! { pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * MaximumBlockWeight::get(); + pub const MaxScheduledPerBlock: u32 = 10; } ord_parameter_types! { pub const One: u64 = 1; @@ -746,6 +759,7 @@ mod tests { type Call = Call; type MaximumWeight = MaximumSchedulerWeight; type ScheduleOrigin = EnsureOneOf, EnsureSignedBy>; + type MaxScheduledPerBlock = MaxScheduledPerBlock; type WeightInfo = (); } type System = system::Module; @@ -982,8 +996,8 @@ mod tests { #[test] fn on_initialize_weight_is_correct() { new_test_ext().execute_with(|| { - let base_weight: Weight = ::DbWeight::get().reads_writes(1, 2) + 10_000_000; - let base_multiplier = 25_000_000; + let base_weight: Weight = ::DbWeight::get().reads_writes(1, 2); + let base_multiplier = 0; let named_multiplier = ::DbWeight::get().writes(1); let periodic_multiplier = ::DbWeight::get().reads_writes(1, 1);