WeightMeter: more consistent naming (#14586)

* Rename WeightMeter functions

* Fixes

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fixup and doc + tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* One more test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fixup pallets

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use correct function 🤦

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Apply suggestions from code review

Co-authored-by: Juan <juangirini@gmail.com>

* Update primitives/weights/src/weight_meter.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update primitives/weights/src/weight_meter.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update primitives/weights/src/weight_meter.rs

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Juan <juangirini@gmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
This commit is contained in:
Oliver Tale-Yazdi
2023-07-18 16:38:44 +02:00
committed by GitHub
parent 1dc6610524
commit 892831e55c
8 changed files with 173 additions and 57 deletions
+4 -4
View File
@@ -175,7 +175,7 @@ pub mod pallet {
fn on_idle(_: BlockNumberFor<T>, remaining_weight: Weight) -> Weight {
let mut meter = WeightMeter::from_limit(remaining_weight);
if !meter.check_accrue(T::WeightInfo::empty_on_idle()) {
if meter.try_consume(T::WeightInfo::empty_on_idle()).is_err() {
return T::WeightInfo::empty_on_idle()
}
@@ -191,7 +191,7 @@ pub mod pallet {
Self::waste_at_most_proof_size(&mut meter);
Self::waste_at_most_ref_time(&mut meter);
meter.consumed
meter.consumed()
}
}
@@ -273,7 +273,7 @@ pub mod pallet {
pub(crate) fn waste_at_most_proof_size(meter: &mut WeightMeter) {
let Ok(n) = Self::calculate_proof_size_iters(&meter) else { return };
meter.defensive_saturating_accrue(T::WeightInfo::waste_proof_size_some(n));
meter.consume(T::WeightInfo::waste_proof_size_some(n));
(0..n).for_each(|i| {
TrashData::<T>::get(i);
@@ -302,7 +302,7 @@ pub mod pallet {
/// Tries to come as close to the limit as possible.
pub(crate) fn waste_at_most_ref_time(meter: &mut WeightMeter) {
let Ok(n) = Self::calculate_ref_time_iters(&meter) else { return };
meter.defensive_saturating_accrue(T::WeightInfo::waste_ref_time_iter(n));
meter.consume(T::WeightInfo::waste_ref_time_iter(n));
let clobber = Self::waste_ref_time_iter(vec![0u8; 64], n);
@@ -166,7 +166,7 @@ mod benchmarks {
}
assert_eq!(ServiceHead::<T>::get().unwrap(), 10u32.into());
assert_eq!(weight.consumed, T::WeightInfo::bump_service_head());
assert_eq!(weight.consumed(), T::WeightInfo::bump_service_head());
}
#[benchmark]
+28 -18
View File
@@ -737,7 +737,7 @@ impl<T: Config> Pallet<T> {
///
/// Returns the current head if it got be bumped and `None` otherwise.
fn bump_service_head(weight: &mut WeightMeter) -> Option<MessageOriginOf<T>> {
if !weight.check_accrue(T::WeightInfo::bump_service_head()) {
if weight.try_consume(T::WeightInfo::bump_service_head()).is_err() {
return None
}
@@ -865,7 +865,7 @@ impl<T: Config> Pallet<T> {
book_state.message_count,
book_state.size,
);
Ok(weight_counter.consumed.saturating_add(page_weight))
Ok(weight_counter.consumed().saturating_add(page_weight))
},
}
}
@@ -948,9 +948,13 @@ impl<T: Config> Pallet<T> {
overweight_limit: Weight,
) -> (bool, Option<MessageOriginOf<T>>) {
use PageExecutionStatus::*;
if !weight.check_accrue(
T::WeightInfo::service_queue_base().saturating_add(T::WeightInfo::ready_ring_unknit()),
) {
if weight
.try_consume(
T::WeightInfo::service_queue_base()
.saturating_add(T::WeightInfo::ready_ring_unknit()),
)
.is_err()
{
return (false, None)
}
@@ -1003,10 +1007,13 @@ impl<T: Config> Pallet<T> {
overweight_limit: Weight,
) -> (u32, PageExecutionStatus) {
use PageExecutionStatus::*;
if !weight.check_accrue(
T::WeightInfo::service_page_base_completion()
.max(T::WeightInfo::service_page_base_no_completion()),
) {
if weight
.try_consume(
T::WeightInfo::service_page_base_completion()
.max(T::WeightInfo::service_page_base_no_completion()),
)
.is_err()
{
return (0, Bailed)
}
@@ -1066,7 +1073,7 @@ impl<T: Config> Pallet<T> {
if page.is_complete() {
return ItemExecutionStatus::NoItem
}
if !weight.check_accrue(T::WeightInfo::service_page_item()) {
if weight.try_consume(T::WeightInfo::service_page_item()).is_err() {
return ItemExecutionStatus::Bailed
}
@@ -1163,7 +1170,7 @@ impl<T: Config> Pallet<T> {
) -> MessageExecutionStatus {
let hash = sp_io::hashing::blake2_256(message);
use ProcessMessageError::*;
let prev_consumed = meter.consumed;
let prev_consumed = meter.consumed();
let mut id = hash;
match T::MessageProcessor::process_message(message, origin.clone(), meter, &mut id) {
@@ -1193,7 +1200,7 @@ impl<T: Config> Pallet<T> {
},
Ok(success) => {
// Success
let weight_used = meter.consumed.saturating_sub(prev_consumed);
let weight_used = meter.consumed().saturating_sub(prev_consumed);
Self::deposit_event(Event::<T>::Processed { id, origin, weight_used, success });
MessageExecutionStatus::Processed
},
@@ -1254,7 +1261,7 @@ impl<T: Config> ServiceQueues for Pallet<T> {
let mut next = match Self::bump_service_head(&mut weight) {
Some(h) => h,
None => return weight.consumed,
None => return weight.consumed(),
};
// The last queue that did not make any progress.
// The loop aborts as soon as it arrives at this queue again without making any progress
@@ -1280,7 +1287,7 @@ impl<T: Config> ServiceQueues for Pallet<T> {
None => break,
}
}
weight.consumed
weight.consumed()
}
/// Execute a single overweight message.
@@ -1291,10 +1298,13 @@ impl<T: Config> ServiceQueues for Pallet<T> {
(message_origin, page, index): Self::OverweightMessageAddress,
) -> Result<Weight, ExecuteOverweightError> {
let mut weight = WeightMeter::from_limit(weight_limit);
if !weight.check_accrue(
T::WeightInfo::execute_overweight_page_removed()
.max(T::WeightInfo::execute_overweight_page_updated()),
) {
if weight
.try_consume(
T::WeightInfo::execute_overweight_page_removed()
.max(T::WeightInfo::execute_overweight_page_updated()),
)
.is_err()
{
return Err(ExecuteOverweightError::InsufficientWeight)
}
+2 -2
View File
@@ -188,7 +188,7 @@ impl ProcessMessage for RecordingMessageProcessor {
};
let required = Weight::from_parts(weight, weight);
if meter.check_accrue(required) {
if meter.try_consume(required).is_ok() {
let mut m = MessagesProcessed::get();
m.push((message.to_vec(), origin));
MessagesProcessed::set(m);
@@ -245,7 +245,7 @@ impl ProcessMessage for CountingMessageProcessor {
}
let required = Weight::from_parts(1, 1);
if meter.check_accrue(required) {
if meter.try_consume(required).is_ok() {
NumMessagesProcessed::set(NumMessagesProcessed::get() + 1);
Ok(true)
} else {
@@ -66,7 +66,7 @@ where
) -> Result<bool, ProcessMessageError> {
let required = Weight::from_parts(REQUIRED_WEIGHT, REQUIRED_WEIGHT);
if meter.check_accrue(required) {
if meter.try_consume(required).is_ok() {
Ok(true)
} else {
Err(ProcessMessageError::Overweight(required))
+11 -11
View File
@@ -381,7 +381,7 @@ fn service_queue_bails() {
let mut meter = WeightMeter::from_limit(1.into_weight());
assert_storage_noop!(MessageQueue::service_queue(0u32.into(), &mut meter, Weight::MAX));
assert!(meter.consumed.is_zero());
assert!(meter.consumed().is_zero());
});
// Not enough weight for `ready_ring_unknit`.
test_closure(|| {
@@ -389,7 +389,7 @@ fn service_queue_bails() {
let mut meter = WeightMeter::from_limit(1.into_weight());
assert_storage_noop!(MessageQueue::service_queue(0u32.into(), &mut meter, Weight::MAX));
assert!(meter.consumed.is_zero());
assert!(meter.consumed().is_zero());
});
// Not enough weight for `service_queue_base` and `ready_ring_unknit`.
test_closure(|| {
@@ -398,7 +398,7 @@ fn service_queue_bails() {
let mut meter = WeightMeter::from_limit(3.into_weight());
assert_storage_noop!(MessageQueue::service_queue(0.into(), &mut meter, Weight::MAX));
assert!(meter.consumed.is_zero());
assert!(meter.consumed().is_zero());
});
}
@@ -458,7 +458,7 @@ fn service_page_bails() {
&mut meter,
Weight::MAX
));
assert!(meter.consumed.is_zero());
assert!(meter.consumed().is_zero());
});
// Not enough weight for `service_page_base_no_completion`.
test_closure(|| {
@@ -475,7 +475,7 @@ fn service_page_bails() {
&mut meter,
Weight::MAX
));
assert!(meter.consumed.is_zero());
assert!(meter.consumed().is_zero());
});
}
@@ -586,7 +586,7 @@ fn bump_service_head_bails() {
let _guard = StorageNoopGuard::default();
let mut meter = WeightMeter::from_limit(1.into_weight());
assert!(MessageQueue::bump_service_head(&mut meter).is_none());
assert_eq!(meter.consumed, 0.into_weight());
assert_eq!(meter.consumed(), 0.into_weight());
});
}
@@ -597,16 +597,16 @@ fn bump_service_head_trivial_works() {
let mut meter = WeightMeter::max_limit();
assert_eq!(MessageQueue::bump_service_head(&mut meter), None, "Cannot bump");
assert_eq!(meter.consumed, 2.into_weight());
assert_eq!(meter.consumed(), 2.into_weight());
setup_bump_service_head::<Test>(0.into(), 1.into());
assert_eq!(MessageQueue::bump_service_head(&mut meter), Some(0.into()));
assert_eq!(ServiceHead::<Test>::get().unwrap(), 1.into(), "Bumped the head");
assert_eq!(meter.consumed, 4.into_weight());
assert_eq!(meter.consumed(), 4.into_weight());
assert_eq!(MessageQueue::bump_service_head(&mut meter), None, "Cannot bump");
assert_eq!(meter.consumed, 6.into_weight());
assert_eq!(meter.consumed(), 6.into_weight());
});
}
@@ -649,7 +649,7 @@ fn service_page_item_consumes_correct_weight() {
),
ItemExecutionStatus::Executed(true)
);
assert_eq!(weight.consumed, 5.into_weight());
assert_eq!(weight.consumed(), 5.into_weight());
});
}
@@ -673,7 +673,7 @@ fn service_page_item_skips_perm_overweight_message() {
),
ItemExecutionStatus::Executed(false)
);
assert_eq!(weight.consumed, 2.into_weight());
assert_eq!(weight.consumed(), 2.into_weight());
assert_last_event::<Test>(
Event::OverweightEnqueued {
id: blake2_256(b"TooMuch"),
+11 -10
View File
@@ -296,7 +296,7 @@ pub mod pallet {
fn on_initialize(now: BlockNumberFor<T>) -> Weight {
let mut weight_counter = WeightMeter::from_limit(T::MaximumWeight::get());
Self::service_agendas(&mut weight_counter, now, u32::max_value());
weight_counter.consumed
weight_counter.consumed()
}
}
@@ -959,7 +959,7 @@ use ServiceTaskError::*;
impl<T: Config> Pallet<T> {
/// Service up to `max` agendas queue starting from earliest incompletely executed agenda.
fn service_agendas(weight: &mut WeightMeter, now: BlockNumberFor<T>, max: u32) {
if !weight.check_accrue(T::WeightInfo::service_agendas_base()) {
if weight.try_consume(T::WeightInfo::service_agendas_base()).is_err() {
return
}
@@ -970,7 +970,7 @@ impl<T: Config> Pallet<T> {
let max_items = T::MaxScheduledPerBlock::get();
let mut count_down = max;
let service_agenda_base_weight = T::WeightInfo::service_agenda_base(max_items);
while count_down > 0 && when <= now && weight.can_accrue(service_agenda_base_weight) {
while count_down > 0 && when <= now && weight.can_consume(service_agenda_base_weight) {
if !Self::service_agenda(weight, &mut executed, now, when, u32::max_value()) {
incomplete_since = incomplete_since.min(when);
}
@@ -1001,8 +1001,9 @@ impl<T: Config> Pallet<T> {
})
.collect::<Vec<_>>();
ordered.sort_by_key(|k| k.1);
let within_limit =
weight.check_accrue(T::WeightInfo::service_agenda_base(ordered.len() as u32));
let within_limit = weight
.try_consume(T::WeightInfo::service_agenda_base(ordered.len() as u32))
.is_ok();
debug_assert!(within_limit, "weight limit should have been checked in advance");
// Items which we know can be executed and have postponed for execution in a later block.
@@ -1020,7 +1021,7 @@ impl<T: Config> Pallet<T> {
task.maybe_id.is_some(),
task.maybe_periodic.is_some(),
);
if !weight.can_accrue(base_weight) {
if !weight.can_consume(base_weight) {
postponed += 1;
break
}
@@ -1072,7 +1073,7 @@ impl<T: Config> Pallet<T> {
Err(_) => return Err((Unavailable, Some(task))),
};
weight.check_accrue(T::WeightInfo::service_task(
let _ = weight.try_consume(T::WeightInfo::service_task(
lookup_len.map(|x| x as usize),
task.maybe_id.is_some(),
task.maybe_periodic.is_some(),
@@ -1148,7 +1149,7 @@ impl<T: Config> Pallet<T> {
// We only allow a scheduled call if it cannot push the weight past the limit.
let max_weight = base_weight.saturating_add(call_weight);
if !weight.can_accrue(max_weight) {
if !weight.can_consume(max_weight) {
return Err(Overweight)
}
@@ -1159,8 +1160,8 @@ impl<T: Config> Pallet<T> {
(error_and_info.post_info.actual_weight, Err(error_and_info.error)),
};
let call_weight = maybe_actual_call_weight.unwrap_or(call_weight);
weight.check_accrue(base_weight);
weight.check_accrue(call_weight);
let _ = weight.try_consume(base_weight);
let _ = weight.try_consume(call_weight);
Ok(result)
}
}
+115 -10
View File
@@ -32,18 +32,21 @@ use sp_arithmetic::Perbill;
///
/// // The weight is limited to (10, 0).
/// let mut meter = WeightMeter::from_limit(Weight::from_parts(10, 0));
/// // There is enough weight remaining for an operation with (5, 0) weight.
/// assert!(meter.check_accrue(Weight::from_parts(5, 0)));
/// // There is not enough weight remaining for an operation with (6, 0) weight.
/// assert!(!meter.check_accrue(Weight::from_parts(6, 0)));
/// // There is enough weight remaining for an operation with (6, 0) weight.
/// assert!(meter.try_consume(Weight::from_parts(6, 0)).is_ok());
/// assert_eq!(meter.remaining(), Weight::from_parts(4, 0));
/// // There is not enough weight remaining for an operation with (5, 0) weight.
/// assert!(!meter.try_consume(Weight::from_parts(5, 0)).is_ok());
/// // The total limit is obviously unchanged:
/// assert_eq!(meter.limit(), Weight::from_parts(10, 0));
/// ```
#[derive(Debug, Clone)]
pub struct WeightMeter {
/// The already consumed weight.
pub consumed: Weight,
consumed: Weight,
/// The maximal consumable weight.
pub limit: Weight,
limit: Weight,
}
impl WeightMeter {
@@ -57,6 +60,16 @@ impl WeightMeter {
Self::from_limit(Weight::MAX)
}
/// The already consumed weight.
pub fn consumed(&self) -> Weight {
self.consumed
}
/// The limit can ever be accrued.
pub fn limit(&self) -> Weight {
self.limit
}
/// The remaining weight that can still be consumed.
pub fn remaining(&self) -> Weight {
self.limit.saturating_sub(self.consumed)
@@ -65,6 +78,28 @@ impl WeightMeter {
/// The ratio of consumed weight to the limit.
///
/// Calculates one ratio per component and returns the largest.
///
/// # Example
/// ```rust
/// use sp_weights::{Weight, WeightMeter};
/// use sp_arithmetic::Perbill;
///
/// let mut meter = WeightMeter::from_limit(Weight::from_parts(10, 20));
/// // Nothing consumed so far:
/// assert_eq!(meter.consumed_ratio(), Perbill::from_percent(0));
/// meter.consume(Weight::from_parts(5, 5));
/// // The ref-time is the larger ratio:
/// assert_eq!(meter.consumed_ratio(), Perbill::from_percent(50));
/// meter.consume(Weight::from_parts(1, 10));
/// // Now the larger ratio is proof-size:
/// assert_eq!(meter.consumed_ratio(), Perbill::from_percent(75));
/// // Eventually it reaches 100%:
/// meter.consume(Weight::from_parts(4, 0));
/// assert_eq!(meter.consumed_ratio(), Perbill::from_percent(100));
/// // Saturating the second component won't change anything anymore:
/// meter.consume(Weight::from_parts(0, 5));
/// assert_eq!(meter.consumed_ratio(), Perbill::from_percent(100));
/// ```
pub fn consumed_ratio(&self) -> Perbill {
let time = Perbill::from_rational(self.consumed.ref_time(), self.limit.ref_time());
let pov = Perbill::from_rational(self.consumed.proof_size(), self.limit.proof_size());
@@ -72,25 +107,46 @@ impl WeightMeter {
}
/// Consume some weight and defensively fail if it is over the limit. Saturate in any case.
#[deprecated(note = "Use `consume` instead. Will be removed after December 2023.")]
pub fn defensive_saturating_accrue(&mut self, w: Weight) {
self.consume(w);
}
/// Consume some weight and defensively fail if it is over the limit. Saturate in any case.
pub fn consume(&mut self, w: Weight) {
self.consumed.saturating_accrue(w);
debug_assert!(self.consumed.all_lte(self.limit), "Weight counter overflow");
}
/// Consume the given weight after checking that it can be consumed. Otherwise do nothing.
/// Consume the given weight after checking that it can be consumed and return `true`. Otherwise
/// do nothing and return `false`.
#[deprecated(note = "Use `try_consume` instead. Will be removed after December 2023.")]
pub fn check_accrue(&mut self, w: Weight) -> bool {
self.consumed.checked_add(&w).map_or(false, |test| {
self.try_consume(w).is_ok()
}
/// Consume the given weight after checking that it can be consumed.
///
/// Returns `Ok` if the weight can be consumed or otherwise an `Err`.
pub fn try_consume(&mut self, w: Weight) -> Result<(), ()> {
self.consumed.checked_add(&w).map_or(Err(()), |test| {
if test.any_gt(self.limit) {
false
Err(())
} else {
self.consumed = test;
true
Ok(())
}
})
}
/// Check if the given weight can be consumed.
#[deprecated(note = "Use `can_consume` instead. Will be removed after December 2023.")]
pub fn can_accrue(&self, w: Weight) -> bool {
self.can_consume(w)
}
/// Check if the given weight can be consumed.
pub fn can_consume(&self, w: Weight) -> bool {
self.consumed.checked_add(&w).map_or(false, |t| t.all_lte(self.limit))
}
}
@@ -98,6 +154,7 @@ impl WeightMeter {
#[cfg(test)]
mod tests {
use crate::*;
use sp_arithmetic::traits::Zero;
#[test]
fn weight_meter_remaining_works() {
@@ -179,4 +236,52 @@ mod tests {
assert!(meter.check_accrue(Weight::from_parts(0, 4)));
assert_eq!(meter.consumed_ratio(), Perbill::from_percent(100));
}
#[test]
fn try_consume_works() {
let mut meter = WeightMeter::from_limit(Weight::from_parts(10, 0));
assert!(meter.try_consume(Weight::from_parts(11, 0)).is_err());
assert!(meter.consumed().is_zero(), "No modification");
assert!(meter.try_consume(Weight::from_parts(9, 0)).is_ok());
assert!(meter.try_consume(Weight::from_parts(2, 0)).is_err());
assert!(meter.try_consume(Weight::from_parts(1, 0)).is_ok());
assert!(meter.remaining().is_zero());
assert_eq!(meter.consumed(), Weight::from_parts(10, 0));
}
#[test]
fn can_consume_works() {
let mut meter = WeightMeter::from_limit(Weight::from_parts(10, 0));
assert!(!meter.can_consume(Weight::from_parts(11, 0)));
assert!(meter.consumed().is_zero(), "No modification");
assert!(meter.can_consume(Weight::from_parts(9, 0)));
meter.consume(Weight::from_parts(9, 0));
assert!(!meter.can_consume(Weight::from_parts(2, 0)));
assert!(meter.can_consume(Weight::from_parts(1, 0)));
}
#[test]
#[cfg(debug_assertions)]
fn consume_works() {
let mut meter = WeightMeter::from_limit(Weight::from_parts(5, 10));
meter.consume(Weight::from_parts(4, 0));
assert_eq!(meter.remaining(), Weight::from_parts(1, 10));
meter.consume(Weight::from_parts(1, 0));
assert_eq!(meter.remaining(), Weight::from_parts(0, 10));
meter.consume(Weight::from_parts(0, 10));
assert_eq!(meter.consumed(), Weight::from_parts(5, 10));
}
#[test]
#[cfg(debug_assertions)]
#[should_panic(expected = "Weight counter overflow")]
fn consume_defensive_fail() {
let mut meter = WeightMeter::from_limit(Weight::from_parts(10, 0));
let _ = meter.consume(Weight::from_parts(11, 0));
}
}