Skip to content

refactor: Hoist idle-canister drop in RoundSchedule::start_iteration #10288

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.

Sign up for GitHub

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

Open
alin-at-dfinity wants to merge 2 commits into
base: master
Choose a base branch
from
+42 −18
Open

refactor: Hoist idle-canister drop in RoundSchedule::start_iteration #10288

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
686802a
refactor: Hoist idle-canister drop in `RoundSchedule::start_iteration`
alin-at-dfinity May 22, 2026
c2374e6
refactor: Drop redundant `SubnetSchedule::get_mut` in `start_iteration`
alin-at-dfinity May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 42 additions & 18 deletions rs/execution_environment/src/scheduler/round_schedule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ pub struct IterationSchedule {
}

impl IterationSchedule {
/// Partitions the executable canisters to the available cores for execution.
/// Splits the scheduled canisters off into per-core vectors, leaving any
/// non-scheduled canisters (the "inactive" set) in `canisters`. The
/// scheduled canisters are to be reinserted by the caller after execution.
#[allow(clippy::type_complexity)]
pub fn partition_canisters_to_cores(
&self,
Expand Down Expand Up @@ -277,6 +279,41 @@ impl RoundSchedule {
// Sum of all long execution canisters' compute allocations.
let mut long_executions_compute_allocation = ZERO;

if is_first_iteration {
// (Only) drop from the schedule idle canisters with 0-100 AP.
//
// Idle canisters with negative AP are kept in the schedule until they reach 0
// AP, to prevent them from jumping from the back of the schedule to the middle
// just by being idle for a round. Similarly, idle canisters with positive AP
// burn it down as if they had executed full rounds until they (almost) reach 0.
let idle_canisters_to_drop = subnet_schedule
.iter()
.filter_map(|(canister_id, canister_priority)| {
if canister_priority.accumulated_priority < ZERO
|| canister_priority.accumulated_priority > ONE_HUNDRED_PERCENT
{
// Not in the 0-100 AP range: keep.
return None;
}
let Some(canister) = canister_states.get(canister_id) else {
// Canister was deleted: drop.
return Some(*canister_id);
};
if !canister.must_be_in_schedule()
&& canister.next_execution() == NextExecution::None
{
// Canister is idle: drop.
Some(*canister_id)
} else {
None
}
})
.collect::<Vec<_>>();
for canister_id in idle_canisters_to_drop {
subnet_schedule.remove(&canister_id);
}
}

// Collect all active canisters and their next executions.
//
// Unfortunately, not all active canisters are in the subnet schedule, so we
Expand Down Expand Up @@ -307,35 +344,22 @@ impl RoundSchedule {
if self.long_execution_canisters.contains(canister_id) {
return None;
}
CanisterRoundState::new(canister, subnet_schedule.get_mut(*canister_id))
CanisterRoundState::new(canister, subnet_schedule.get(canister_id))
Copy link
Copy Markdown
Contributor Author

@alin-at-dfinity alin-at-dfinity May 22, 2026

Choose a reason for hiding this comment

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

FYI: This replacement (and the similar one right below) result in a change in behavior: canisters no longer get a SubnetSchedule entry as soon as they are scheduled by start_iteration().

But this was accidentally left in (during development I had a version of CanisterRoundState::new() that actually mutated the provided CanisterPriority (don't remember why). And finish_round() will call get_mut() to create an entry for all scheduled canisters. And no one looks at the SubnetSchedule in-between.

}

NextExecution::ContinueLong => {
if is_first_iteration {
self.long_execution_canisters.insert(*canister_id);
}
let priority = subnet_schedule.get_mut(*canister_id);
let rs = CanisterRoundState::new(canister, priority);
let rs =
CanisterRoundState::new(canister, subnet_schedule.get(canister_id));
long_executions_count += 1;
long_executions_compute_allocation += rs.compute_allocation;
rs
}

NextExecution::None => {
// (Only) drop from the schedule idle canisters with 0-100 AP.
//
// Idle canisters with negative AP are kept in the schedule until they reach 0
// AP, to prevent them from jumping from the back of the schedule to the middle
// just by being idle for a round. Similarly, idle canisters with positive AP
// burn it down as if they had executed full rounds until they (almost) reach 0.
if is_first_iteration && !canister.must_be_in_schedule() {
let canister_priority = subnet_schedule.get(canister_id);
if canister_priority.accumulated_priority >= ZERO
&& canister_priority.accumulated_priority <= ONE_HUNDRED_PERCENT
{
subnet_schedule.remove(canister_id);
}
}
// Already dropped idle canisters with 0-100 AP above. Nothing to do here.
return None;
}

Expand Down
Loading