From a3a7f275580a78b5f0839e55e04e9008df4c8d79 Mon Sep 17 00:00:00 2001 From: Alin Sinpalean Date: Fri, 22 May 2026 06:57:12 +0000 Subject: [PATCH] perf: Look up sandbox scheduler priorities per canister `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` 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 --- .../sandboxed_execution_controller.rs | 16 +++++++--------- rs/replicated_state/src/replicated_state.rs | 18 +----------------- 2 files changed, 8 insertions(+), 26 deletions(-) diff --git a/rs/canister_sandbox/src/replica_controller/sandboxed_execution_controller.rs b/rs/canister_sandbox/src/replica_controller/sandboxed_execution_controller.rs index f3f9dc47a5d1..3df0a7c9114a 100644 --- a/rs/canister_sandbox/src/replica_controller/sandboxed_execution_controller.rs +++ b/rs/canister_sandbox/src/replica_controller/sandboxed_execution_controller.rs @@ -2071,11 +2071,7 @@ fn evict_sandbox_processes( Backend::Empty => false, }); - let scheduler_priorities = state_reader - .get_latest_state() - .get_ref() - .canister_accumulated_priorities(); - + let state = state_reader.get_latest_state(); let min_scheduler_priority = AccumulatedPriority::new(i64::MIN); let candidates: Vec<_> = backends @@ -2085,10 +2081,12 @@ fn evict_sandbox_processes( id: *id, last_used: stats.last_used, rss: stats.rss, - scheduler_priority: *scheduler_priorities - .get(id) - // This should happen only if the canister is deleted. - .unwrap_or(&min_scheduler_priority), + scheduler_priority: if state.get_ref().canister_state(id).is_some() { + state.get_ref().canister_priority(id).accumulated_priority + } else { + // Canister was deleted. + min_scheduler_priority + }, }), Backend::Evicted { .. } | Backend::Empty => None, }) diff --git a/rs/replicated_state/src/replicated_state.rs b/rs/replicated_state/src/replicated_state.rs index ebee68d7506b..835860acbef7 100644 --- a/rs/replicated_state/src/replicated_state.rs +++ b/rs/replicated_state/src/replicated_state.rs @@ -25,7 +25,7 @@ use ic_registry_resource_limits::ResourceLimits; use ic_registry_routing_table::RoutingTable; use ic_registry_subnet_type::SubnetType; use ic_types::{ - AccumulatedPriority, CanisterId, NumBytes, SubnetId, Time, + CanisterId, NumBytes, SubnetId, Time, batch::{ConsensusResponse, RawQueryStats}, consensus::idkg::IDkgMasterPublicKeyId, ingress::IngressStatus, @@ -686,22 +686,6 @@ impl ReplicatedState { ) } - /// Time complexity: `O(n)` in the number of active canisters. - pub fn canister_accumulated_priorities(&self) -> BTreeMap { - self.canister_states - .keys() - .map(|canister_id| { - ( - *canister_id, - self.metadata - .subnet_schedule - .get(canister_id) - .accumulated_priority, - ) - }) - .collect() - } - /// Prunes the canister priorities of deleted canisters; and those that have /// all-zero accumulated priority, priority credit, heap delta and install code /// debits, and do not have a long-running execution.