Skip to content

Commit

Permalink
WeightMeter: more consistent naming (paritytech#14586)
Browse files Browse the repository at this point in the history
* Rename WeightMeter functions

* Fixes

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Fixup and doc + tests

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* One more test

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Fixup pallets

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Use correct function 🤦

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Apply suggestions from code review

Co-authored-by: Juan <[email protected]>

* Update primitives/weights/src/weight_meter.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Update primitives/weights/src/weight_meter.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Update primitives/weights/src/weight_meter.rs

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Juan <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
  • Loading branch information
3 people authored Jul 18, 2023
1 parent b1c18db commit cfe2432
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 57 deletions.
8 changes: 4 additions & 4 deletions frame/glutton/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand All @@ -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()
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion frame/message-queue/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
46 changes: 28 additions & 18 deletions frame/message-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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))
},
}
}
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
Expand All @@ -1280,7 +1287,7 @@ impl<T: Config> ServiceQueues for Pallet<T> {
None => break,
}
}
weight.consumed
weight.consumed()
}

/// Execute a single overweight message.
Expand All @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions frame/message-queue/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion frame/message-queue/src/mock_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
22 changes: 11 additions & 11 deletions frame/message-queue/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,15 +381,15 @@ 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(|| {
set_weight("ready_ring_unknit", 2.into_weight());
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(|| {
Expand All @@ -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());
});
}

Expand Down Expand Up @@ -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(|| {
Expand All @@ -475,7 +475,7 @@ fn service_page_bails() {
&mut meter,
Weight::MAX
));
assert!(meter.consumed.is_zero());
assert!(meter.consumed().is_zero());
});
}

Expand Down Expand Up @@ -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());
});
}

Expand All @@ -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());
});
}

Expand Down Expand Up @@ -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());
});
}

Expand All @@ -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"),
Expand Down
21 changes: 11 additions & 10 deletions frame/scheduler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -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.
Expand All @@ -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
}
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}
}
Expand Down
Loading

0 comments on commit cfe2432

Please sign in to comment.