Skip to content

Commit b5efbab

Browse files
perf: Look up sandbox scheduler priorities per canister (#10279)
`evict_sandbox_processes` only needs the accumulated priority of the canisters that have an active sandbox backend, but currently asks `ReplicatedState` for the priorities of *all* canisters on the subnet, materializing a fresh `BTreeMap<CanisterId, AccumulatedPriority>` on every eviction pass. Replace the bulk lookup with per-id `canister_state(id)` / `canister_priority(id)` calls inside the existing `backends.iter()` loop. Behaviour is unchanged: a missing canister still falls through to `AccumulatedPriority::MIN`. With the sandbox controller migrated, `canister_accumulated_priorities()` has no remaining callers, so drop it (and its now-unused `AccumulatedPriority` import) from `ReplicatedState`. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 63b841f commit b5efbab

2 files changed

Lines changed: 8 additions & 26 deletions

File tree

rs/canister_sandbox/src/replica_controller/sandboxed_execution_controller.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2068,11 +2068,7 @@ fn evict_sandbox_processes(
20682068
Backend::Empty => false,
20692069
});
20702070

2071-
let scheduler_priorities = state_reader
2072-
.get_latest_state()
2073-
.get_ref()
2074-
.canister_accumulated_priorities();
2075-
2071+
let state = state_reader.get_latest_state();
20762072
let min_scheduler_priority = AccumulatedPriority::new(i64::MIN);
20772073

20782074
let candidates: Vec<_> = backends
@@ -2082,10 +2078,12 @@ fn evict_sandbox_processes(
20822078
id: *id,
20832079
last_used: stats.last_used,
20842080
rss: stats.rss,
2085-
scheduler_priority: *scheduler_priorities
2086-
.get(id)
2087-
// This should happen only if the canister is deleted.
2088-
.unwrap_or(&min_scheduler_priority),
2081+
scheduler_priority: if state.get_ref().canister_state(id).is_some() {
2082+
state.get_ref().canister_priority(id).accumulated_priority
2083+
} else {
2084+
// Canister was deleted.
2085+
min_scheduler_priority
2086+
},
20892087
}),
20902088
Backend::Evicted { .. } | Backend::Empty => None,
20912089
})

rs/replicated_state/src/replicated_state.rs

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use ic_registry_resource_limits::ResourceLimits;
2525
use ic_registry_routing_table::RoutingTable;
2626
use ic_registry_subnet_type::SubnetType;
2727
use ic_types::{
28-
AccumulatedPriority, CanisterId, NumBytes, SubnetId, Time,
28+
CanisterId, NumBytes, SubnetId, Time,
2929
batch::{ConsensusResponse, RawQueryStats},
3030
consensus::idkg::IDkgMasterPublicKeyId,
3131
ingress::IngressStatus,
@@ -686,22 +686,6 @@ impl ReplicatedState {
686686
)
687687
}
688688

689-
/// Time complexity: `O(n)` in the number of active canisters.
690-
pub fn canister_accumulated_priorities(&self) -> BTreeMap<CanisterId, AccumulatedPriority> {
691-
self.canister_states
692-
.keys()
693-
.map(|canister_id| {
694-
(
695-
*canister_id,
696-
self.metadata
697-
.subnet_schedule
698-
.get(canister_id)
699-
.accumulated_priority,
700-
)
701-
})
702-
.collect()
703-
}
704-
705689
/// Prunes the canister priorities of deleted canisters; and those that have
706690
/// all-zero accumulated priority, priority credit, heap delta and install code
707691
/// debits, and do not have a long-running execution.

0 commit comments

Comments
 (0)