From 7442cfaac52290244db102794dbd6098de4afac8 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 25 Jun 2024 12:32:46 +0000 Subject: [PATCH 01/51] . --- .../src/execution/update.rs | 10 +++++++++- .../src/execution_environment.rs | 5 +++++ rs/execution_environment/src/scheduler.rs | 19 +++++++++++++++---- .../v1/canister_state_bits.proto | 2 ++ .../gen/state/state.canister_state_bits.v1.rs | 8 ++++++++ rs/replicated_state/src/canister_state.rs | 12 ++++++++++++ .../src/canister_state/execution_state.rs | 9 +++++++++ .../src/canister_state/system_state.rs | 5 +++++ .../execution_environment/src/lib.rs | 6 ++++++ rs/types/types/src/messages.rs | 5 +++++ rs/types/types/src/methods.rs | 6 ++++++ 11 files changed, 82 insertions(+), 5 deletions(-) diff --git a/rs/execution_environment/src/execution/update.rs b/rs/execution_environment/src/execution/update.rs index dee34f33b51..ead270c3a33 100644 --- a/rs/execution_environment/src/execution/update.rs +++ b/rs/execution_environment/src/execution/update.rs @@ -149,6 +149,12 @@ pub fn execute_update( time, helper.call_context_id(), ), + CanisterCallOrTask::Task(CanisterTask::OnLowWasmMemory) => ApiType::system_task( + IC_00.get(), + SystemMethod::CanisterOnLowWasmMemory, + time, + helper.call_context_id(), + ), }; let memory_usage = helper.canister().memory_usage(); @@ -333,7 +339,9 @@ impl UpdateHelper { let initial_cycles_balance = canister.system_state.balance(); match original.call_or_task { - CanisterCallOrTask::Call(_) | CanisterCallOrTask::Task(CanisterTask::Heartbeat) => {} + CanisterCallOrTask::Call(_) + | CanisterCallOrTask::Task(CanisterTask::Heartbeat) + | CanisterCallOrTask::Task(CanisterTask::OnLowWasmMemory) => {} CanisterCallOrTask::Task(CanisterTask::GlobalTimer) => { // The global timer is one-off. canister.system_state.global_timer = CanisterTimer::Inactive; diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index d128f8ed23e..ffc0938eebb 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -3201,6 +3201,7 @@ impl ExecutionEnvironment { match task { ExecutionTask::Heartbeat | ExecutionTask::GlobalTimer + | ExecutionTask::OnLowWasmMemory | ExecutionTask::PausedExecution { .. } | ExecutionTask::AbortedExecution { .. } => { panic!( @@ -3853,6 +3854,10 @@ pub fn execute_canister( let task = CanisterMessageOrTask::Task(CanisterTask::GlobalTimer); (task, None) } + ExecutionTask::OnLowWasmMemory => { + let task = CanisterMessageOrTask::Task(CanisterTask::OnLowWasmMemory); + (task, None) + } ExecutionTask::AbortedExecution { input, prepaid_execution_cycles, diff --git a/rs/execution_environment/src/scheduler.rs b/rs/execution_environment/src/scheduler.rs index c53980c6272..5a2cbfd83d3 100644 --- a/rs/execution_environment/src/scheduler.rs +++ b/rs/execution_environment/src/scheduler.rs @@ -815,12 +815,14 @@ impl SchedulerImpl { .metrics .round_inner_heartbeat_overhead_duration .start_timer(); - // Remove all remaining `Heartbeat` and `GlobalTimer` tasks + // Remove all remaining `Heartbeat`, `GlobalTimer`, and `OnLowWasmMemory` tasks // because they will be added again in the next round. for canister_id in &heartbeat_and_timer_canister_ids { let canister = state.canister_state_mut(canister_id).unwrap(); canister.system_state.task_queue.retain(|task| match task { - ExecutionTask::Heartbeat | ExecutionTask::GlobalTimer => false, + ExecutionTask::Heartbeat + | ExecutionTask::GlobalTimer + | ExecutionTask::OnLowWasmMemory => false, ExecutionTask::PausedExecution { .. } | ExecutionTask::PausedInstallCode(..) | ExecutionTask::AbortedExecution { .. } @@ -1357,7 +1359,7 @@ impl SchedulerImpl { .iter() .filter(|(_, canister)| !canister.system_state.task_queue.is_empty()); - // 1. Heartbeat and GlobalTimer tasks exist only during the round + // 1. Heartbeat, GlobalTimer, and OnLowWasmMemory tasks exist only during the round // and must not exist after the round. // 2. Paused executions can exist only in ordinary rounds (not checkpoint rounds). // 3. If deterministic time slicing is disabled, then there are no paused tasks. @@ -1379,6 +1381,12 @@ impl SchedulerImpl { id ); } + ExecutionTask::OnLowWasmMemory => { + panic!( + "Unexpected on low wasm memory task after a round in canister {:?}", + id + ); + } ExecutionTask::PausedExecution { .. } | ExecutionTask::PausedInstallCode(_) => { assert_eq!( self.deterministic_time_slicing, @@ -2154,7 +2162,10 @@ fn observe_replicated_state_metrics( Some(&ExecutionTask::AbortedInstallCode { .. }) => { num_aborted_install += 1; } - Some(&ExecutionTask::Heartbeat) | Some(&ExecutionTask::GlobalTimer) | None => {} + Some(&ExecutionTask::Heartbeat) + | Some(&ExecutionTask::GlobalTimer) + | Some(&ExecutionTask::OnLowWasmMemory) + | None => {} } consumed_cycles_total += canister.system_state.canister_metrics.consumed_cycles; join_consumed_cycles_by_use_case( diff --git a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto index 5c790e48b47..0e32225d979 100644 --- a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto +++ b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto @@ -102,6 +102,7 @@ message WasmMethod { SYSTEM_METHOD_CANISTER_HEARTBEAT = 6; reserved 7; // deprecated SYSTEM_METHOD_EMPTY SYSTEM_METHOD_CANISTER_GLOBAL_TIMER = 8; + SYSTEM_METHOD_CANISTER_ON_LOW_WASM_MEMORY = 9; } oneof wasm_method { string update = 1; @@ -183,6 +184,7 @@ message ExecutionTask { CANISTER_TASK_UNSPECIFIED = 0; CANISTER_TASK_HEARTBEAT = 1; CANISTER_TASK_TIMER = 2; + CANISTER_TASK_ON_LOW_WASM_MEMORY = 3; } message AbortedExecution { diff --git a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs index fdea230ef2f..ffd0ec8bfa6 100644 --- a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs +++ b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs @@ -168,6 +168,7 @@ pub mod wasm_method { CanisterInspectMessage = 5, CanisterHeartbeat = 6, CanisterGlobalTimer = 8, + CanisterOnLowWasmMemory = 9, } impl SystemMethod { /// String value of the enum field names used in the ProtoBuf definition. @@ -184,6 +185,9 @@ pub mod wasm_method { SystemMethod::CanisterInspectMessage => "SYSTEM_METHOD_CANISTER_INSPECT_MESSAGE", SystemMethod::CanisterHeartbeat => "SYSTEM_METHOD_CANISTER_HEARTBEAT", SystemMethod::CanisterGlobalTimer => "SYSTEM_METHOD_CANISTER_GLOBAL_TIMER", + SystemMethod::CanisterOnLowWasmMemory => { + "SYSTEM_METHOD_CANISTER_ON_LOW_WASM_MEMORY" + } } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -197,6 +201,7 @@ pub mod wasm_method { "SYSTEM_METHOD_CANISTER_INSPECT_MESSAGE" => Some(Self::CanisterInspectMessage), "SYSTEM_METHOD_CANISTER_HEARTBEAT" => Some(Self::CanisterHeartbeat), "SYSTEM_METHOD_CANISTER_GLOBAL_TIMER" => Some(Self::CanisterGlobalTimer), + "SYSTEM_METHOD_CANISTER_ON_LOW_WASM_MEMORY" => Some(Self::CanisterOnLowWasmMemory), _ => None, } } @@ -374,6 +379,7 @@ pub mod execution_task { Unspecified = 0, Heartbeat = 1, Timer = 2, + OnLowWasmMemory = 3, } impl CanisterTask { /// String value of the enum field names used in the ProtoBuf definition. @@ -385,6 +391,7 @@ pub mod execution_task { CanisterTask::Unspecified => "CANISTER_TASK_UNSPECIFIED", CanisterTask::Heartbeat => "CANISTER_TASK_HEARTBEAT", CanisterTask::Timer => "CANISTER_TASK_TIMER", + CanisterTask::OnLowWasmMemory => "CANISTER_TASK_ON_LOW_WASM_MEMORY", } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -393,6 +400,7 @@ pub mod execution_task { "CANISTER_TASK_UNSPECIFIED" => Some(Self::Unspecified), "CANISTER_TASK_HEARTBEAT" => Some(Self::Heartbeat), "CANISTER_TASK_TIMER" => Some(Self::Timer), + "CANISTER_TASK_ON_LOW_WASM_MEMORY" => Some(Self::OnLowWasmMemory), _ => None, } } diff --git a/rs/replicated_state/src/canister_state.rs b/rs/replicated_state/src/canister_state.rs index 3f428e7f703..f3a0243c217 100644 --- a/rs/replicated_state/src/canister_state.rs +++ b/rs/replicated_state/src/canister_state.rs @@ -217,6 +217,7 @@ impl CanisterState { (None, true) => NextExecution::StartNew, (Some(ExecutionTask::Heartbeat), _) => NextExecution::StartNew, (Some(ExecutionTask::GlobalTimer), _) => NextExecution::StartNew, + (Some(ExecutionTask::OnLowWasmMemory), _) => NextExecution::StartNew, (Some(ExecutionTask::AbortedExecution { .. }), _) | (Some(ExecutionTask::PausedExecution { .. }), _) => NextExecution::ContinueLong, (Some(ExecutionTask::AbortedInstallCode { .. }), _) @@ -236,6 +237,7 @@ impl CanisterState { None | Some(ExecutionTask::Heartbeat) | Some(ExecutionTask::GlobalTimer) + | Some(ExecutionTask::OnLowWasmMemory) | Some(ExecutionTask::PausedExecution { .. }) | Some(ExecutionTask::PausedInstallCode(..)) | Some(ExecutionTask::AbortedInstallCode { .. }) => false, @@ -249,6 +251,7 @@ impl CanisterState { None | Some(ExecutionTask::Heartbeat) | Some(ExecutionTask::GlobalTimer) + | Some(ExecutionTask::OnLowWasmMemory) | Some(ExecutionTask::PausedInstallCode(..)) | Some(ExecutionTask::AbortedExecution { .. }) | Some(ExecutionTask::AbortedInstallCode { .. }) => false, @@ -262,6 +265,7 @@ impl CanisterState { None | Some(ExecutionTask::Heartbeat) | Some(ExecutionTask::GlobalTimer) + | Some(ExecutionTask::OnLowWasmMemory) | Some(ExecutionTask::PausedExecution { .. }) | Some(ExecutionTask::AbortedExecution { .. }) | Some(ExecutionTask::AbortedInstallCode { .. }) => false, @@ -275,6 +279,7 @@ impl CanisterState { None | Some(ExecutionTask::Heartbeat) | Some(ExecutionTask::GlobalTimer) + | Some(ExecutionTask::OnLowWasmMemory) | Some(ExecutionTask::PausedExecution { .. }) | Some(ExecutionTask::PausedInstallCode(..)) | Some(ExecutionTask::AbortedExecution { .. }) => false, @@ -461,6 +466,12 @@ impl CanisterState { self.exports_method(&WasmMethod::System(SystemMethod::CanisterGlobalTimer)) } + /// Returns true if the canister exports the `canister_on_low_wasm_memory` + /// system method. + pub fn exports_on_low_wasm_memory(&self) -> bool { + self.exports_method(&WasmMethod::System(SystemMethod::CanisterOnLowWasmMemory)) + } + /// Returns true if the canister exports the given Wasm method. pub fn exports_method(&self, method: &WasmMethod) -> bool { match &self.execution_state { @@ -527,6 +538,7 @@ impl CanisterState { ExecutionTask::AbortedInstallCode { .. } => false, ExecutionTask::Heartbeat | ExecutionTask::GlobalTimer + | Some(ExecutionTask::OnLowWasmMemory) | ExecutionTask::PausedExecution { .. } | ExecutionTask::PausedInstallCode(_) | ExecutionTask::AbortedExecution { .. } => true, diff --git a/rs/replicated_state/src/canister_state/execution_state.rs b/rs/replicated_state/src/canister_state/execution_state.rs index 20c86a01e47..fa556fda2d3 100644 --- a/rs/replicated_state/src/canister_state/execution_state.rs +++ b/rs/replicated_state/src/canister_state/execution_state.rs @@ -157,6 +157,9 @@ pub struct ExportedFunctions { /// Cached info about exporting a global timer method to skip expensive BTreeSet lookup. exports_global_timer: bool, + + /// Cached info about exporting a on low Wasm memory to skip expensive BTreeSet lookup. + exports_on_low_wasm_memory: bool, } impl ExportedFunctions { @@ -165,10 +168,13 @@ impl ExportedFunctions { exported_functions.contains(&WasmMethod::System(SystemMethod::CanisterHeartbeat)); let exports_global_timer = exported_functions.contains(&WasmMethod::System(SystemMethod::CanisterGlobalTimer)); + let exports_on_low_wasm_memory = + exported_functions.contains(&WasmMethod::System(SystemMethod::CanisterOnLowWasmMemory)); Self { exported_functions: Arc::new(exported_functions), exports_heartbeat, exports_global_timer, + exports_on_low_wasm_memory, } } @@ -177,6 +183,9 @@ impl ExportedFunctions { // Cached values. WasmMethod::System(SystemMethod::CanisterHeartbeat) => self.exports_heartbeat, WasmMethod::System(SystemMethod::CanisterGlobalTimer) => self.exports_global_timer, + WasmMethod::System(SystemMethod::CanisterOnLowWasmMemory) => { + self.exports_on_low_wasm_memory + } // Expensive lookup. _ => self.exported_functions.contains(method), } diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 69ed593da26..8768299ffbd 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -453,6 +453,10 @@ pub enum ExecutionTask { /// The task exists only within an execution round, it never gets serialized. GlobalTimer, + /// On low Wasm memory hook. + /// The task exists only within an execution round, it never gets serialized. + OnLowWasmMemory, + /// A paused execution task exists only within an epoch (between /// checkpoints). It is never serialized, and it turns into `AbortedExecution` /// before the checkpoint or when there are too many long-running executions. @@ -498,6 +502,7 @@ impl From<&ExecutionTask> for pb::ExecutionTask { match item { ExecutionTask::Heartbeat | ExecutionTask::GlobalTimer + | ExecutionTask::OnLowWasmMemory | ExecutionTask::PausedExecution { .. } | ExecutionTask::PausedInstallCode(_) => { panic!("Attempt to serialize ephemeral task: {:?}.", item); diff --git a/rs/test_utilities/execution_environment/src/lib.rs b/rs/test_utilities/execution_environment/src/lib.rs index 18367bb962c..77639069837 100644 --- a/rs/test_utilities/execution_environment/src/lib.rs +++ b/rs/test_utilities/execution_environment/src/lib.rs @@ -998,6 +998,12 @@ impl ExecutionTest { .task_queue .push_front(ExecutionTask::GlobalTimer); } + CanisterTask::OnLowWasmMemory => { + canister + .system_state + .task_queue + .push_front(ExecutionTask::OnLowWasmMemory); + } } let result = execute_canister( &self.exec_env, diff --git a/rs/types/types/src/messages.rs b/rs/types/types/src/messages.rs index ead44e244d4..7fcb64b0bdc 100644 --- a/rs/types/types/src/messages.rs +++ b/rs/types/types/src/messages.rs @@ -449,6 +449,7 @@ impl TryFrom for CanisterCall { pub enum CanisterTask { Heartbeat = 1, GlobalTimer = 2, + OnLowWasmMemory = 3, } impl From for SystemMethod { @@ -456,6 +457,7 @@ impl From for SystemMethod { match task { CanisterTask::Heartbeat => SystemMethod::CanisterHeartbeat, CanisterTask::GlobalTimer => SystemMethod::CanisterGlobalTimer, + CanisterTask::OnLowWasmMemory => SystemMethod::CanisterOnLowWasmMemory, } } } @@ -465,6 +467,7 @@ impl Display for CanisterTask { match self { Self::Heartbeat => write!(f, "Heartbeat task"), Self::GlobalTimer => write!(f, "Global timer task"), + Self::OnLowWasmMemory => write!(f, "On low Wasm memory task"), } } } @@ -474,6 +477,7 @@ impl From<&CanisterTask> for pb::execution_task::CanisterTask { match task { CanisterTask::Heartbeat => pb::execution_task::CanisterTask::Heartbeat, CanisterTask::GlobalTimer => pb::execution_task::CanisterTask::Timer, + CanisterTask::OnLowWasmMemory => pb::execution_task::CanisterTask::OnLowWasmMemory, } } } @@ -491,6 +495,7 @@ impl TryFrom for CanisterTask { } pb::execution_task::CanisterTask::Heartbeat => Ok(CanisterTask::Heartbeat), pb::execution_task::CanisterTask::Timer => Ok(CanisterTask::GlobalTimer), + pb::execution_task::CanisterTask::OnLowWasmMemory => Ok(CanisterTask::OnLowWasmMemory), } } } diff --git a/rs/types/types/src/methods.rs b/rs/types/types/src/methods.rs index 78ce563dfa9..4d8a64f80dc 100644 --- a/rs/types/types/src/methods.rs +++ b/rs/types/types/src/methods.rs @@ -147,6 +147,8 @@ pub enum SystemMethod { CanisterHeartbeat = 6, /// A system method that is run after a specified time. CanisterGlobalTimer = 7, + /// A system method that runs when the available Wasm memory is below threshold. + CanisterOnLowWasmMemory = 8, } impl TryFrom<&str> for SystemMethod { @@ -161,6 +163,7 @@ impl TryFrom<&str> for SystemMethod { "canister_inspect_message" => Ok(SystemMethod::CanisterInspectMessage), "canister_heartbeat" => Ok(SystemMethod::CanisterHeartbeat), "canister_global_timer" => Ok(SystemMethod::CanisterGlobalTimer), + "canister_on_low_wasm_memory" => Ok(SystemMethod::CanisterOnLowWasmMemory), _ => Err(format!("Cannot convert {} to SystemMethod.", value)), } } @@ -176,6 +179,7 @@ impl fmt::Display for SystemMethod { Self::CanisterInspectMessage => write!(f, "canister_inspect_message"), Self::CanisterHeartbeat => write!(f, "canister_heartbeat"), Self::CanisterGlobalTimer => write!(f, "canister_global_timer"), + Self::CanisterOnLowWasmMemory => write!(f, "canister_on_low_wasm_memory"), } } } @@ -192,6 +196,7 @@ impl From<&SystemMethod> for pb::wasm_method::SystemMethod { SystemMethod::CanisterInspectMessage => PbSystemMethod::CanisterInspectMessage, SystemMethod::CanisterHeartbeat => PbSystemMethod::CanisterHeartbeat, SystemMethod::CanisterGlobalTimer => PbSystemMethod::CanisterGlobalTimer, + SystemMethod::CanisterOnLowWasmMemory => PbSystemMethod::CanisterOnLowWasmMemory, } } } @@ -214,6 +219,7 @@ impl TryFrom for SystemMethod { PbSystemMethod::CanisterInspectMessage => Ok(SystemMethod::CanisterInspectMessage), PbSystemMethod::CanisterHeartbeat => Ok(SystemMethod::CanisterHeartbeat), PbSystemMethod::CanisterGlobalTimer => Ok(SystemMethod::CanisterGlobalTimer), + PbSystemMethod::CanisterOnLowWasmMemory => Ok(SystemMethod::CanisterOnLowWasmMemory), } } } From 26790e0db186aee493a4670a985c497027cf02c4 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 25 Jun 2024 12:40:22 +0000 Subject: [PATCH 02/51] . --- rs/replicated_state/src/canister_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/replicated_state/src/canister_state.rs b/rs/replicated_state/src/canister_state.rs index f3a0243c217..913e0d81009 100644 --- a/rs/replicated_state/src/canister_state.rs +++ b/rs/replicated_state/src/canister_state.rs @@ -538,7 +538,7 @@ impl CanisterState { ExecutionTask::AbortedInstallCode { .. } => false, ExecutionTask::Heartbeat | ExecutionTask::GlobalTimer - | Some(ExecutionTask::OnLowWasmMemory) + | ExecutionTask::OnLowWasmMemory | ExecutionTask::PausedExecution { .. } | ExecutionTask::PausedInstallCode(_) | ExecutionTask::AbortedExecution { .. } => true, From ba9b326ce1ec1801e737100163f00e957200723e Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 25 Jun 2024 12:44:33 +0000 Subject: [PATCH 03/51] . --- rs/execution_environment/src/execution_environment.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index ffc0938eebb..fae338b0126 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -3298,7 +3298,8 @@ impl ExecutionEnvironment { ExecutionTask::AbortedExecution { .. } | ExecutionTask::AbortedInstallCode { .. } | ExecutionTask::Heartbeat - | ExecutionTask::GlobalTimer => task, + | ExecutionTask::GlobalTimer + | ExecutionTask::OnLowWasmMemory => task, ExecutionTask::PausedExecution { id, .. } => { let paused = self.take_paused_execution(id).unwrap(); let (input, prepaid_execution_cycles) = paused.abort(log); From 6c7a6dceb35834c7c47aad49d00f7a863b9b925e Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Mon, 1 Jul 2024 14:36:00 +0000 Subject: [PATCH 04/51] fix types test --- rs/types/types/src/messages.rs | 2 +- rs/types/types/src/methods.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/types/types/src/messages.rs b/rs/types/types/src/messages.rs index 7fcb64b0bdc..627684d1a7e 100644 --- a/rs/types/types/src/messages.rs +++ b/rs/types/types/src/messages.rs @@ -846,7 +846,7 @@ mod tests { // See note [Handling changes to Enums in Replicated State] for how to proceed. assert_eq!( CanisterTask::iter().map(|x| x as i32).collect::>(), - [1, 2] + [1, 2, 3] ); } diff --git a/rs/types/types/src/methods.rs b/rs/types/types/src/methods.rs index 4d8a64f80dc..3b4dbcaad2a 100644 --- a/rs/types/types/src/methods.rs +++ b/rs/types/types/src/methods.rs @@ -417,7 +417,7 @@ mod tests { // See note [Handling changes to Enums in Replicated State] for how to proceed. assert_eq!( SystemMethod::iter().map(|x| x as i32).collect::>(), - [1, 2, 3, 4, 5, 6, 7] + [1, 2, 3, 4, 5, 6, 7, 8] ); } From 2c74c747c34c27567bdf46ae533fd02465db9be1 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 2 Jul 2024 11:35:40 +0000 Subject: [PATCH 05/51] . --- rs/system_api/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 073ba833493..3f084ec30e7 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -615,7 +615,10 @@ impl ApiType { ApiType::SystemTask { system_task, .. } => match system_task { SystemMethod::CanisterHeartbeat => "heartbeat", SystemMethod::CanisterGlobalTimer => "global timer", - _ => panic!("Only `canister_heartbeat` and `canister_global_timer` are allowed."), + SystemMethod::CanisterOnLowWasmMemory => "on low Wasm memory", + _ => { + panic!("Only `canister_heartbeat`, `canister_global_timer`, and "canister_on_low_wasm_memory" are allowed.") + } }, ApiType::Update { .. } => "update", ApiType::ReplicatedQuery { .. } => "replicated query", From 0846be817f3ae316cd0cc4f65d6b8dbe37b08e04 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 2 Jul 2024 11:42:47 +0000 Subject: [PATCH 06/51] . --- rs/system_api/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 3f084ec30e7..53d3a8264fb 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -616,8 +616,12 @@ impl ApiType { SystemMethod::CanisterHeartbeat => "heartbeat", SystemMethod::CanisterGlobalTimer => "global timer", SystemMethod::CanisterOnLowWasmMemory => "on low Wasm memory", - _ => { - panic!("Only `canister_heartbeat`, `canister_global_timer`, and "canister_on_low_wasm_memory" are allowed.") + SystemMethod::CanisterStart + | SystemMethod::CanisterInit + | SystemMethod::CanisterPreUpgrade + | SystemMethod::CanisterPostUpgrade + | SystemMethod::CanisterInspectMessage => { + panic!("Only `canister_heartbeat`, `canister_global_timer`, and `canister_on_low_wasm_memory` are allowed.") } }, ApiType::Update { .. } => "update", From 3edf927f0c01441141dda79f735feaad723b3a39 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 19 Jul 2024 12:54:55 +0000 Subject: [PATCH 07/51] . --- rs/canister_sandbox/src/sandbox_server.rs | 1 + .../src/canister_manager.rs | 3 ++ .../src/canister_settings.rs | 53 +++++++++++++++++++ .../src/execution/install_code.rs | 1 + .../v1/canister_state_bits.proto | 1 + .../gen/state/state.canister_state_bits.v1.rs | 2 + rs/replicated_state/src/canister_state.rs | 5 ++ .../src/canister_state/system_state.rs | 5 ++ rs/state_layout/src/state_layout.rs | 3 ++ rs/state_layout/src/state_layout/tests.rs | 1 + rs/state_manager/src/checkpoint.rs | 1 + rs/state_manager/src/tip.rs | 1 + .../src/sandbox_safe_system_state.rs | 8 ++- rs/test_utilities/state/src/lib.rs | 12 +++++ rs/types/management_canister_types/src/lib.rs | 13 +++++ 15 files changed, 109 insertions(+), 1 deletion(-) diff --git a/rs/canister_sandbox/src/sandbox_server.rs b/rs/canister_sandbox/src/sandbox_server.rs index f47498eb704..1e6e9b39aff 100644 --- a/rs/canister_sandbox/src/sandbox_server.rs +++ b/rs/canister_sandbox/src/sandbox_server.rs @@ -202,6 +202,7 @@ mod tests { CanisterStatusView::Running, NumSeconds::from(3600), MemoryAllocation::BestEffort, + NumBytes::new(0), ComputeAllocation::default(), Cycles::new(1_000_000), Cycles::zero(), diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 85454688664..a72b7889e29 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -629,6 +629,9 @@ impl CanisterManager { } canister.system_state.memory_allocation = memory_allocation; } + if let Some(wasm_memory_threshold) = settings.wasm_memory_threshold() { + canister.system_state.wasm_memory_threshold = wasm_memory_threshold; + } if let Some(limit) = settings.reserved_cycles_limit() { canister.system_state.set_reserved_balance_limit(limit); } diff --git a/rs/execution_environment/src/canister_settings.rs b/rs/execution_environment/src/canister_settings.rs index 8d3cf9c5613..6162a435c0c 100644 --- a/rs/execution_environment/src/canister_settings.rs +++ b/rs/execution_environment/src/canister_settings.rs @@ -22,6 +22,7 @@ pub(crate) struct CanisterSettings { pub(crate) controllers: Option>, pub(crate) compute_allocation: Option, pub(crate) memory_allocation: Option, + pub(crate) wasm_memory_threshold: Option, pub(crate) freezing_threshold: Option, pub(crate) reserved_cycles_limit: Option, pub(crate) log_visibility: Option, @@ -34,6 +35,7 @@ impl CanisterSettings { controllers: Option>, compute_allocation: Option, memory_allocation: Option, + wasm_memory_threshold: Option, freezing_threshold: Option, reserved_cycles_limit: Option, log_visibility: Option, @@ -44,6 +46,7 @@ impl CanisterSettings { controllers, compute_allocation, memory_allocation, + wasm_memory_threshold, freezing_threshold, reserved_cycles_limit, log_visibility, @@ -67,6 +70,10 @@ impl CanisterSettings { self.memory_allocation } + pub fn wasm_memory_threshold(&self) -> Option { + self.wasm_memory_threshold + } + pub fn freezing_threshold(&self) -> Option { self.freezing_threshold } @@ -135,6 +142,17 @@ impl TryFrom for CanisterSettings { None => None, }; + let wasm_memory_threshold = match input.wasm_memory_threshold { + Some(wmt) => { + let wmt = wmt + .0 + .to_u64() + .ok_or(UpdateSettingsError::WasmMemoryThresholdOutOfRange { provided: wmt })?; + Some(NumBytes::new(wmt)) + } + None => None, + }; + Ok(CanisterSettings::new( controller, input @@ -142,6 +160,7 @@ impl TryFrom for CanisterSettings { .map(|controllers| controllers.get().clone()), compute_allocation, memory_allocation, + wasm_memory_threshold, freezing_threshold, reserved_cycles_limit, input.log_visibility, @@ -166,6 +185,7 @@ pub(crate) struct CanisterSettingsBuilder { controllers: Option>, compute_allocation: Option, memory_allocation: Option, + wasm_memory_threshold: Option, freezing_threshold: Option, reserved_cycles_limit: Option, log_visibility: Option, @@ -180,6 +200,7 @@ impl CanisterSettingsBuilder { controllers: None, compute_allocation: None, memory_allocation: None, + wasm_memory_threshold: None, freezing_threshold: None, reserved_cycles_limit: None, log_visibility: None, @@ -193,6 +214,7 @@ impl CanisterSettingsBuilder { controllers: self.controllers, compute_allocation: self.compute_allocation, memory_allocation: self.memory_allocation, + wasm_memory_threshold: self.wasm_memory_threshold, freezing_threshold: self.freezing_threshold, reserved_cycles_limit: self.reserved_cycles_limit, log_visibility: self.log_visibility, @@ -228,6 +250,13 @@ impl CanisterSettingsBuilder { } } + pub fn with_wasm_memory_threshold(self, wasm_memory_threshold: NumBytes) -> Self { + Self { + wasm_memory_threshold: Some(wasm_memory_threshold), + ..self + } + } + pub fn with_freezing_threshold(self, freezing_threshold: NumSeconds) -> Self { Self { freezing_threshold: Some(freezing_threshold), @@ -263,6 +292,7 @@ pub enum UpdateSettingsError { FreezingThresholdOutOfRange { provided: candid::Nat }, ReservedCyclesLimitOutOfRange { provided: candid::Nat }, WasmMemoryLimitOutOfRange { provided: candid::Nat }, + WasmMemoryThresholdOutOfRange { provided: candid::Nat }, } impl From for UserError { @@ -305,6 +335,13 @@ impl From for UserError { provided ), ), + UpdateSettingsError::WasmMemoryThresholdOutOfRange { provided } => UserError::new( + ErrorCode::CanisterContractViolation, + format!( + "Wasm memory threshold expected to be in the range of [0..2^64-1], got {}", + provided + ), + ), } } } @@ -326,6 +363,7 @@ pub(crate) struct ValidatedCanisterSettings { controllers: Option>, compute_allocation: Option, memory_allocation: Option, + wasm_memory_threshold: Option, freezing_threshold: Option, reserved_cycles_limit: Option, reservation_cycles: Cycles, @@ -350,6 +388,10 @@ impl ValidatedCanisterSettings { self.memory_allocation } + pub fn wasm_memory_threshold(&self) -> Option { + self.wasm_memory_threshold + } + pub fn freezing_threshold(&self) -> Option { self.freezing_threshold } @@ -472,6 +514,16 @@ pub(crate) fn validate_canister_settings( None => {} } + if let Some(wasm_memory_limit) = settings.wasm_memory_limit() { + if let Some(wasm_memory_threshold) = settings.wasm_memory_threshold() { + if wasm_memory_threshold > wasm_memory_limit { + return Err(CanisterManagerError::InvalidSettings { + message: format!("Invalid settings: 'wasm_memory_threshold' cannot be larger than 'wasm_memory_limit'. 'wasm_memory_threshold': {}, 'wasm_memory_limit': {}", wasm_memory_threshold, wasm_memory_limit), + }); + } + } + } + let new_memory_allocation = settings .memory_allocation .unwrap_or(canister_memory_allocation); @@ -561,6 +613,7 @@ pub(crate) fn validate_canister_settings( controllers: settings.controllers(), compute_allocation: settings.compute_allocation(), memory_allocation: settings.memory_allocation(), + wasm_memory_threshold: settings.wasm_memory_threshold(), freezing_threshold: settings.freezing_threshold(), reserved_cycles_limit: settings.reserved_cycles_limit(), reservation_cycles, diff --git a/rs/execution_environment/src/execution/install_code.rs b/rs/execution_environment/src/execution/install_code.rs index 2ada18fb1b6..e7a1c918311 100644 --- a/rs/execution_environment/src/execution/install_code.rs +++ b/rs/execution_environment/src/execution/install_code.rs @@ -509,6 +509,7 @@ impl InstallCodeHelper { controllers: None, compute_allocation: original.requested_compute_allocation, memory_allocation: original.requested_memory_allocation, + wasm_memory_threshold: None, freezing_threshold: None, reserved_cycles_limit: None, log_visibility: None, diff --git a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto index 0e32225d979..8a78cf1c077 100644 --- a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto +++ b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto @@ -417,4 +417,5 @@ message CanisterStateBits { reserved 47; int64 priority_credit = 48; LongExecutionMode long_execution_mode = 49; + optional uint64 wasm_memory_threshold = 50; } diff --git a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs index ffd0ec8bfa6..5e848791ac9 100644 --- a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs +++ b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs @@ -666,6 +666,8 @@ pub struct CanisterStateBits { pub priority_credit: i64, #[prost(enumeration = "LongExecutionMode", tag = "49")] pub long_execution_mode: i32, + #[prost(uint64, optional, tag = "50")] + pub wasm_memory_threshold: ::core::option::Option, #[prost(oneof = "canister_state_bits::CanisterStatus", tags = "11, 12, 13")] pub canister_status: ::core::option::Option, } diff --git a/rs/replicated_state/src/canister_state.rs b/rs/replicated_state/src/canister_state.rs index 913e0d81009..5cd4601b64a 100644 --- a/rs/replicated_state/src/canister_state.rs +++ b/rs/replicated_state/src/canister_state.rs @@ -435,6 +435,11 @@ impl CanisterState { self.system_state.memory_allocation } + /// Returns the current Wasm memory threshold of the canister. + pub fn wasm_memory_threshold(&self) -> NumBytes { + self.system_state.wasm_memory_threshold + } + /// Returns the canister's memory limit: its reservation, if set; else the /// provided `default_limit`. pub fn memory_limit(&self, default_limit: NumBytes) -> NumBytes { diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 8768299ffbd..88d6b59e407 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -274,6 +274,8 @@ pub struct SystemState { queues: CanisterQueues, /// The canister's memory allocation. pub memory_allocation: MemoryAllocation, + /// Threshold used for activation of canister_on_low_wasm_memory hook. + pub wasm_memory_threshold: NumBytes, pub freeze_threshold: NumSeconds, /// The status of the canister: Running, Stopping, or Stopped. /// Different statuses allow for different behaviors on the SystemState. @@ -710,6 +712,7 @@ impl SystemState { reserved_balance: Cycles::zero(), reserved_balance_limit: None, memory_allocation: MemoryAllocation::BestEffort, + wasm_memory_threshold: NumBytes::new(0), freeze_threshold, status, certified_data: Default::default(), @@ -732,6 +735,7 @@ impl SystemState { canister_id: CanisterId, queues: CanisterQueues, memory_allocation: MemoryAllocation, + wasm_memory_threshold: NumBytes, freeze_threshold: NumSeconds, status: CanisterStatus, certified_data: Vec, @@ -756,6 +760,7 @@ impl SystemState { canister_id, queues, memory_allocation, + wasm_memory_threshold, freeze_threshold, status, certified_data, diff --git a/rs/state_layout/src/state_layout.rs b/rs/state_layout/src/state_layout.rs index 6b312b26e5c..5f9143965e8 100644 --- a/rs/state_layout/src/state_layout.rs +++ b/rs/state_layout/src/state_layout.rs @@ -143,6 +143,7 @@ pub struct CanisterStateBits { pub long_execution_mode: LongExecutionMode, pub execution_state_bits: Option, pub memory_allocation: MemoryAllocation, + pub wasm_memory_threshold: NumBytes, pub freeze_threshold: NumSeconds, pub cycles_balance: Cycles, pub cycles_debit: Cycles, @@ -1881,6 +1882,7 @@ impl From for pb_canister_state_bits::CanisterStateBits { .into(), execution_state_bits: item.execution_state_bits.as_ref().map(|v| v.into()), memory_allocation: item.memory_allocation.bytes().get(), + wasm_memory_threshold: Some(item.wasm_memory_threshold.get()), freeze_threshold: item.freeze_threshold.get(), cycles_balance: Some(item.cycles_balance.into()), cycles_debit: Some(item.cycles_debit.into()), @@ -2017,6 +2019,7 @@ impl TryFrom for CanisterStateBits { typ: "MemoryAllocation", err: format!("{:?}", e), })?, + wasm_memory_threshold: NumBytes::new(value.wasm_memory_threshold.unwrap_or(0)), freeze_threshold: NumSeconds::from(value.freeze_threshold), cycles_balance, cycles_debit, diff --git a/rs/state_layout/src/state_layout/tests.rs b/rs/state_layout/src/state_layout/tests.rs index 8352400893a..22fdad792d3 100644 --- a/rs/state_layout/src/state_layout/tests.rs +++ b/rs/state_layout/src/state_layout/tests.rs @@ -30,6 +30,7 @@ fn default_canister_state_bits() -> CanisterStateBits { long_execution_mode: LongExecutionMode::default(), execution_state_bits: None, memory_allocation: MemoryAllocation::default(), + wasm_memory_threshold: NumBytes::new(0), freeze_threshold: NumSeconds::from(0), cycles_balance: Cycles::zero(), cycles_debit: Cycles::zero(), diff --git a/rs/state_manager/src/checkpoint.rs b/rs/state_manager/src/checkpoint.rs index 1aecec12ce5..665fd6b56e7 100644 --- a/rs/state_manager/src/checkpoint.rs +++ b/rs/state_manager/src/checkpoint.rs @@ -401,6 +401,7 @@ pub fn load_canister_state( *canister_id, queues, canister_state_bits.memory_allocation, + canister_state_bits.wasm_memory_threshold, canister_state_bits.freeze_threshold, canister_state_bits.status, canister_state_bits.certified_data, diff --git a/rs/state_manager/src/tip.rs b/rs/state_manager/src/tip.rs index 966e3cb5a34..13396322ed6 100644 --- a/rs/state_manager/src/tip.rs +++ b/rs/state_manager/src/tip.rs @@ -840,6 +840,7 @@ fn serialize_canister_to_tip( long_execution_mode: canister_state.scheduler_state.long_execution_mode, accumulated_priority: canister_state.scheduler_state.accumulated_priority, memory_allocation: canister_state.system_state.memory_allocation, + wasm_memory_threshold: canister_state.system_state.wasm_memory_threshold, freeze_threshold: canister_state.system_state.freeze_threshold, cycles_balance: canister_state.system_state.balance(), cycles_debit: canister_state.system_state.ingress_induction_cycles_debit(), diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 17afd32f008..b742deee5d1 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -558,6 +558,7 @@ pub struct SandboxSafeSystemState { dirty_page_overhead: NumInstructions, freeze_threshold: NumSeconds, memory_allocation: MemoryAllocation, + wasm_memory_threshold: NumBytes, compute_allocation: ComputeAllocation, initial_cycles_balance: Cycles, initial_reserved_balance: Cycles, @@ -588,6 +589,7 @@ impl SandboxSafeSystemState { status: CanisterStatusView, freeze_threshold: NumSeconds, memory_allocation: MemoryAllocation, + wasm_memory_threshold: NumBytes, compute_allocation: ComputeAllocation, initial_cycles_balance: Cycles, initial_reserved_balance: Cycles, @@ -617,6 +619,7 @@ impl SandboxSafeSystemState { dirty_page_overhead, freeze_threshold, memory_allocation, + wasm_memory_threshold, compute_allocation, system_state_changes: SystemStateChanges { // Start indexing new batch of canister log records from the given index. @@ -701,6 +704,7 @@ impl SandboxSafeSystemState { CanisterStatusView::from_full_status(&system_state.status), system_state.freeze_threshold, system_state.memory_allocation, + system_state.wasm_memory_threshold, compute_allocation, system_state.balance(), system_state.reserved_balance(), @@ -1288,7 +1292,8 @@ mod tests { use ic_types::{ messages::{RequestMetadata, NO_DEADLINE}, time::CoarseTime, - CanisterTimer, ComputeAllocation, Cycles, MemoryAllocation, NumInstructions, Time, + CanisterTimer, ComputeAllocation, Cycles, MemoryAllocation, NumBytes, NumInstructions, + Time, }; use crate::{ @@ -1366,6 +1371,7 @@ mod tests { CanisterStatusView::Running, NumSeconds::from(3600), MemoryAllocation::BestEffort, + NumBytes::new(0), ComputeAllocation::default(), Cycles::new(1_000_000), Cycles::zero(), diff --git a/rs/test_utilities/state/src/lib.rs b/rs/test_utilities/state/src/lib.rs index 0fc2df067a4..5ac7380cab0 100644 --- a/rs/test_utilities/state/src/lib.rs +++ b/rs/test_utilities/state/src/lib.rs @@ -198,6 +198,7 @@ pub struct CanisterStateBuilder { stable_memory: Option>, wasm: Option>, memory_allocation: MemoryAllocation, + wasm_memory_threshold: NumBytes, compute_allocation: ComputeAllocation, ingress_queue: Vec, status: CanisterStatusType, @@ -245,6 +246,11 @@ impl CanisterStateBuilder { self } + pub fn with_wasm_memory_threshold>(mut self, num_bytes: B) -> Self { + self.wasm_memory_threshold = num_bytes.into(); + self + } + pub fn with_compute_allocation(mut self, allocation: ComputeAllocation) -> Self { self.compute_allocation = allocation; self @@ -398,6 +404,7 @@ impl Default for CanisterStateBuilder { stable_memory: None, wasm: None, memory_allocation: MemoryAllocation::BestEffort, + wasm_memory_threshold: NumBytes::new(0), compute_allocation: ComputeAllocation::zero(), ingress_queue: Vec::default(), status: CanisterStatusType::Running, @@ -456,6 +463,11 @@ impl SystemStateBuilder { self } + pub fn wasm_memory_threshold(mut self, wasm_memory_threshold: NumBytes) -> Self { + self.system_state.wasm_memory_threshold = wasm_memory_threshold; + self + } + pub fn freeze_threshold(mut self, threshold: NumSeconds) -> Self { self.system_state.freeze_threshold = threshold; self diff --git a/rs/types/management_canister_types/src/lib.rs b/rs/types/management_canister_types/src/lib.rs index 74f57a46c32..6d9151599ff 100644 --- a/rs/types/management_canister_types/src/lib.rs +++ b/rs/types/management_canister_types/src/lib.rs @@ -1642,6 +1642,7 @@ impl DataSize for PrincipalId { /// reserved_cycles_limit: opt nat; /// log_visibility : opt log_visibility; /// wasm_memory_limit: opt nat; +/// wasm_memory_threshold: opt nat; /// })` #[derive(Default, Clone, CandidType, Deserialize, Debug, PartialEq, Eq)] pub struct CanisterSettingsArgs { @@ -1654,6 +1655,7 @@ pub struct CanisterSettingsArgs { pub reserved_cycles_limit: Option, pub log_visibility: Option, pub wasm_memory_limit: Option, + pub wasm_memory_threshold: Option, } impl Payload<'_> for CanisterSettingsArgs {} @@ -1671,6 +1673,7 @@ impl CanisterSettingsArgs { reserved_cycles_limit: None, log_visibility: None, wasm_memory_limit: None, + wasm_memory_threshold: None, } } @@ -1689,6 +1692,7 @@ pub struct CanisterSettingsArgsBuilder { reserved_cycles_limit: Option, log_visibility: Option, wasm_memory_limit: Option, + wasm_memory_threshold: Option, } #[allow(dead_code)] @@ -1707,6 +1711,7 @@ impl CanisterSettingsArgsBuilder { reserved_cycles_limit: self.reserved_cycles_limit, log_visibility: self.log_visibility, wasm_memory_limit: self.wasm_memory_limit, + wasm_memory_threshold: self.wasm_memory_threshold, } } @@ -1790,6 +1795,14 @@ impl CanisterSettingsArgsBuilder { ..self } } + + /// Sets the Wasm memory threshold in bytes. + pub fn with_wasm_memory_threshold(self, wasm_memory_threshold: u64) -> Self { + Self { + wasm_memory_threshold: Some(candid::Nat::from(wasm_memory_threshold)), + ..self + } + } } /// Struct used for encoding/decoding From 72535d6c789d0a91562ef13d472a343b634dab1d Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Mon, 22 Jul 2024 14:16:29 +0000 Subject: [PATCH 08/51] fix test --- rs/cycles_account_manager/src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/rs/cycles_account_manager/src/lib.rs b/rs/cycles_account_manager/src/lib.rs index 0520050132e..91c0e1c7b14 100644 --- a/rs/cycles_account_manager/src/lib.rs +++ b/rs/cycles_account_manager/src/lib.rs @@ -44,7 +44,7 @@ const SECONDS_PER_DAY: u128 = 24 * 60 * 60; /// Maximum payload size of a management call to update_settings /// overriding the canister's freezing threshold. -const MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE: usize = 256; +const MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE: usize = 267; /// Errors returned by the [`CyclesAccountManager`]. #[derive(Clone, Debug, PartialEq, Eq)] @@ -1183,7 +1183,15 @@ mod tests { .build(), sender_canister_version: None, // ingress messages are not supposed to set this field }; - assert!(2 * Encode!(&payload).unwrap().len() <= MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE); + + let payload_size = 2 * Encode!(&payload).unwrap().len(); + + assert!( + payload_size <= MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE, + "Payload size: {}, is greater than MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE: {}.", + payload_size, + MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE + ); } #[test] From c15f18388aa28cc0059eda66f9ebbedf2e7f5650 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 23 Jul 2024 08:11:13 +0000 Subject: [PATCH 09/51] . --- rs/execution_environment/src/scheduler.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rs/execution_environment/src/scheduler.rs b/rs/execution_environment/src/scheduler.rs index 4473f431e57..33ae32768af 100644 --- a/rs/execution_environment/src/scheduler.rs +++ b/rs/execution_environment/src/scheduler.rs @@ -816,7 +816,6 @@ impl SchedulerImpl { .round_inner_heartbeat_overhead_duration .start_timer(); // Remove all remaining `Heartbeat`, `GlobalTimer`, and `OnLowWasmMemory` tasks - // Remove all remaining `Heartbeat`, `GlobalTimer`, and `OnLowWasmMemory` tasks // because they will be added again in the next round. for canister_id in &heartbeat_and_timer_canister_ids { let canister = state.canister_state_mut(canister_id).unwrap(); From 0388a31aad0fe448127ebfe55e661feb32cbbcf9 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 23 Jul 2024 08:31:44 +0000 Subject: [PATCH 10/51] fix test --- rs/nns/cmc/cmc.did | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/nns/cmc/cmc.did b/rs/nns/cmc/cmc.did index fb751cbd8a3..91133bbac71 100644 --- a/rs/nns/cmc/cmc.did +++ b/rs/nns/cmc/cmc.did @@ -12,6 +12,7 @@ type CanisterSettings = record { reserved_cycles_limit : opt nat; log_visibility : opt log_visibility; wasm_memory_limit : opt nat; + wasm_memory_threshold : opt nat; }; type Subaccount = opt blob; type Memo = opt blob; From 15e4785c992400e8a12aa429e795d2ea0a7a5e37 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 24 Jul 2024 21:54:39 +0000 Subject: [PATCH 11/51] . --- rs/execution_environment/src/canister_settings.rs | 1 + rs/replicated_state/src/canister_state.rs | 5 ----- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/rs/execution_environment/src/canister_settings.rs b/rs/execution_environment/src/canister_settings.rs index 7d6e7646356..96408b5571d 100644 --- a/rs/execution_environment/src/canister_settings.rs +++ b/rs/execution_environment/src/canister_settings.rs @@ -21,6 +21,7 @@ pub(crate) struct CanisterSettings { pub(crate) controllers: Option>, pub(crate) compute_allocation: Option, pub(crate) memory_allocation: Option, + /// Threshold used for activation of canister_on_low_wasm_memory hook. pub(crate) wasm_memory_threshold: Option, pub(crate) freezing_threshold: Option, pub(crate) reserved_cycles_limit: Option, diff --git a/rs/replicated_state/src/canister_state.rs b/rs/replicated_state/src/canister_state.rs index 7623dae197b..3caa0c13ade 100644 --- a/rs/replicated_state/src/canister_state.rs +++ b/rs/replicated_state/src/canister_state.rs @@ -449,11 +449,6 @@ impl CanisterState { } } - /// Returns the Wasm memory limit from the canister settings. - pub fn wasm_memory_limit(&self) -> Option { - self.system_state.wasm_memory_limit - } - /// Returns the current compute allocation for the canister. pub fn compute_allocation(&self) -> ComputeAllocation { self.scheduler_state.compute_allocation From 6e271f5f8fe51c138971fd0e52c9b4bcae34309a Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 24 Jul 2024 21:59:41 +0000 Subject: [PATCH 12/51] Revert accidentally deleted function. --- rs/replicated_state/src/canister_state.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rs/replicated_state/src/canister_state.rs b/rs/replicated_state/src/canister_state.rs index 3caa0c13ade..7623dae197b 100644 --- a/rs/replicated_state/src/canister_state.rs +++ b/rs/replicated_state/src/canister_state.rs @@ -449,6 +449,11 @@ impl CanisterState { } } + /// Returns the Wasm memory limit from the canister settings. + pub fn wasm_memory_limit(&self) -> Option { + self.system_state.wasm_memory_limit + } + /// Returns the current compute allocation for the canister. pub fn compute_allocation(&self) -> ComputeAllocation { self.scheduler_state.compute_allocation From 6dc8de8c8e3d5a8d067dd07427c01049dd2df14b Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Mon, 29 Jul 2024 15:13:42 +0000 Subject: [PATCH 13/51] . --- .../src/canister_state/system_state.rs | 50 +++++++++++++++ rs/system_api/src/lib.rs | 62 ++++++++++++++++++- .../src/sandbox_safe_system_state.rs | 20 ++++++ 3 files changed, 131 insertions(+), 1 deletion(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index ca3dadcc181..900370dcd46 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -356,6 +356,56 @@ pub struct SystemState { /// Next local snapshot id. pub next_snapshot_id: u64, + + /// Status of low_on_wasm_memory hook execution. + pub on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, +} + +/// A wrapper around the different canister statuses. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum OnLowWasmMemoryHookStatus { + ConditionNotSatisfied, + Ready, + Executed, +} + +impl OnLowWasmMemoryHookStatus { + fn update( + &mut self, + wasm_memory_threshold: NumBytes, + memory_allocation: Option, + used_stable_memory: NumBytes, + used_wasm_memory: NumBytes, + ) { + // The maximum Wasm memory occupied by the canister is 4 GiB. + let max_wasm_capacity = NumBytes::new(4 * 1024 * 1024); + + // If the canister has memory allocation, then it maximum allowed Wasm memory + // can be calculated as min(memory_allocation - used_stable_memory, 4 GiB). + let wasm_capacity = if let Some(memory_allocation) = memory_allocation{ + std::cmp::min(memory_allocation - used_stable_memory, max_wasm_capacity) + } else { + max_wasm_capacity + }; + + let left_wasm_space = wasm_capacity - used_wasm_memory; + + if left_wasm_space >= wasm_memory_threshold { + *self = OnLowWasmMemoryHookStatus::ConditionNotSatisfied; + } else { + if *self != OnLowWasmMemoryHookStatus::Executed { + *self = OnLowWasmMemoryHookStatus::Ready; + } + } + } + + fn should_schedule_hook(&self) -> bool { + *self == OnLowWasmMemoryHookStatus::Ready + } + + fn execute_hook(&mut self) { + *self = OnLowWasmMemoryHookStatus::Executed; + } } /// A wrapper around the different canister statuses. diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index d5d958fcffc..196804bde33 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -683,6 +683,12 @@ impl std::fmt::Display for ApiType { } } +#[derive(Debug, PartialEq, Eq)] +enum ExecutionMemoryType { + WasmMemory, + StableMemory +} + /// A struct to gather the relevant fields that correspond to a canister's /// memory consumption. struct MemoryUsage { @@ -695,6 +701,12 @@ struct MemoryUsage { /// The current amount of execution memory that the canister is using. current_usage: NumBytes, + /// The current amount of stable memory that the canister is using. + stable_memory_usage: NumBytes, + + /// The current amount of Wasm memory that the canister is using. + wasm_memory_usage: NumBytes, + /// The current amount of message memory that the canister is using. current_message_usage: NumBytes, @@ -720,6 +732,7 @@ impl MemoryUsage { limit: NumBytes, wasm_memory_limit: Option, current_usage: NumBytes, + stable_memory_usage: NumBytes, current_message_usage: NumBytes, subnet_available_memory: SubnetAvailableMemory, memory_allocation: MemoryAllocation, @@ -737,10 +750,16 @@ impl MemoryUsage { limit ); } + let (wasm_memory_usage, overflow) = current_usage.get().overflowing_sub(stable_memory_usage.get()); + + debug_assert!(!overflow); + Self { limit, wasm_memory_limit, current_usage, + stable_memory_usage, + wasm_memory_usage: NumBytes::new(wasm_memory_usage), current_message_usage, subnet_available_memory, allocated_execution_memory: NumBytes::from(0), @@ -811,6 +830,7 @@ impl MemoryUsage { api_type: &ApiType, sandbox_safe_system_state: &mut SandboxSafeSystemState, subnet_memory_saturation: &ResourceSaturation, + execution_memory_type: ExecutionMemoryType, ) -> HypervisorResult<()> { let (new_usage, overflow) = self .current_usage @@ -850,9 +870,14 @@ impl MemoryUsage { ); self.current_usage = NumBytes::from(new_usage); self.allocated_execution_memory += execution_bytes; + + self.add_execution_memory(execution_bytes, execution_memory_type)?; + + sandbox_safe_system_state.update_on_low_wasm_memory_hook_status(None, self.stable_memory_usage, self.wasm_memory_usage); + Ok(()) } - Err(_err) => Err(HypervisorError::OutOfMemory), + Err(_err) => Err(HypervisorError::OutOfMemory), } } MemoryAllocation::Reserved(reserved_bytes) => { @@ -868,11 +893,41 @@ impl MemoryUsage { return Err(HypervisorError::OutOfMemory); } self.current_usage = NumBytes::from(new_usage); + self.add_execution_memory(execution_bytes, execution_memory_type)?; + + sandbox_safe_system_state.system_state_changes.on_low_wasm_memory_hook_status.update(Some(reserved_bytes), self.stable_memory_usage, self.wasm_memory_usage); Ok(()) } } } + fn add_execution_memory(&mut self, execution_bytes: NumBytes, execution_memory_type: ExecutionMemoryType) { + match execution_memory_type { + ExecutionMemoryType::WasmMemory => { + let (new_usage, overflow) = self + .wasm_memory_usage + .get() + .overflowing_add(execution_bytes.get()); + if overflow { + return Err(HypervisorError::OutOfMemory); + } + self.wasm_memory_usage = NumBytes::new(new_usage); + } + ExecutionMemoryType::StableMemory => { + let (new_usage, overflow) = self + .stable_memory_usage + .get() + .overflowing_add(execution_bytes.get()); + if overflow { + return Err(HypervisorError::OutOfMemory); + } + self.stable_memory_usage = NumBytes::new(new_usage); + } + } + + debug_assert_eq!(self.current_usage.get(), self.stable_memory_usage.get() + self.wasm_memory_usage.get()); + } + /// Tries to allocate the requested amount of message memory. /// /// Returns `Err(HypervisorError::OutOfMemory)` and leaves `self` unchanged @@ -1013,6 +1068,9 @@ impl SystemApiImpl { execution_parameters.canister_memory_limit, execution_parameters.wasm_memory_limit, canister_current_memory_usage, + stable_memory_usage: stable_memory.size.checked_mul(WASM_PAGE_SIZE_IN_BYTES as u64) + .map(NumBytes::new) + .ok_or(HypervisorError::OutOfMemory)?, canister_current_message_memory_usage, subnet_available_memory, execution_parameters.memory_allocation, @@ -2663,6 +2721,7 @@ impl SystemApi for SystemApiImpl { &self.api_type, &mut self.sandbox_safe_system_state, &self.execution_parameters.subnet_memory_saturation, + ExecutionMemoryType::WasmMemory ) { Ok(()) => Ok(()), Err(err @ HypervisorError::InsufficientCyclesInMemoryGrow { .. }) => { @@ -2714,6 +2773,7 @@ impl SystemApi for SystemApiImpl { &self.api_type, &mut self.sandbox_safe_system_state, &self.execution_parameters.subnet_memory_saturation, + ExecutionMemoryType::StableMemory, ) { Ok(()) => Ok(StableGrowOutcome::Success), Err(err @ HypervisorError::InsufficientCyclesInMemoryGrow { .. }) => { diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 1e9de687cbb..b7425febf6d 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -72,6 +72,7 @@ pub struct SystemStateChanges { requests: Vec, pub(super) new_global_timer: Option, canister_log: CanisterLog, + pub on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, } impl Default for SystemStateChanges { @@ -87,6 +88,7 @@ impl Default for SystemStateChanges { requests: vec![], new_global_timer: None, canister_log: Default::default(), + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus::ConditionNotSatisfied, } } } @@ -299,6 +301,8 @@ impl SystemStateChanges { self.validate_cycle_change(system_state.canister_id == CYCLES_MINTING_CANISTER_ID)?; self.apply_balance_changes(system_state); + system_state.on_low_wasm_memory_hook_status = self.on_low_wasm_memory_hook_status; + // Verify we don't accept more cycles than are available from call // context and update the call context balance. if let Some((context_id, call_context_balance_taken)) = self.call_context_balance_taken { @@ -1206,6 +1210,22 @@ impl SandboxSafeSystemState { Ok(()) } } + + pub fn update_on_low_wasm_memory_hook_status( + &mut self, + memory_allocation: Option, + used_stable_memory: NumBytes, + used_wasm_memory: NumBytes, + ) { + self.system_state_changes + .on_low_wasm_memory_hook_status + .update( + self.wasm_memory_threshold, + memory_allocation, + used_stable_memory, + used_wasm_memory, + ); + } } // Returns `true` if storage cycles need to be reserved for the given From 1d4d250f0bcb5f7384d6557210bd3ee5f5a61eaa Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Mon, 29 Jul 2024 15:27:20 +0000 Subject: [PATCH 14/51] . --- .../src/canister_state/system_state.rs | 2 +- rs/system_api/src/lib.rs | 98 ++++++++++--------- 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 900370dcd46..dc974edc6cc 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -382,7 +382,7 @@ impl OnLowWasmMemoryHookStatus { // If the canister has memory allocation, then it maximum allowed Wasm memory // can be calculated as min(memory_allocation - used_stable_memory, 4 GiB). - let wasm_capacity = if let Some(memory_allocation) = memory_allocation{ + let wasm_capacity = if let Some(memory_allocation) = memory_allocation { std::cmp::min(memory_allocation - used_stable_memory, max_wasm_capacity) } else { max_wasm_capacity diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 196804bde33..2023044fed1 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -1,9 +1,3 @@ -pub mod cycles_balance_change; -mod request_in_prep; -mod routing; -pub mod sandbox_safe_system_state; -mod stable_memory; - use ic_base_types::PrincipalIdBlobParseError; use ic_config::embedders::StableMemoryPageLimit; use ic_config::flag_status::FlagStatus; @@ -19,16 +13,16 @@ use ic_interfaces::execution_environment::{ use ic_logger::{error, ReplicaLogger}; use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ - canister_state::WASM_PAGE_SIZE_IN_BYTES, memory_required_to_push_request, Memory, NumWasmPages, + canister_state::WASM_PAGE_SIZE_IN_BYTES, Memory, memory_required_to_push_request, NumWasmPages, PageIndex, }; use ic_sys::PageBytes; use ic_types::{ - ingress::WasmResult, - messages::{CallContextId, RejectContext, Request, MAX_INTER_CANISTER_PAYLOAD_IN_BYTES}, - methods::{SystemMethod, WasmClosure}, - CanisterId, CanisterLog, CanisterTimer, ComputeAllocation, Cycles, MemoryAllocation, NumBytes, - NumInstructions, NumOsPages, PrincipalId, SubnetId, Time, MAX_STABLE_MEMORY_IN_BYTES, + CanisterId, + CanisterLog, + CanisterTimer, + ComputeAllocation, Cycles, ingress::WasmResult, MAX_STABLE_MEMORY_IN_BYTES, MemoryAllocation, messages::{CallContextId, MAX_INTER_CANISTER_PAYLOAD_IN_BYTES, RejectContext, Request}, methods::{SystemMethod, WasmClosure}, + NumBytes, NumInstructions, NumOsPages, PrincipalId, SubnetId, Time, }; use ic_utils::deterministic_operations::deterministic_copy_from_slice; use request_in_prep::{into_request, RequestInPrep}; @@ -40,6 +34,12 @@ use std::{ rc::Rc, }; +pub mod cycles_balance_change; +mod request_in_prep; +mod routing; +pub mod sandbox_safe_system_state; +mod stable_memory; + pub const MULTIPLIER_MAX_SIZE_LOCAL_SUBNET: u64 = 5; const MAX_NON_REPLICATED_QUERY_REPLY_SIZE: NumBytes = NumBytes::new(3 << 20); const CERTIFIED_DATA_MAX_LENGTH: usize = 32; @@ -590,9 +590,9 @@ impl ApiType { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - call_context_id, .. - }, + NonReplicatedQueryKind::Stateful { + call_context_id, .. + }, .. } | ApiType::ReplyCallback { @@ -686,7 +686,7 @@ impl std::fmt::Display for ApiType { #[derive(Debug, PartialEq, Eq)] enum ExecutionMemoryType { WasmMemory, - StableMemory + StableMemory, } /// A struct to gather the relevant fields that correspond to a canister's @@ -877,7 +877,7 @@ impl MemoryUsage { Ok(()) } - Err(_err) => Err(HypervisorError::OutOfMemory), + Err(_err) => Err(HypervisorError::OutOfMemory), } } MemoryAllocation::Reserved(reserved_bytes) => { @@ -1062,15 +1062,17 @@ impl SystemApiImpl { out_of_instructions_handler: Rc, log: ReplicaLogger, ) -> Self { + let stable_memory_usage = stable_memory.size.checked_mul(WASM_PAGE_SIZE_IN_BYTES as u64) + .map(NumBytes::new) + .ok_or(HypervisorError::OutOfMemory)?; + let memory_usage = MemoryUsage::new( log.clone(), sandbox_safe_system_state.canister_id, execution_parameters.canister_memory_limit, execution_parameters.wasm_memory_limit, canister_current_memory_usage, - stable_memory_usage: stable_memory.size.checked_mul(WASM_PAGE_SIZE_IN_BYTES as u64) - .map(NumBytes::new) - .ok_or(HypervisorError::OutOfMemory)?, + stable_memory_usage, canister_current_message_memory_usage, subnet_available_memory, execution_parameters.memory_allocation, @@ -1964,9 +1966,9 @@ impl SystemApi for SystemApiImpl { ResponseStatus::NotRepliedYet => { if size as u64 > max_reply_size.get() { let string = format!( - "ic0.msg_reject: application payload size ({}) cannot be larger than {}.", - size, max_reply_size - ); + "ic0.msg_reject: application payload size ({}) cannot be larger than {}.", + size, max_reply_size + ); return Err(UserContractViolation { error: string, suggestion: "".to_string(), @@ -2139,9 +2141,9 @@ impl SystemApi for SystemApiImpl { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - outgoing_request, .. - }, + NonReplicatedQueryKind::Stateful { + outgoing_request, .. + }, .. } | ApiType::SystemTask { @@ -2214,9 +2216,9 @@ impl SystemApi for SystemApiImpl { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - outgoing_request, .. - }, + NonReplicatedQueryKind::Stateful { + outgoing_request, .. + }, .. } | ApiType::SystemTask { @@ -2256,9 +2258,9 @@ impl SystemApi for SystemApiImpl { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - outgoing_request, .. - }, + NonReplicatedQueryKind::Stateful { + outgoing_request, .. + }, .. } | ApiType::SystemTask { @@ -2341,10 +2343,10 @@ impl SystemApi for SystemApiImpl { | ApiType::NonReplicatedQuery { time, query_kind: - NonReplicatedQueryKind::Stateful { - call_context_id, - outgoing_request, - }, + NonReplicatedQueryKind::Stateful { + call_context_id, + outgoing_request, + }, .. } => { let req_in_prep = @@ -2721,7 +2723,7 @@ impl SystemApi for SystemApiImpl { &self.api_type, &mut self.sandbox_safe_system_state, &self.execution_parameters.subnet_memory_saturation, - ExecutionMemoryType::WasmMemory + ExecutionMemoryType::WasmMemory, ) { Ok(()) => Ok(()), Err(err @ HypervisorError::InsufficientCyclesInMemoryGrow { .. }) => { @@ -2983,13 +2985,13 @@ impl SystemApi for SystemApiImpl { if overflow || upper_bound > data_certificate.len() { return Err(ToolchainContractViolation { error: format!( - "ic0_data_certificate_copy failed because offset + size is out \ + "ic0_data_certificate_copy failed because offset + size is out \ of bounds. Found offset = {} and size = {} while offset + size \ must be <= {}", - offset, - size, - data_certificate.len() - ), + offset, + size, + data_certificate.len() + ), }); } @@ -3249,9 +3251,9 @@ impl SystemApi for SystemApiImpl { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - outgoing_request, .. - }, + NonReplicatedQueryKind::Stateful { + outgoing_request, .. + }, .. } | ApiType::SystemTask { @@ -3263,13 +3265,13 @@ impl SystemApi for SystemApiImpl { | ApiType::RejectCallback { outgoing_request, .. } => match outgoing_request { - None => Err(HypervisorError::ToolchainContractViolation{ + None => Err(HypervisorError::ToolchainContractViolation { error: "ic0.call_with_best_effort_response called when no call is under construction." - .to_string(), + .to_string(), }), Some(request) => { if request.is_timeout_set() { - Err(HypervisorError::ToolchainContractViolation{ + Err(HypervisorError::ToolchainContractViolation { error: "ic0_call_with_best_effort_response failed because a timeout is already set.".to_string(), }) } else { From 68aa922cec65c3111ce0b93a17e93e387bb207e4 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 31 Jul 2024 06:24:02 +0000 Subject: [PATCH 15/51] Add on_low_wasm_memory_hook_status field to CanisterStateBits. --- .../v1/canister_state_bits.proto | 7 ++ .../gen/state/state.canister_state_bits.v1.rs | 31 ++++++++ .../src/canister_state/system_state.rs | 5 +- rs/state_layout/src/state_layout.rs | 29 +++---- rs/state_manager/src/checkpoint.rs | 10 ++- rs/system_api/src/lib.rs | 78 ++++++++++++------- 6 files changed, 114 insertions(+), 46 deletions(-) diff --git a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto index b5e38fd5cd9..7565a54d960 100644 --- a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto +++ b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto @@ -334,6 +334,12 @@ enum LongExecutionMode { LONG_EXECUTION_MODE_PRIORITIZED = 2; } +enum OnLowWasmMemoryHookStatus { + CONDITION_NOT_SATISFIED = 0; + READY = 1; + EXECUTED = 2; +} + message CanisterStateBits { reserved 1; reserved "controller"; @@ -419,4 +425,5 @@ message CanisterStateBits { int64 priority_credit = 48; LongExecutionMode long_execution_mode = 49; optional uint64 wasm_memory_threshold = 50; + optional OnLowWasmMemoryHookStatus on_low_wasm_memory_hook_status = 51; } diff --git a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs index e0ef8468bcf..1207586ca84 100644 --- a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs +++ b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs @@ -670,6 +670,8 @@ pub struct CanisterStateBits { pub long_execution_mode: i32, #[prost(uint64, optional, tag = "50")] pub wasm_memory_threshold: ::core::option::Option, + #[prost(enumeration = "OnLowWasmMemoryHookStatus", optional, tag = "51")] + pub on_low_wasm_memory_hook_status: ::core::option::Option, #[prost(oneof = "canister_state_bits::CanisterStatus", tags = "11, 12, 13")] pub canister_status: ::core::option::Option, } @@ -871,3 +873,32 @@ impl LongExecutionMode { } } } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)] +#[repr(i32)] +pub enum OnLowWasmMemoryHookStatus { + ConditionNotSatisfied = 0, + Ready = 1, + Executed = 2, +} +impl OnLowWasmMemoryHookStatus { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + OnLowWasmMemoryHookStatus::ConditionNotSatisfied => "CONDITION_NOT_SATISFIED", + OnLowWasmMemoryHookStatus::Ready => "READY", + OnLowWasmMemoryHookStatus::Executed => "EXECUTED", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "CONDITION_NOT_SATISFIED" => Some(Self::ConditionNotSatisfied), + "READY" => Some(Self::Ready), + "EXECUTED" => Some(Self::Executed), + _ => None, + } + } +} diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index dc974edc6cc..573f4c764e9 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -362,8 +362,9 @@ pub struct SystemState { } /// A wrapper around the different canister statuses. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Default, Clone, Debug, PartialEq, Eq)] pub enum OnLowWasmMemoryHookStatus { + #[default] ConditionNotSatisfied, Ready, Executed, @@ -804,6 +805,7 @@ impl SystemState { canister_log: CanisterLog, wasm_memory_limit: Option, next_snapshot_id: u64, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, ) -> Self { Self { controllers, @@ -831,6 +833,7 @@ impl SystemState { canister_log, wasm_memory_limit, next_snapshot_id, + on_low_wasm_memory_hook_status, } } diff --git a/rs/state_layout/src/state_layout.rs b/rs/state_layout/src/state_layout.rs index d04a847de18..31d94e959bf 100644 --- a/rs/state_layout/src/state_layout.rs +++ b/rs/state_layout/src/state_layout.rs @@ -1,13 +1,10 @@ -use crate::error::LayoutError; -use crate::utils::do_copy; - use ic_base_types::{NumBytes, NumSeconds}; use ic_config::flag_status::FlagStatus; -use ic_logger::{error, info, warn, ReplicaLogger}; +use ic_logger::{error, info, ReplicaLogger, warn}; use ic_management_canister_types::LogVisibility; use ic_metrics::{buckets::decimal_buckets, MetricsRegistry}; use ic_protobuf::{ - proxy::{try_from_option_field, ProxyDecodeError}, + proxy::{ProxyDecodeError, try_from_option_field}, state::{ canister_snapshot_bits::v1 as pb_canister_snapshot_bits, canister_state_bits::v1 as pb_canister_state_bits, ingress::v1 as pb_ingress, @@ -15,33 +12,36 @@ use ic_protobuf::{ }, }; use ic_replicated_state::{ + CallContextManager, canister_state::{ execution_state::{NextScheduledMethod, WasmMetadata}, - system_state::{wasm_chunk_store::WasmChunkStoreMetadata, CanisterHistory, CyclesUseCase}, + system_state::{CanisterHistory, CyclesUseCase, wasm_chunk_store::WasmChunkStoreMetadata}, }, - page_map::{Shard, StorageLayout}, - CallContextManager, CanisterStatus, ExecutionTask, ExportedFunctions, Global, NumWasmPages, + CanisterStatus, ExecutionTask, ExportedFunctions, Global, NumWasmPages, page_map::{Shard, StorageLayout}, }; use ic_sys::{fs::sync_path, mmap::ScopedMmap}; use ic_types::{ - batch::TotalQueryStats, nominal_cycles::NominalCycles, AccumulatedPriority, CanisterId, - CanisterLog, ComputeAllocation, Cycles, ExecutionRound, Height, LongExecutionMode, - MemoryAllocation, NumInstructions, PrincipalId, SnapshotId, Time, + AccumulatedPriority, batch::TotalQueryStats, CanisterId, CanisterLog, + ComputeAllocation, Cycles, ExecutionRound, Height, LongExecutionMode, MemoryAllocation, + nominal_cycles::NominalCycles, NumInstructions, PrincipalId, SnapshotId, Time, }; use ic_utils::thread::parallel_map; use ic_wasm_types::{CanisterModule, WasmHash}; use prometheus::{Histogram, IntCounterVec}; use std::collections::{BTreeMap, BTreeSet}; -use std::convert::{identity, From, TryFrom, TryInto}; +use std::convert::{From, identity, TryFrom, TryInto}; use std::ffi::OsStr; use std::fs::OpenOptions; use std::io::{Error, Write}; use std::marker::PhantomData; use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Instant; +use crate::error::LayoutError; +use crate::utils::do_copy; + #[cfg(test)] mod tests; @@ -172,6 +172,7 @@ pub struct CanisterStateBits { pub canister_log: CanisterLog, pub wasm_memory_limit: Option, pub next_snapshot_id: u64, + pub on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, } /// This struct contains bits of the `CanisterSnapshot` that are not already @@ -1946,6 +1947,7 @@ impl From for pb_canister_state_bits::CanisterStateBits { next_canister_log_record_idx: item.canister_log.next_idx(), wasm_memory_limit: item.wasm_memory_limit.map(|v| v.get()), next_snapshot_id: item.next_snapshot_id, + on_low_wasm_memory_hook_status: Some(item.on_low_wasm_memory_hook_status), } } } @@ -2103,6 +2105,7 @@ impl TryFrom for CanisterStateBits { ), wasm_memory_limit: value.wasm_memory_limit.map(NumBytes::from), next_snapshot_id: value.next_snapshot_id, + on_low_wasm_memory_hook_status: value.unrwap_or_default(), }) } } diff --git a/rs/state_manager/src/checkpoint.rs b/rs/state_manager/src/checkpoint.rs index f29c5b15cbe..d4475db5755 100644 --- a/rs/state_manager/src/checkpoint.rs +++ b/rs/state_manager/src/checkpoint.rs @@ -1,7 +1,3 @@ -use crate::{ - pagemaptypes_with_num_pages, CheckpointError, CheckpointMetrics, HasDowngrade, TipRequest, - CRITICAL_ERROR_CHECKPOINT_SOFT_INVARIANT_BROKEN, NUMBER_OF_CHECKPOINT_THREADS, -}; use crossbeam_channel::{unbounded, Sender}; use ic_base_types::{subnet_id_try_from_protobuf, CanisterId}; use ic_config::flag_status::FlagStatus; @@ -23,6 +19,11 @@ use std::convert::TryFrom; use std::sync::Arc; use std::time::{Duration, Instant}; +use crate::{ + pagemaptypes_with_num_pages, CheckpointError, CheckpointMetrics, HasDowngrade, TipRequest, + CRITICAL_ERROR_CHECKPOINT_SOFT_INVARIANT_BROKEN, NUMBER_OF_CHECKPOINT_THREADS, +}; + #[cfg(test)] mod tests; @@ -420,6 +421,7 @@ pub fn load_canister_state( canister_state_bits.canister_log, canister_state_bits.wasm_memory_limit, canister_state_bits.next_snapshot_id, + canister_state_bits.on_low_wasm_memory_hook_status, ); let canister_state = CanisterState { diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 2023044fed1..2c35d6d4560 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -13,16 +13,16 @@ use ic_interfaces::execution_environment::{ use ic_logger::{error, ReplicaLogger}; use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ - canister_state::WASM_PAGE_SIZE_IN_BYTES, Memory, memory_required_to_push_request, NumWasmPages, + canister_state::WASM_PAGE_SIZE_IN_BYTES, memory_required_to_push_request, Memory, NumWasmPages, PageIndex, }; use ic_sys::PageBytes; use ic_types::{ - CanisterId, - CanisterLog, - CanisterTimer, - ComputeAllocation, Cycles, ingress::WasmResult, MAX_STABLE_MEMORY_IN_BYTES, MemoryAllocation, messages::{CallContextId, MAX_INTER_CANISTER_PAYLOAD_IN_BYTES, RejectContext, Request}, methods::{SystemMethod, WasmClosure}, - NumBytes, NumInstructions, NumOsPages, PrincipalId, SubnetId, Time, + ingress::WasmResult, + messages::{CallContextId, RejectContext, Request, MAX_INTER_CANISTER_PAYLOAD_IN_BYTES}, + methods::{SystemMethod, WasmClosure}, + CanisterId, CanisterLog, CanisterTimer, ComputeAllocation, Cycles, MemoryAllocation, NumBytes, + NumInstructions, NumOsPages, PrincipalId, SubnetId, Time, MAX_STABLE_MEMORY_IN_BYTES, }; use ic_utils::deterministic_operations::deterministic_copy_from_slice; use request_in_prep::{into_request, RequestInPrep}; @@ -590,9 +590,9 @@ impl ApiType { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - call_context_id, .. - }, + NonReplicatedQueryKind::Stateful { + call_context_id, .. + }, .. } | ApiType::ReplyCallback { @@ -750,7 +750,9 @@ impl MemoryUsage { limit ); } - let (wasm_memory_usage, overflow) = current_usage.get().overflowing_sub(stable_memory_usage.get()); + let (wasm_memory_usage, overflow) = current_usage + .get() + .overflowing_sub(stable_memory_usage.get()); debug_assert!(!overflow); @@ -873,7 +875,11 @@ impl MemoryUsage { self.add_execution_memory(execution_bytes, execution_memory_type)?; - sandbox_safe_system_state.update_on_low_wasm_memory_hook_status(None, self.stable_memory_usage, self.wasm_memory_usage); + sandbox_safe_system_state.update_on_low_wasm_memory_hook_status( + None, + self.stable_memory_usage, + self.wasm_memory_usage, + ); Ok(()) } @@ -895,13 +901,24 @@ impl MemoryUsage { self.current_usage = NumBytes::from(new_usage); self.add_execution_memory(execution_bytes, execution_memory_type)?; - sandbox_safe_system_state.system_state_changes.on_low_wasm_memory_hook_status.update(Some(reserved_bytes), self.stable_memory_usage, self.wasm_memory_usage); + sandbox_safe_system_state + .system_state_changes + .on_low_wasm_memory_hook_status + .update( + Some(reserved_bytes), + self.stable_memory_usage, + self.wasm_memory_usage, + ); Ok(()) } } } - fn add_execution_memory(&mut self, execution_bytes: NumBytes, execution_memory_type: ExecutionMemoryType) { + fn add_execution_memory( + &mut self, + execution_bytes: NumBytes, + execution_memory_type: ExecutionMemoryType, + ) { match execution_memory_type { ExecutionMemoryType::WasmMemory => { let (new_usage, overflow) = self @@ -925,7 +942,10 @@ impl MemoryUsage { } } - debug_assert_eq!(self.current_usage.get(), self.stable_memory_usage.get() + self.wasm_memory_usage.get()); + debug_assert_eq!( + self.current_usage.get(), + self.stable_memory_usage.get() + self.wasm_memory_usage.get() + ); } /// Tries to allocate the requested amount of message memory. @@ -1062,7 +1082,9 @@ impl SystemApiImpl { out_of_instructions_handler: Rc, log: ReplicaLogger, ) -> Self { - let stable_memory_usage = stable_memory.size.checked_mul(WASM_PAGE_SIZE_IN_BYTES as u64) + let stable_memory_usage = stable_memory + .size + .checked_mul(WASM_PAGE_SIZE_IN_BYTES as u64) .map(NumBytes::new) .ok_or(HypervisorError::OutOfMemory)?; @@ -2141,9 +2163,9 @@ impl SystemApi for SystemApiImpl { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - outgoing_request, .. - }, + NonReplicatedQueryKind::Stateful { + outgoing_request, .. + }, .. } | ApiType::SystemTask { @@ -2216,9 +2238,9 @@ impl SystemApi for SystemApiImpl { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - outgoing_request, .. - }, + NonReplicatedQueryKind::Stateful { + outgoing_request, .. + }, .. } | ApiType::SystemTask { @@ -2258,9 +2280,9 @@ impl SystemApi for SystemApiImpl { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - outgoing_request, .. - }, + NonReplicatedQueryKind::Stateful { + outgoing_request, .. + }, .. } | ApiType::SystemTask { @@ -2343,10 +2365,10 @@ impl SystemApi for SystemApiImpl { | ApiType::NonReplicatedQuery { time, query_kind: - NonReplicatedQueryKind::Stateful { - call_context_id, - outgoing_request, - }, + NonReplicatedQueryKind::Stateful { + call_context_id, + outgoing_request, + }, .. } => { let req_in_prep = From ebe1ccb7a1238aef6d593c4b60023352e4a4e15b Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 31 Jul 2024 07:00:02 +0000 Subject: [PATCH 16/51] . --- .../src/canister_state/system_state.rs | 29 +++++++++++++++++-- rs/state_layout/src/state_layout.rs | 28 ++++++++++-------- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 573f4c764e9..06446c93f10 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -362,14 +362,39 @@ pub struct SystemState { } /// A wrapper around the different canister statuses. -#[derive(Default, Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum OnLowWasmMemoryHookStatus { - #[default] ConditionNotSatisfied, Ready, Executed, } +impl From<&OnLowWasmMemoryHookStatus> for pb::OnLowWasmMemoryHookStatus { + fn from(item: &OnLowWasmMemoryHookStatus) -> Self { + match *item { + OnLowWasmMemoryHookStatus::ConditionNotSatisfied => { + pb::OnLowWasmMemoryHookStatus::ConditionNotSatisfied + } + OnLowWasmMemoryHookStatus::Ready => pb::OnLowWasmMemoryHookStatus::Ready, + OnLowWasmMemoryHookStatus::Executed => pb::OnLowWasmMemoryHookStatus::Executed, + } + } +} + +impl TryFrom for OnLowWasmMemoryHookStatus { + type Error = ProxyDecodeError; + + fn try_from(value: pb::OnLowWasmMemoryHookStatus) -> Result { + match value { + pb::OnLowWasmMemoryHookStatus::ConditionNotSatisfied => { + Ok(OnLowWasmMemoryHookStatus::ConditionNotSatisfied) + } + pb::OnLowWasmMemoryHookStatus::Ready => Ok(OnLowWasmMemoryHookStatus::Ready), + pb::OnLowWasmMemoryHookStatus::Executed => Ok(OnLowWasmMemoryHookStatus::Executed), + } + } +} + impl OnLowWasmMemoryHookStatus { fn update( &mut self, diff --git a/rs/state_layout/src/state_layout.rs b/rs/state_layout/src/state_layout.rs index 31d94e959bf..068d3ce2502 100644 --- a/rs/state_layout/src/state_layout.rs +++ b/rs/state_layout/src/state_layout.rs @@ -1,10 +1,10 @@ use ic_base_types::{NumBytes, NumSeconds}; use ic_config::flag_status::FlagStatus; -use ic_logger::{error, info, ReplicaLogger, warn}; +use ic_logger::{error, info, warn, ReplicaLogger}; use ic_management_canister_types::LogVisibility; use ic_metrics::{buckets::decimal_buckets, MetricsRegistry}; use ic_protobuf::{ - proxy::{ProxyDecodeError, try_from_option_field}, + proxy::{try_from_option_field, ProxyDecodeError}, state::{ canister_snapshot_bits::v1 as pb_canister_snapshot_bits, canister_state_bits::v1 as pb_canister_state_bits, ingress::v1 as pb_ingress, @@ -12,31 +12,31 @@ use ic_protobuf::{ }, }; use ic_replicated_state::{ - CallContextManager, canister_state::{ execution_state::{NextScheduledMethod, WasmMetadata}, - system_state::{CanisterHistory, CyclesUseCase, wasm_chunk_store::WasmChunkStoreMetadata}, + system_state::{wasm_chunk_store::WasmChunkStoreMetadata, CanisterHistory, CyclesUseCase}, }, - CanisterStatus, ExecutionTask, ExportedFunctions, Global, NumWasmPages, page_map::{Shard, StorageLayout}, + page_map::{Shard, StorageLayout}, + CallContextManager, CanisterStatus, ExecutionTask, ExportedFunctions, Global, NumWasmPages, }; use ic_sys::{fs::sync_path, mmap::ScopedMmap}; use ic_types::{ - AccumulatedPriority, batch::TotalQueryStats, CanisterId, CanisterLog, - ComputeAllocation, Cycles, ExecutionRound, Height, LongExecutionMode, MemoryAllocation, - nominal_cycles::NominalCycles, NumInstructions, PrincipalId, SnapshotId, Time, + batch::TotalQueryStats, nominal_cycles::NominalCycles, AccumulatedPriority, CanisterId, + CanisterLog, ComputeAllocation, Cycles, ExecutionRound, Height, LongExecutionMode, + MemoryAllocation, NumInstructions, PrincipalId, SnapshotId, Time, }; use ic_utils::thread::parallel_map; use ic_wasm_types::{CanisterModule, WasmHash}; use prometheus::{Histogram, IntCounterVec}; use std::collections::{BTreeMap, BTreeSet}; -use std::convert::{From, identity, TryFrom, TryInto}; +use std::convert::{identity, From, TryFrom, TryInto}; use std::ffi::OsStr; use std::fs::OpenOptions; use std::io::{Error, Write}; use std::marker::PhantomData; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; use std::time::Instant; use crate::error::LayoutError; @@ -1947,7 +1947,7 @@ impl From for pb_canister_state_bits::CanisterStateBits { next_canister_log_record_idx: item.canister_log.next_idx(), wasm_memory_limit: item.wasm_memory_limit.map(|v| v.get()), next_snapshot_id: item.next_snapshot_id, - on_low_wasm_memory_hook_status: Some(item.on_low_wasm_memory_hook_status), + on_low_wasm_memory_hook_status: Some(item.on_low_wasm_memory_hook_status.into()), } } } @@ -2105,7 +2105,11 @@ impl TryFrom for CanisterStateBits { ), wasm_memory_limit: value.wasm_memory_limit.map(NumBytes::from), next_snapshot_id: value.next_snapshot_id, - on_low_wasm_memory_hook_status: value.unrwap_or_default(), + on_low_wasm_memory_hook_status: try_from_option_field( + value.on_low_wasm_memory_hook_status, + "CanisterStateBits::on_low_wasm_memory_hook_status", + ) + .unwrap_or_default(), }) } } From ae7e6ffafa9d9713e9a77c20805cd3d242cc74b6 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 6 Aug 2024 14:49:06 +0000 Subject: [PATCH 17/51] fix errors --- .../src/canister_manager.rs | 1 + .../v1/canister_state_bits.proto | 7 ++--- .../gen/state/state.canister_state_bits.v1.rs | 25 ++++++++++------- .../src/canister_state/system_state.rs | 27 +++++++++++++------ rs/state_layout/src/state_layout.rs | 20 +++++++++++--- 5 files changed, 57 insertions(+), 23 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 22d8e977943..1750d25165a 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -1416,6 +1416,7 @@ impl CanisterManager { cycles, self.config.default_freeze_threshold, Arc::clone(&self.fd_factory), + OnLowWasmMemoryHookStatus::ConditionsNotSatisfied, ); system_state.remove_cycles(creation_fee, CyclesUseCase::CanisterCreation); diff --git a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto index 7565a54d960..cf97a87ee7f 100644 --- a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto +++ b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto @@ -335,9 +335,10 @@ enum LongExecutionMode { } enum OnLowWasmMemoryHookStatus { - CONDITION_NOT_SATISFIED = 0; - READY = 1; - EXECUTED = 2; + ON_LOW_WASM_MEMORY_HOOK_STATUS_UNSPECIFIED = 0; + ON_LOW_WASM_MEMORY_HOOK_STATUS_CONDITION_NOT_SATISFIED = 1; + ON_LOW_WASM_MEMORY_HOOK_STATUS_READY = 2; + ON_LOW_WASM_MEMORY_HOOK_STATUS_EXECUTED = 3; } message CanisterStateBits { diff --git a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs index 1207586ca84..a84394f7200 100644 --- a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs +++ b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs @@ -876,9 +876,10 @@ impl LongExecutionMode { #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)] #[repr(i32)] pub enum OnLowWasmMemoryHookStatus { - ConditionNotSatisfied = 0, - Ready = 1, - Executed = 2, + Unspecified = 0, + ConditionNotSatisfied = 1, + Ready = 2, + Executed = 3, } impl OnLowWasmMemoryHookStatus { /// String value of the enum field names used in the ProtoBuf definition. @@ -887,17 +888,23 @@ impl OnLowWasmMemoryHookStatus { /// (if the ProtoBuf definition does not change) and safe for programmatic use. pub fn as_str_name(&self) -> &'static str { match self { - OnLowWasmMemoryHookStatus::ConditionNotSatisfied => "CONDITION_NOT_SATISFIED", - OnLowWasmMemoryHookStatus::Ready => "READY", - OnLowWasmMemoryHookStatus::Executed => "EXECUTED", + OnLowWasmMemoryHookStatus::Unspecified => "ON_LOW_WASM_MEMORY_HOOK_STATUS_UNSPECIFIED", + OnLowWasmMemoryHookStatus::ConditionNotSatisfied => { + "ON_LOW_WASM_MEMORY_HOOK_STATUS_CONDITION_NOT_SATISFIED" + } + OnLowWasmMemoryHookStatus::Ready => "ON_LOW_WASM_MEMORY_HOOK_STATUS_READY", + OnLowWasmMemoryHookStatus::Executed => "ON_LOW_WASM_MEMORY_HOOK_STATUS_EXECUTED", } } /// Creates an enum from field names used in the ProtoBuf definition. pub fn from_str_name(value: &str) -> ::core::option::Option { match value { - "CONDITION_NOT_SATISFIED" => Some(Self::ConditionNotSatisfied), - "READY" => Some(Self::Ready), - "EXECUTED" => Some(Self::Executed), + "ON_LOW_WASM_MEMORY_HOOK_STATUS_UNSPECIFIED" => Some(Self::Unspecified), + "ON_LOW_WASM_MEMORY_HOOK_STATUS_CONDITION_NOT_SATISFIED" => { + Some(Self::ConditionNotSatisfied) + } + "ON_LOW_WASM_MEMORY_HOOK_STATUS_READY" => Some(Self::Ready), + "ON_LOW_WASM_MEMORY_HOOK_STATUS_EXECUTED" => Some(Self::Executed), _ => None, } } diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 06446c93f10..6eccb0d48ea 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -362,8 +362,9 @@ pub struct SystemState { } /// A wrapper around the different canister statuses. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, Default)] pub enum OnLowWasmMemoryHookStatus { + #[default] ConditionNotSatisfied, Ready, Executed, @@ -386,6 +387,13 @@ impl TryFrom for OnLowWasmMemoryHookStatus { fn try_from(value: pb::OnLowWasmMemoryHookStatus) -> Result { match value { + pb::OnLowWasmMemoryHookStatus::Unspecified => Err(ProxyDecodeError::ValueOutOfRange { + typ: "OnLowWasmMemoryHookStatus", + err: format!( + "Unexpected value of status of on low wasm memory hook: {:?}", + value + ), + }), pb::OnLowWasmMemoryHookStatus::ConditionNotSatisfied => { Ok(OnLowWasmMemoryHookStatus::ConditionNotSatisfied) } @@ -396,7 +404,7 @@ impl TryFrom for OnLowWasmMemoryHookStatus { } impl OnLowWasmMemoryHookStatus { - fn update( + fn _update( &mut self, wasm_memory_threshold: NumBytes, memory_allocation: Option, @@ -418,18 +426,16 @@ impl OnLowWasmMemoryHookStatus { if left_wasm_space >= wasm_memory_threshold { *self = OnLowWasmMemoryHookStatus::ConditionNotSatisfied; - } else { - if *self != OnLowWasmMemoryHookStatus::Executed { - *self = OnLowWasmMemoryHookStatus::Ready; - } + } else if *self != OnLowWasmMemoryHookStatus::Executed { + *self = OnLowWasmMemoryHookStatus::Ready; } } - fn should_schedule_hook(&self) -> bool { + fn _should_schedule_hook(&self) -> bool { *self == OnLowWasmMemoryHookStatus::Ready } - fn execute_hook(&mut self) { + fn _execute_hook(&mut self) { *self = OnLowWasmMemoryHookStatus::Executed; } } @@ -760,6 +766,7 @@ impl SystemState { initial_cycles: Cycles, freeze_threshold: NumSeconds, fd_factory: Arc, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, ) -> Self { Self::new_internal( canister_id, @@ -768,6 +775,7 @@ impl SystemState { freeze_threshold, CanisterStatus::new_running(), WasmChunkStore::new(fd_factory), + on_low_wasm_memory_hook_status, ) } @@ -778,6 +786,7 @@ impl SystemState { freeze_threshold: NumSeconds, status: CanisterStatus, wasm_chunk_store: WasmChunkStore, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, ) -> Self { Self { canister_id, @@ -802,6 +811,7 @@ impl SystemState { canister_log: Default::default(), wasm_memory_limit: None, next_snapshot_id: 0, + on_low_wasm_memory_hook_status, } } @@ -924,6 +934,7 @@ impl SystemState { freeze_threshold, status, WasmChunkStore::new_for_testing(), + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ) } diff --git a/rs/state_layout/src/state_layout.rs b/rs/state_layout/src/state_layout.rs index 068d3ce2502..4c9e212c46e 100644 --- a/rs/state_layout/src/state_layout.rs +++ b/rs/state_layout/src/state_layout.rs @@ -14,7 +14,10 @@ use ic_protobuf::{ use ic_replicated_state::{ canister_state::{ execution_state::{NextScheduledMethod, WasmMetadata}, - system_state::{wasm_chunk_store::WasmChunkStoreMetadata, CanisterHistory, CyclesUseCase}, + system_state::{ + wasm_chunk_store::WasmChunkStoreMetadata, CanisterHistory, CyclesUseCase, + OnLowWasmMemoryHookStatus, + }, }, page_map::{Shard, StorageLayout}, CallContextManager, CanisterStatus, ExecutionTask, ExportedFunctions, Global, NumWasmPages, @@ -1947,7 +1950,12 @@ impl From for pb_canister_state_bits::CanisterStateBits { next_canister_log_record_idx: item.canister_log.next_idx(), wasm_memory_limit: item.wasm_memory_limit.map(|v| v.get()), next_snapshot_id: item.next_snapshot_id, - on_low_wasm_memory_hook_status: Some(item.on_low_wasm_memory_hook_status.into()), + on_low_wasm_memory_hook_status: Some( + pb_canister_state_bits::OnLowWasmMemoryHookStatus::from( + &item.on_low_wasm_memory_hook_status, + ) + .into(), + ), } } } @@ -2018,6 +2026,12 @@ impl TryFrom for CanisterStateBits { ); } + let on_low_wasm_memory_hook_status: Option< + pb_canister_state_bits::OnLowWasmMemoryHookStatus, + > = value.on_low_wasm_memory_hook_status.map(|v| { + pb_canister_state_bits::OnLowWasmMemoryHookStatus::try_from(v).unwrap_or_default() + }); + Ok(Self { controllers, last_full_execution_round: value.last_full_execution_round.into(), @@ -2106,7 +2120,7 @@ impl TryFrom for CanisterStateBits { wasm_memory_limit: value.wasm_memory_limit.map(NumBytes::from), next_snapshot_id: value.next_snapshot_id, on_low_wasm_memory_hook_status: try_from_option_field( - value.on_low_wasm_memory_hook_status, + on_low_wasm_memory_hook_status, "CanisterStateBits::on_low_wasm_memory_hook_status", ) .unwrap_or_default(), From bdc3be7e3d2dee68d47d8581ea4a08d6a4e607d7 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 6 Aug 2024 19:15:39 +0000 Subject: [PATCH 18/51] fix errors --- rs/system_api/src/lib.rs | 2 +- .../src/sandbox_safe_system_state.rs | 30 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 2c35d6d4560..aa9631b810d 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -918,7 +918,7 @@ impl MemoryUsage { &mut self, execution_bytes: NumBytes, execution_memory_type: ExecutionMemoryType, - ) { + ) -> Result<(), HypervisorError> { match execution_memory_type { ExecutionMemoryType::WasmMemory => { let (new_usage, overflow) = self diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index b7425febf6d..770f3cd2044 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -1210,22 +1210,22 @@ impl SandboxSafeSystemState { Ok(()) } } + } - pub fn update_on_low_wasm_memory_hook_status( - &mut self, - memory_allocation: Option, - used_stable_memory: NumBytes, - used_wasm_memory: NumBytes, - ) { - self.system_state_changes - .on_low_wasm_memory_hook_status - .update( - self.wasm_memory_threshold, - memory_allocation, - used_stable_memory, - used_wasm_memory, - ); - } + pub fn update_on_low_wasm_memory_hook_status( + &mut self, + memory_allocation: Option, + used_stable_memory: NumBytes, + used_wasm_memory: NumBytes, + ) { + self.system_state_changes + .on_low_wasm_memory_hook_status + .update( + self.wasm_memory_threshold, + memory_allocation, + used_stable_memory, + used_wasm_memory, + ); } // Returns `true` if storage cycles need to be reserved for the given From 71666468946724736e66a1f00adbb38669d19575 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 6 Aug 2024 19:36:46 +0000 Subject: [PATCH 19/51] fix some more errors --- rs/replicated_state/src/canister_state/system_state.rs | 4 ++-- rs/state_layout/src/state_layout/tests.rs | 7 +++++-- rs/state_manager/src/tip.rs | 4 ++++ rs/system_api/src/lib.rs | 2 ++ rs/system_api/src/sandbox_safe_system_state.rs | 7 +++++-- 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 6eccb0d48ea..8ccf1613a51 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -362,7 +362,7 @@ pub struct SystemState { } /// A wrapper around the different canister statuses. -#[derive(Clone, Debug, PartialEq, Eq, Default)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)] pub enum OnLowWasmMemoryHookStatus { #[default] ConditionNotSatisfied, @@ -404,7 +404,7 @@ impl TryFrom for OnLowWasmMemoryHookStatus { } impl OnLowWasmMemoryHookStatus { - fn _update( + pub fn update( &mut self, wasm_memory_threshold: NumBytes, memory_allocation: Option, diff --git a/rs/state_layout/src/state_layout/tests.rs b/rs/state_layout/src/state_layout/tests.rs index 4d4f1ab9234..6cac7ec3039 100644 --- a/rs/state_layout/src/state_layout/tests.rs +++ b/rs/state_layout/src/state_layout/tests.rs @@ -5,8 +5,10 @@ use ic_management_canister_types::{ LogVisibility, IC_00, }; use ic_replicated_state::{ - canister_state::system_state::CanisterHistory, - metadata_state::subnet_call_context_manager::InstallCodeCallId, page_map::Shard, NumWasmPages, + canister_state::system_state::{CanisterHistory, OnLowWasmMemoryHookStatus}, + metadata_state::subnet_call_context_manager::InstallCodeCallId, + page_map::Shard, + NumWasmPages, }; use ic_test_utilities_logger::with_test_replica_logger; use ic_test_utilities_tmpdir::tmpdir; @@ -58,6 +60,7 @@ fn default_canister_state_bits() -> CanisterStateBits { canister_log: Default::default(), wasm_memory_limit: None, next_snapshot_id: 0, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus::default(), } } diff --git a/rs/state_manager/src/tip.rs b/rs/state_manager/src/tip.rs index 13396322ed6..68ebbff8019 100644 --- a/rs/state_manager/src/tip.rs +++ b/rs/state_manager/src/tip.rs @@ -901,6 +901,10 @@ fn serialize_canister_to_tip( canister_log: canister_state.system_state.canister_log.clone(), wasm_memory_limit: canister_state.system_state.wasm_memory_limit, next_snapshot_id: canister_state.system_state.next_snapshot_id, + on_low_wasm_memory_hook_status: canister_state + .system_state + .on_low_wasm_memory_hook_status + .clone(), } .into(), )?; diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index aa9631b810d..17db0b6eb87 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -946,6 +946,8 @@ impl MemoryUsage { self.current_usage.get(), self.stable_memory_usage.get() + self.wasm_memory_usage.get() ); + + Ok(()) } /// Tries to allocate the requested amount of message memory. diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 770f3cd2044..fe56c53eceb 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -17,7 +17,10 @@ use ic_management_canister_types::{ use ic_nns_constants::CYCLES_MINTING_CANISTER_ID; use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ - canister_state::{system_state::CyclesUseCase, DEFAULT_QUEUE_CAPACITY}, + canister_state::{ + system_state::{CyclesUseCase, OnLowWasmMemoryHookStatus}, + DEFAULT_QUEUE_CAPACITY, + }, CallOrigin, CanisterStatus, NetworkTopology, SystemState, }; use ic_types::{ @@ -57,7 +60,7 @@ pub enum CallbackUpdate { } /// Tracks changes to the system state that the canister has requested. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct SystemStateChanges { pub(super) new_certified_data: Option>, pub(super) callback_updates: Vec, From 7963c80982527dc24f2bb31bcb6a544b24fa81a6 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 7 Aug 2024 08:50:24 +0000 Subject: [PATCH 20/51] . --- rs/system_api/src/lib.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 17db0b6eb87..8bca793fe34 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -901,14 +901,11 @@ impl MemoryUsage { self.current_usage = NumBytes::from(new_usage); self.add_execution_memory(execution_bytes, execution_memory_type)?; - sandbox_safe_system_state - .system_state_changes - .on_low_wasm_memory_hook_status - .update( - Some(reserved_bytes), - self.stable_memory_usage, - self.wasm_memory_usage, - ); + sandbox_safe_system_state.update_on_low_wasm_memory_hook_status( + Some(reserved_bytes), + self.stable_memory_usage, + self.wasm_memory_usage, + ); Ok(()) } } @@ -1086,9 +1083,10 @@ impl SystemApiImpl { ) -> Self { let stable_memory_usage = stable_memory .size - .checked_mul(WASM_PAGE_SIZE_IN_BYTES as u64) - .map(NumBytes::new) - .ok_or(HypervisorError::OutOfMemory)?; + .get() + .checked_mul(WASM_PAGE_SIZE_IN_BYTES.try_into().unwrap()) + .map(|v| NumBytes::new(v as u64)) + .expect("Stable memory size is larger than maximal expected."); let memory_usage = MemoryUsage::new( log.clone(), From de78992f13ee8885ef806246fc937cae205d5b8d Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 7 Aug 2024 08:52:09 +0000 Subject: [PATCH 21/51] . --- rs/system_api/src/lib.rs | 2 +- rs/system_api/src/sandbox_safe_system_state.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 8bca793fe34..9fef6f65138 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -1084,7 +1084,7 @@ impl SystemApiImpl { let stable_memory_usage = stable_memory .size .get() - .checked_mul(WASM_PAGE_SIZE_IN_BYTES.try_into().unwrap()) + .checked_mul(WASM_PAGE_SIZE_IN_BYTES) .map(|v| NumBytes::new(v as u64)) .expect("Stable memory size is larger than maximal expected."); diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index fe56c53eceb..d663020365b 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -60,7 +60,7 @@ pub enum CallbackUpdate { } /// Tracks changes to the system state that the canister has requested. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct SystemStateChanges { pub(super) new_certified_data: Option>, pub(super) callback_updates: Vec, From e0b1a2f569824b8d01b8671596b2c3de9cd6a40d Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 7 Aug 2024 08:56:55 +0000 Subject: [PATCH 22/51] . --- rs/execution_environment/src/canister_manager.rs | 5 ++--- rs/system_api/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 1750d25165a..7ccd3bb9de5 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -26,14 +26,13 @@ use ic_management_canister_types::{ }; use ic_registry_provisional_whitelist::ProvisionalWhitelist; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::canister_state::system_state::ReservationError; use ic_replicated_state::{ canister_snapshots::CanisterSnapshot, canister_state::{ execution_state::Memory, system_state::{ wasm_chunk_store::{self, WasmChunkStore}, - CyclesUseCase, + CyclesUseCase, OnLowWasmMemoryHookStatus, ReservationError, }, NextExecution, }, @@ -1416,7 +1415,7 @@ impl CanisterManager { cycles, self.config.default_freeze_threshold, Arc::clone(&self.fd_factory), - OnLowWasmMemoryHookStatus::ConditionsNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ); system_state.remove_cycles(creation_fee, CyclesUseCase::CanisterCreation); diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 9fef6f65138..170b4807c3b 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -1086,7 +1086,7 @@ impl SystemApiImpl { .get() .checked_mul(WASM_PAGE_SIZE_IN_BYTES) .map(|v| NumBytes::new(v as u64)) - .expect("Stable memory size is larger than maximal expected."); + .expect("Stable memory size is larger than maximal allowed."); let memory_usage = MemoryUsage::new( log.clone(), From f3a552b01f176e2ddc5d23455d7d4273a2ed526e Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 7 Aug 2024 15:27:46 +0000 Subject: [PATCH 23/51] . --- rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs index 2f3882bbb41..3cefdbb9e9d 100644 --- a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs +++ b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs @@ -10,6 +10,7 @@ use ic_cycles_account_manager::ResourceSaturation; use ic_interfaces::execution_environment::{ExecutionMode, SubnetAvailableMemory}; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::canister_state::system_state::OnLowWasmMemoryHookStatus; use ic_replicated_state::page_map::TestPageAllocatorFileDescriptorImpl; use ic_replicated_state::{Memory, NetworkTopology, SystemState}; use ic_system_api::{ @@ -51,6 +52,7 @@ fn test_wasmtime_system_api() { Cycles::zero(), NumSeconds::from(0), Arc::new(TestPageAllocatorFileDescriptorImpl), + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ); let api_type = ApiType::start(UNIX_EPOCH); let sandbox_safe_system_state = SandboxSafeSystemState::new( From dc7f4ffe0b4473c373b89c5e9feb49fc43050132 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Thu, 8 Aug 2024 08:40:33 +0000 Subject: [PATCH 24/51] . --- .../src/canister_state/system_state.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 8ccf1613a51..b573a937012 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -412,16 +412,25 @@ impl OnLowWasmMemoryHookStatus { used_wasm_memory: NumBytes, ) { // The maximum Wasm memory occupied by the canister is 4 GiB. - let max_wasm_capacity = NumBytes::new(4 * 1024 * 1024); + let max_wasm_capacity = NumBytes::new(4 * 1024 * 1024 * 1024); // If the canister has memory allocation, then it maximum allowed Wasm memory // can be calculated as min(memory_allocation - used_stable_memory, 4 GiB). let wasm_capacity = if let Some(memory_allocation) = memory_allocation { + debug_assert!( + used_stable_memory <= memory_allocation, + "Used stable memory is larger than memory allocation.", + ); std::cmp::min(memory_allocation - used_stable_memory, max_wasm_capacity) } else { max_wasm_capacity }; + debug_assert!( + used_wasm_memory <= wasm_capacity, + "Used wasm memory is larger than maximal possible.", + ); + let left_wasm_space = wasm_capacity - used_wasm_memory; if left_wasm_space >= wasm_memory_threshold { From 0d04856d9ce45dd15b490dd1467a0214e6fe59c1 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Thu, 8 Aug 2024 09:31:17 +0000 Subject: [PATCH 25/51] . --- rs/replicated_state/src/canister_state/system_state.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index b573a937012..cb7432549b6 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -419,7 +419,9 @@ impl OnLowWasmMemoryHookStatus { let wasm_capacity = if let Some(memory_allocation) = memory_allocation { debug_assert!( used_stable_memory <= memory_allocation, - "Used stable memory is larger than memory allocation.", + "Used stable memory: {:?}, is larger than memory allocation: {:?}.", + used_stable_memory, + memory_allocation ); std::cmp::min(memory_allocation - used_stable_memory, max_wasm_capacity) } else { @@ -428,7 +430,9 @@ impl OnLowWasmMemoryHookStatus { debug_assert!( used_wasm_memory <= wasm_capacity, - "Used wasm memory is larger than maximal possible.", + "Used wasm memory: {:?}, is larger than maximal possible: {:?}.", + used_wasm_memory, + wasm_capacity ); let left_wasm_space = wasm_capacity - used_wasm_memory; From cf11649e10ec18691067acf4b59932a0c54ba965 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 9 Aug 2024 09:14:42 +0000 Subject: [PATCH 26/51] . --- .../src/canister_state/system_state.rs | 15 ++++++++++----- rs/system_api/src/lib.rs | 17 ++++++++++++----- rs/system_api/src/sandbox_safe_system_state.rs | 2 ++ 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index cb7432549b6..d874381a92c 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -408,14 +408,19 @@ impl OnLowWasmMemoryHookStatus { &mut self, wasm_memory_threshold: NumBytes, memory_allocation: Option, + wasm_memory_limit: Option, used_stable_memory: NumBytes, used_wasm_memory: NumBytes, ) { - // The maximum Wasm memory occupied by the canister is 4 GiB. - let max_wasm_capacity = NumBytes::new(4 * 1024 * 1024 * 1024); + // If wasm memory limit is not set, the default is 4 GiB. + let wasm_memory_limit = if let Some(limit) = wasm_memory_limit { + limit + } else { + NumBytes::new(4 * 1024 * 1024 * 1024) + }; // If the canister has memory allocation, then it maximum allowed Wasm memory - // can be calculated as min(memory_allocation - used_stable_memory, 4 GiB). + // can be calculated as min(memory_allocation - used_stable_memory, wasm_memory_limit). let wasm_capacity = if let Some(memory_allocation) = memory_allocation { debug_assert!( used_stable_memory <= memory_allocation, @@ -423,9 +428,9 @@ impl OnLowWasmMemoryHookStatus { used_stable_memory, memory_allocation ); - std::cmp::min(memory_allocation - used_stable_memory, max_wasm_capacity) + std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) } else { - max_wasm_capacity + wasm_memory_limit }; debug_assert!( diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 170b4807c3b..b2db3c5c1c3 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -877,6 +877,7 @@ impl MemoryUsage { sandbox_safe_system_state.update_on_low_wasm_memory_hook_status( None, + self.wasm_memory_limit, self.stable_memory_usage, self.wasm_memory_usage, ); @@ -903,6 +904,7 @@ impl MemoryUsage { sandbox_safe_system_state.update_on_low_wasm_memory_hook_status( Some(reserved_bytes), + self.wasm_memory_limit, self.stable_memory_usage, self.wasm_memory_usage, ); @@ -918,11 +920,16 @@ impl MemoryUsage { ) -> Result<(), HypervisorError> { match execution_memory_type { ExecutionMemoryType::WasmMemory => { - let (new_usage, overflow) = self - .wasm_memory_usage - .get() - .overflowing_add(execution_bytes.get()); - if overflow { + let wasm_memory_limit = if let Some(wasm_memory_limit) = self.wasm_memory_limit { + wasm_memory_limit.get() + } else { + // If the wasm memory limit is not set, default is 4 GiB. + 4 * 1024 * 1024 * 1024 + }; + + let new_usage = self.wasm_memory_usage.get() + execution_bytes.get(); + + if new_usage > wasm_memory_limit { return Err(HypervisorError::OutOfMemory); } self.wasm_memory_usage = NumBytes::new(new_usage); diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index d663020365b..8486613bdd4 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -1218,6 +1218,7 @@ impl SandboxSafeSystemState { pub fn update_on_low_wasm_memory_hook_status( &mut self, memory_allocation: Option, + wasm_memory_limit: Option, used_stable_memory: NumBytes, used_wasm_memory: NumBytes, ) { @@ -1226,6 +1227,7 @@ impl SandboxSafeSystemState { .update( self.wasm_memory_threshold, memory_allocation, + wasm_memory_limit, used_stable_memory, used_wasm_memory, ); From 88a44fcaed72b6bd6114d94b1e348a24df2a879d Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 6 Sep 2024 10:09:37 +0000 Subject: [PATCH 27/51] fix wasm memory size missmatch --- rs/embedders/src/wasm_executor.rs | 1 + .../wasmtime_embedder_tests.rs | 1 + .../tests/wasmtime_random_memory_writes.rs | 1 + rs/system_api/src/lib.rs | 21 +++++++++---------- rs/system_api/tests/common/mod.rs | 1 + rs/system_api/tests/system_api.rs | 3 +++ rs/test_utilities/embedders/src/lib.rs | 1 + 7 files changed, 18 insertions(+), 11 deletions(-) diff --git a/rs/embedders/src/wasm_executor.rs b/rs/embedders/src/wasm_executor.rs index 9a74bf52ea7..10042329a7e 100644 --- a/rs/embedders/src/wasm_executor.rs +++ b/rs/embedders/src/wasm_executor.rs @@ -594,6 +594,7 @@ pub fn process( embedder.config().feature_flags.wasm_native_stable_memory, embedder.config().max_sum_exported_function_name_lengths, stable_memory.clone(), + wasm_memory.size, out_of_instructions_handler, logger, ); diff --git a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs index 3cefdbb9e9d..736984f3c47 100644 --- a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs +++ b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs @@ -93,6 +93,7 @@ fn test_wasmtime_system_api() { .wasm_native_stable_memory, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + Memory::new_for_testing().size, Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); diff --git a/rs/embedders/tests/wasmtime_random_memory_writes.rs b/rs/embedders/tests/wasmtime_random_memory_writes.rs index 60c69ea2532..4f9e65b52a4 100644 --- a/rs/embedders/tests/wasmtime_random_memory_writes.rs +++ b/rs/embedders/tests/wasmtime_random_memory_writes.rs @@ -110,6 +110,7 @@ fn test_api_for_update( .wasm_native_stable_memory, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + Memory::new_for_testing().size, Rc::new(DefaultOutOfInstructionsHandler::new(instruction_limit)), log, ) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index b2db3c5c1c3..719013f0ce8 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -733,6 +733,7 @@ impl MemoryUsage { wasm_memory_limit: Option, current_usage: NumBytes, stable_memory_usage: NumBytes, + wasm_memory_usage: NumBytes, current_message_usage: NumBytes, subnet_available_memory: SubnetAvailableMemory, memory_allocation: MemoryAllocation, @@ -750,18 +751,13 @@ impl MemoryUsage { limit ); } - let (wasm_memory_usage, overflow) = current_usage - .get() - .overflowing_sub(stable_memory_usage.get()); - - debug_assert!(!overflow); Self { limit, wasm_memory_limit, current_usage, stable_memory_usage, - wasm_memory_usage: NumBytes::new(wasm_memory_usage), + wasm_memory_usage, current_message_usage, subnet_available_memory, allocated_execution_memory: NumBytes::from(0), @@ -946,11 +942,6 @@ impl MemoryUsage { } } - debug_assert_eq!( - self.current_usage.get(), - self.stable_memory_usage.get() + self.wasm_memory_usage.get() - ); - Ok(()) } @@ -1085,6 +1076,7 @@ impl SystemApiImpl { wasm_native_stable_memory: FlagStatus, max_sum_exported_function_name_lengths: usize, stable_memory: Memory, + wasm_memory_size: NumWasmPages, out_of_instructions_handler: Rc, log: ReplicaLogger, ) -> Self { @@ -1095,6 +1087,12 @@ impl SystemApiImpl { .map(|v| NumBytes::new(v as u64)) .expect("Stable memory size is larger than maximal allowed."); + let wasm_memory_usage = wasm_memory_size + .get() + .checked_mul(WASM_PAGE_SIZE_IN_BYTES) + .map(|v| NumBytes::new(v as u64)) + .expect("Wasm memory size is larger than maximal allowed."); + let memory_usage = MemoryUsage::new( log.clone(), sandbox_safe_system_state.canister_id, @@ -1102,6 +1100,7 @@ impl SystemApiImpl { execution_parameters.wasm_memory_limit, canister_current_memory_usage, stable_memory_usage, + wasm_memory_usage, canister_current_message_memory_usage, subnet_available_memory, execution_parameters.memory_allocation, diff --git a/rs/system_api/tests/common/mod.rs b/rs/system_api/tests/common/mod.rs index 8a3b27fe978..d89b7cb46d1 100644 --- a/rs/system_api/tests/common/mod.rs +++ b/rs/system_api/tests/common/mod.rs @@ -207,6 +207,7 @@ pub fn get_system_api( .wasm_native_stable_memory, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + Memory::new_for_testing().size, Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ) diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 9776668d035..a72747cfb13 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -1345,6 +1345,7 @@ fn growing_wasm_memory_updates_subnet_available_memory() { .wasm_native_stable_memory, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + Memory::new_for_testing().size, Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); @@ -1417,6 +1418,7 @@ fn push_output_request_respects_memory_limits() { .wasm_native_stable_memory, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + Memory::new_for_testing().size, Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); @@ -1528,6 +1530,7 @@ fn push_output_request_oversized_request_memory_limits() { .wasm_native_stable_memory, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + Memory::new_for_testing().size, Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); diff --git a/rs/test_utilities/embedders/src/lib.rs b/rs/test_utilities/embedders/src/lib.rs index d5fec3b3a15..7c4f4752be3 100644 --- a/rs/test_utilities/embedders/src/lib.rs +++ b/rs/test_utilities/embedders/src/lib.rs @@ -166,6 +166,7 @@ impl WasmtimeInstanceBuilder { embedder.config().feature_flags.wasm_native_stable_memory, embedder.config().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + Memory::new_for_testing().size, Rc::new(ic_system_api::DefaultOutOfInstructionsHandler::new( self.num_instructions, )), From 97e9238179b38b0530ef801b36d8fbcf72c6160a Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Mon, 9 Sep 2024 09:21:48 +0000 Subject: [PATCH 28/51] . --- .../state/canister_state_bits/v1/canister_state_bits.proto | 2 +- rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs | 2 +- rs/replicated_state/src/canister_state/system_state.rs | 1 - rs/state_manager/src/checkpoint.rs | 6 +----- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto index 6920e69427f..82b21728442 100644 --- a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto +++ b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto @@ -436,5 +436,5 @@ message CanisterStateBits { int64 priority_credit = 48; LongExecutionMode long_execution_mode = 49; optional uint64 wasm_memory_threshold = 50; - optional OnLowWasmMemoryHookStatus on_low_wasm_memory_hook_status = 51; + optional OnLowWasmMemoryHookStatus on_low_wasm_memory_hook_status = 53; } diff --git a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs index cf5a888543a..16f873c641b 100644 --- a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs +++ b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs @@ -698,7 +698,7 @@ pub struct CanisterStateBits { pub long_execution_mode: i32, #[prost(uint64, optional, tag = "50")] pub wasm_memory_threshold: ::core::option::Option, - #[prost(enumeration = "OnLowWasmMemoryHookStatus", optional, tag = "51")] + #[prost(enumeration = "OnLowWasmMemoryHookStatus", optional, tag = "53")] pub on_low_wasm_memory_hook_status: ::core::option::Option, #[prost(oneof = "canister_state_bits::CanisterStatus", tags = "11, 12, 13")] pub canister_status: ::core::option::Option, diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index c77bf262848..7125095553a 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -366,7 +366,6 @@ pub struct SystemState { /// Next local snapshot id. pub next_snapshot_id: u64, - /// Cumulative memory usage of all snapshots that belong to this canister. /// /// This amount contributes to the total `memory_usage` of the canister as diff --git a/rs/state_manager/src/checkpoint.rs b/rs/state_manager/src/checkpoint.rs index a4bd000e6c5..eaba3196ce1 100644 --- a/rs/state_manager/src/checkpoint.rs +++ b/rs/state_manager/src/checkpoint.rs @@ -1,7 +1,3 @@ -use crate::{ - CheckpointError, CheckpointMetrics, HasDowngrade, PageMapType, TipRequest, - CRITICAL_ERROR_CHECKPOINT_SOFT_INVARIANT_BROKEN, NUMBER_OF_CHECKPOINT_THREADS, -}; use crossbeam_channel::{unbounded, Sender}; use ic_base_types::{subnet_id_try_from_protobuf, CanisterId, SnapshotId}; use ic_config::flag_status::FlagStatus; @@ -30,7 +26,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use crate::{ - pagemaptypes_with_num_pages, CheckpointError, CheckpointMetrics, HasDowngrade, TipRequest, + CheckpointError, CheckpointMetrics, HasDowngrade, PageMapType, TipRequest, CRITICAL_ERROR_CHECKPOINT_SOFT_INVARIANT_BROKEN, NUMBER_OF_CHECKPOINT_THREADS, }; From e79e24c84a381ad4b496e064691821b9b50deeb4 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 10 Sep 2024 07:58:17 +0000 Subject: [PATCH 29/51] fix execution test --- .../src/canister_state/system_state.rs | 22 +++++++++---------- rs/system_api/src/lib.rs | 15 +++++-------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 7125095553a..fbd8fcd32e6 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -427,7 +427,9 @@ impl OnLowWasmMemoryHookStatus { used_stable_memory: NumBytes, used_wasm_memory: NumBytes, ) { - // If wasm memory limit is not set, the default is 4 GiB. + // If wasm memory limit is not set, the default is 4 GiB. Wasm memory + // limit is ignored for query methods, response callback handlers, + // global timers, heartbeats, and canister pre_upgrade. let wasm_memory_limit = if let Some(limit) = wasm_memory_limit { limit } else { @@ -448,16 +450,14 @@ impl OnLowWasmMemoryHookStatus { wasm_memory_limit }; - debug_assert!( - used_wasm_memory <= wasm_capacity, - "Used wasm memory: {:?}, is larger than maximal possible: {:?}.", - used_wasm_memory, - wasm_capacity - ); - - let left_wasm_space = wasm_capacity - used_wasm_memory; - - if left_wasm_space >= wasm_memory_threshold { + // Conceptually we can think that the remaining Wasm memory is + // equal to `wasm_capacity - used_wasm_memory` and that should + // be compared with `wasm_memory_threshold` when checking for + // the condition for the hook. However, since `wasm_memory_limit` + // is ignored in some executions as stated above it is possible + // that `used_wasm_memory` is greater than `wasm_capacity` to + // avoid overflowing subtraction we adopted inequality. + if wasm_capacity >= used_wasm_memory + wasm_memory_threshold { *self = OnLowWasmMemoryHookStatus::ConditionNotSatisfied; } else if *self != OnLowWasmMemoryHookStatus::Executed { *self = OnLowWasmMemoryHookStatus::Ready; diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 223f2a552f9..c3cbbe5feca 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -917,18 +917,15 @@ impl MemoryUsage { ) -> Result<(), HypervisorError> { match execution_memory_type { ExecutionMemoryType::WasmMemory => { - let wasm_memory_limit = if let Some(wasm_memory_limit) = self.wasm_memory_limit { - wasm_memory_limit.get() - } else { - // If the wasm memory limit is not set, default is 4 GiB. - 4 * 1024 * 1024 * 1024 - }; - - let new_usage = self.wasm_memory_usage.get() + execution_bytes.get(); + let (new_usage, overflow) = self + .wasm_memory_usage + .get() + .overflowing_add(execution_bytes.get()); - if new_usage > wasm_memory_limit { + if overflow { return Err(HypervisorError::OutOfMemory); } + self.wasm_memory_usage = NumBytes::new(new_usage); } ExecutionMemoryType::StableMemory => { From 2c084af263d09e4d08f894df1e00d43055353655 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 11 Sep 2024 11:54:39 +0000 Subject: [PATCH 30/51] make field private --- rs/replicated_state/src/canister_state/system_state.rs | 10 +++++++++- rs/state_manager/src/tip.rs | 2 +- rs/system_api/src/sandbox_safe_system_state.rs | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index fbd8fcd32e6..28c13ad61bc 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -373,7 +373,7 @@ pub struct SystemState { pub snapshots_memory_usage: NumBytes, /// Status of low_on_wasm_memory hook execution. - pub on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, } /// A wrapper around the different canister statuses. @@ -1661,6 +1661,14 @@ impl SystemState { _ => None, } } + + pub fn set_on_low_wasm_memory_hook_status(&mut self, status: OnLowWasmMemoryHookStatus) { + self.on_low_wasm_memory_hook_status = status; + } + + pub fn get_on_low_wasm_memory_hook_status(&self) -> &OnLowWasmMemoryHookStatus { + &self.on_low_wasm_memory_hook_status + } } /// Implements memory limits verification for pushing a canister-to-canister diff --git a/rs/state_manager/src/tip.rs b/rs/state_manager/src/tip.rs index c1d59ddbe8f..392458eb319 100644 --- a/rs/state_manager/src/tip.rs +++ b/rs/state_manager/src/tip.rs @@ -1079,7 +1079,7 @@ fn serialize_canister_to_tip( snapshots_memory_usage: canister_state.system_state.snapshots_memory_usage, on_low_wasm_memory_hook_status: canister_state .system_state - .on_low_wasm_memory_hook_status + .get_on_low_wasm_memory_hook_status() .clone(), } .into(), diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 8979bba4e2b..d20aa72aada 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -306,7 +306,7 @@ impl SystemStateChanges { self.validate_cycle_change(system_state.canister_id == CYCLES_MINTING_CANISTER_ID)?; self.apply_balance_changes(system_state); - system_state.on_low_wasm_memory_hook_status = self.on_low_wasm_memory_hook_status; + system_state.set_on_low_wasm_memory_hook_status(self.on_low_wasm_memory_hook_status); // Verify we don't accept more cycles than are available from call // context and update the call context balance. From fa653f5ca5a091346dbe659bf7c52c151ab0db1d Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 11 Sep 2024 12:50:11 +0000 Subject: [PATCH 31/51] remove unused method --- rs/replicated_state/src/canister_state/system_state.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 28c13ad61bc..5e086c6bb1d 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -463,14 +463,6 @@ impl OnLowWasmMemoryHookStatus { *self = OnLowWasmMemoryHookStatus::Ready; } } - - fn _should_schedule_hook(&self) -> bool { - *self == OnLowWasmMemoryHookStatus::Ready - } - - fn _execute_hook(&mut self) { - *self = OnLowWasmMemoryHookStatus::Executed; - } } /// A wrapper around the different canister statuses. From 049231579e92c4eba7b5ae5e20cc722558a2d783 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Wed, 11 Sep 2024 13:48:34 +0000 Subject: [PATCH 32/51] Add unit test for OnLowWasmMemoryHookStatus::update(). --- .../src/canister_state/tests.rs | 91 ++++++++++++++++++- 1 file changed, 90 insertions(+), 1 deletion(-) diff --git a/rs/replicated_state/src/canister_state/tests.rs b/rs/replicated_state/src/canister_state/tests.rs index b53018a6553..6cd3afeec97 100644 --- a/rs/replicated_state/src/canister_state/tests.rs +++ b/rs/replicated_state/src/canister_state/tests.rs @@ -6,7 +6,7 @@ use crate::canister_state::execution_state::CustomSection; use crate::canister_state::execution_state::CustomSectionType; use crate::canister_state::execution_state::WasmMetadata; use crate::canister_state::system_state::{ - CallContextManager, CanisterHistory, CanisterStatus, CyclesUseCase, + CallContextManager, CanisterHistory, CanisterStatus, CyclesUseCase, OnLowWasmMemoryHookStatus, MAX_CANISTER_HISTORY_CHANGES, }; use crate::metadata_state::subnet_call_context_manager::InstallCodeCallId; @@ -988,3 +988,92 @@ fn reverts_stopping_status_after_split() { assert_eq!(expected_state, canister_state); } + +const GIB: u64 = 1 << 30; + +fn helper_is_condition_satisfied_for_on_low_wasm_memory_hook( + wasm_memory_threshold: u64, + memory_allocation: Option, + wasm_memory_limit: Option, + used_stable_memory: u64, + used_wasm_memory: u64, +) -> bool { + let wasm_memory_limit = if let Some(wasm_memory_limit) = wasm_memory_limit { + wasm_memory_limit + } else { + 4 * GIB + }; + + let wasm_capacity = if let Some(memory_allocation) = memory_allocation { + std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) + } else { + wasm_memory_limit + }; + + wasm_capacity < used_wasm_memory + wasm_memory_threshold +} + +fn helper_test_on_low_wasm_memory_hook( + start_status: OnLowWasmMemoryHookStatus, + status_if_condition_satisfied: OnLowWasmMemoryHookStatus, +) { + for wasm_memory_threshold in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { + for memory_allocation in [None, Some(GIB), Some(2 * GIB), Some(3 * GIB), Some(4 * GIB)] { + for wasm_memory_limit in [None, Some(GIB), Some(2 * GIB), Some(3 * GIB), Some(4 * GIB)] + { + for used_stable_memory in [0, GIB] { + for used_wasm_memory in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { + let mut hook_status = start_status.clone(); + + hook_status.update( + wasm_memory_threshold.into(), + memory_allocation.map(|m| m.into()), + wasm_memory_limit.map(|m| m.into()), + used_stable_memory.into(), + used_wasm_memory.into(), + ); + + assert_eq!( + hook_status, + if helper_is_condition_satisfied_for_on_low_wasm_memory_hook( + wasm_memory_threshold, + memory_allocation, + wasm_memory_limit, + used_stable_memory, + used_wasm_memory + ) { + status_if_condition_satisfied.clone() + } else { + OnLowWasmMemoryHookStatus::ConditionNotSatisfied + } + ); + } + } + } + } + } +} + +#[test] +fn test_on_low_wasm_memory_hook_start_status_condition_not_satisfied() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); +} + +#[test] +fn test_on_low_wasm_memory_hook_start_status_ready() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::Ready, + OnLowWasmMemoryHookStatus::Ready, + ); +} + +#[test] +fn test_on_low_wasm_memory_hook_start_status_executed() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::Executed, + OnLowWasmMemoryHookStatus::Executed, + ); +} From f5fe0fe563b082f57bfb30a2c202bc72e10f4ad0 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Thu, 12 Sep 2024 11:09:24 +0000 Subject: [PATCH 33/51] . --- rs/canister_sandbox/src/sandbox_server.rs | 6 ++++- .../src/sandbox_safe_system_state.rs | 9 ++++++- rs/test_utilities/state/src/lib.rs | 24 +++++++++++++++---- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/rs/canister_sandbox/src/sandbox_server.rs b/rs/canister_sandbox/src/sandbox_server.rs index 76738376dcd..a5fc7adbbac 100644 --- a/rs/canister_sandbox/src/sandbox_server.rs +++ b/rs/canister_sandbox/src/sandbox_server.rs @@ -153,7 +153,10 @@ mod tests { use ic_limits::SMALL_APP_SUBNET_MAX_SIZE; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; - use ic_replicated_state::{Global, NumWasmPages, PageIndex, PageMap}; + use ic_replicated_state::{ + canister_state::system_state::OnLowWasmMemoryHookStatus, Global, NumWasmPages, PageIndex, + PageMap, + }; use ic_system_api::{ sandbox_safe_system_state::{CanisterStatusView, SandboxSafeSystemState}, ApiType, ExecutionParameters, InstructionLimits, @@ -228,6 +231,7 @@ mod tests { RequestMetadata::new(0, Time::from_nanos_since_unix_epoch(0)), caller, 0, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ) } diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index d20aa72aada..ee9f816059a 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -617,6 +617,7 @@ impl SandboxSafeSystemState { request_metadata: RequestMetadata, caller: Option, next_canister_log_record_idx: u64, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, ) -> Self { Self { canister_id, @@ -633,6 +634,7 @@ impl SandboxSafeSystemState { canister_log: CanisterLog::new_with_next_index(next_canister_log_record_idx), call_context_balance_taken: call_context_id .map(|call_context_id| (call_context_id, Cycles::zero())), + on_low_wasm_memory_hook_status, ..SystemStateChanges::default() }, initial_cycles_balance, @@ -734,6 +736,7 @@ impl SandboxSafeSystemState { request_metadata, caller, system_state.canister_log.next_idx(), + system_state.get_on_low_wasm_memory_hook_status().clone(), ) } @@ -1306,7 +1309,10 @@ mod tests { use ic_cycles_account_manager::CyclesAccountManager; use ic_limits::SMALL_APP_SUBNET_MAX_SIZE; use ic_registry_subnet_type::SubnetType; - use ic_replicated_state::{canister_state::system_state::CyclesUseCase, SystemState}; + use ic_replicated_state::{ + canister_state::system_state::{CyclesUseCase, OnLowWasmMemoryHookStatus}, + SystemState, + }; use ic_test_utilities_types::ids::{canister_test_id, subnet_test_id, user_test_id}; use ic_types::{ messages::{RequestMetadata, NO_DEADLINE}, @@ -1416,6 +1422,7 @@ mod tests { RequestMetadata::new(0, Time::from_nanos_since_unix_epoch(0)), None, 0, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ); sandbox_state.msg_deadline() } diff --git a/rs/test_utilities/state/src/lib.rs b/rs/test_utilities/state/src/lib.rs index 54412cedfba..36dc0c766ff 100644 --- a/rs/test_utilities/state/src/lib.rs +++ b/rs/test_utilities/state/src/lib.rs @@ -12,13 +12,15 @@ use ic_replicated_state::{ execution_state::{ CustomSection, CustomSectionType, NextScheduledMethod, WasmBinary, WasmMetadata, }, - system_state::CyclesUseCase, + system_state::{CyclesUseCase, OnLowWasmMemoryHookStatus}, testing::new_canister_output_queues_for_test, }, - metadata_state::subnet_call_context_manager::{ - BitcoinGetSuccessorsContext, BitcoinSendTransactionInternalContext, SubnetCallContext, + metadata_state::{ + subnet_call_context_manager::{ + BitcoinGetSuccessorsContext, BitcoinSendTransactionInternalContext, SubnetCallContext, + }, + Stream, SubnetMetrics, }, - metadata_state::{Stream, SubnetMetrics}, page_map::PageMap, testing::{CanisterQueuesTesting, ReplicatedStateTesting, SystemStateTesting}, CallContext, CallOrigin, CanisterState, CanisterStatus, ExecutionState, ExportedFunctions, @@ -482,6 +484,20 @@ impl SystemStateBuilder { self } + pub fn wasm_memory_limit(mut self, wasm_memory_limit: Option) -> Self { + self.system_state.wasm_memory_limit = wasm_memory_limit; + self + } + + pub fn on_low_wasm_memory_hook_status( + mut self, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, + ) -> Self { + self.system_state + .set_on_low_wasm_memory_hook_status(on_low_wasm_memory_hook_status); + self + } + pub fn freeze_threshold(mut self, threshold: NumSeconds) -> Self { self.system_state.freeze_threshold = threshold; self From cd149adc082fdd6bbd21105aa5bec2067494763a Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Thu, 12 Sep 2024 15:38:38 +0000 Subject: [PATCH 34/51] Add system_api tests --- rs/system_api/tests/system_api.rs | 249 +++++++++++++++++++++++++++++- 1 file changed, 246 insertions(+), 3 deletions(-) diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 7449a5dafcf..0a378a5a87b 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -1,4 +1,4 @@ -use ic_base_types::{NumSeconds, PrincipalIdBlobParseError}; +use ic_base_types::{NumBytes, NumSeconds, PrincipalIdBlobParseError}; use ic_config::{ embedders::Config as EmbeddersConfig, flag_status::FlagStatus, subnet_config::SchedulerConfig, }; @@ -6,13 +6,15 @@ use ic_cycles_account_manager::CyclesAccountManager; use ic_error_types::RejectCode; use ic_interfaces::execution_environment::{ CanisterOutOfCyclesError, ExecutionMode, HypervisorError, HypervisorResult, - PerformanceCounterType, SubnetAvailableMemory, SystemApi, SystemApiCallId, TrapCode, + PerformanceCounterType, StableMemoryApi, SubnetAvailableMemory, SystemApi, SystemApiCallId, + TrapCode, }; use ic_limits::SMALL_APP_SUBNET_MAX_SIZE; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ - testing::CanisterQueuesTesting, CallOrigin, Memory, NetworkTopology, SystemState, + canister_state::system_state::OnLowWasmMemoryHookStatus, testing::CanisterQueuesTesting, + CallOrigin, Memory, NetworkTopology, SystemState, }; use ic_system_api::{ sandbox_safe_system_state::SandboxSafeSystemState, ApiType, DefaultOutOfInstructionsHandler, @@ -1368,6 +1370,247 @@ fn growing_wasm_memory_updates_subnet_available_memory() { ); } +const GIB: i64 = 1 << 30; + +fn helper_test_on_low_wasm_memory( + wasm_memory_threshold: NumBytes, + wasm_memory_limit: Option, + memory_allocation: Option, + grow_memory_size: i64, + grow_wasm_memory: bool, + start_status: OnLowWasmMemoryHookStatus, + expected_status: OnLowWasmMemoryHookStatus, +) { + let wasm_page_size = 64 << 10; + let subnet_available_memory_bytes = 20 * GIB; + let subnet_available_memory = SubnetAvailableMemory::new(subnet_available_memory_bytes, 0, 0); + + let mut state_builder = SystemStateBuilder::default() + .wasm_memory_threshold(wasm_memory_threshold) + .wasm_memory_limit(wasm_memory_limit) + .on_low_wasm_memory_hook_status(start_status.clone()) + .initial_cycles(Cycles::from(10_000_000_000_000_000u128)); + + if let Some(memory_allocation) = memory_allocation { + state_builder = state_builder.memory_allocation(memory_allocation); + }; + + let mut system_state = state_builder.build(); + + let api_type = ApiTypeBuilder::build_update_api(); + let mut execution_parameters = execution_parameters(api_type.execution_mode()); + execution_parameters.memory_allocation = system_state.memory_allocation; + execution_parameters.wasm_memory_limit = system_state.wasm_memory_limit; + + let sandbox_safe_system_state = SandboxSafeSystemState::new( + &system_state, + CyclesAccountManagerBuilder::new().build(), + &NetworkTopology::default(), + SchedulerConfig::application_subnet().dirty_page_overhead, + execution_parameters.compute_allocation, + RequestMetadata::new(0, UNIX_EPOCH), + api_type.caller(), + api_type.call_context_id(), + ); + + let mut api = SystemApiImpl::new( + api_type, + sandbox_safe_system_state, + CANISTER_CURRENT_MEMORY_USAGE, + CANISTER_CURRENT_MESSAGE_MEMORY_USAGE, + execution_parameters, + subnet_available_memory, + EmbeddersConfig::default() + .feature_flags + .wasm_native_stable_memory, + EmbeddersConfig::default().feature_flags.canister_backtrace, + EmbeddersConfig::default().max_sum_exported_function_name_lengths, + Memory::new_for_testing(), + Memory::new_for_testing().size, + Rc::new(DefaultOutOfInstructionsHandler::default()), + no_op_logger(), + ); + + let additional_wasm_pages = (grow_memory_size as u64).div_ceil(wasm_page_size as u64); + + if grow_wasm_memory { + api.try_grow_wasm_memory(0, additional_wasm_pages).unwrap(); + } else { + api.try_grow_stable_memory(0, additional_wasm_pages, StableMemoryApi::Stable64) + .unwrap(); + } + + let system_state_changes = api.into_system_state_changes(); + system_state_changes + .apply_changes( + UNIX_EPOCH, + &mut system_state, + &default_network_topology(), + subnet_test_id(1), + &no_op_logger(), + ) + .unwrap(); + + assert_eq!( + *system_state.get_on_low_wasm_memory_hook_status(), + expected_status + ); +} + +#[test] +fn test_on_low_wasm_memory_grow_wasm_memory_all_status_changes() { + let wasm_memory_threshold = NumBytes::new(GIB as u64); + let wasm_memory_limit = Some(NumBytes::new(3 * GIB as u64)); + let memory_allocation = None; + // `max_allowed_wasm_memory` = `wasm_memory_limit` - `wasm_memory_threshold` + let max_allowed_wasm_memory = 2 * GIB; + let grow_wasm_memory = true; + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::Ready, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::Ready, + OnLowWasmMemoryHookStatus::Ready, + ); + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::Executed, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::Executed, + OnLowWasmMemoryHookStatus::Executed, + ); +} + +#[test] +fn test_on_low_wasm_memory_grow_stable_memory() { + // When memory_allocation is provided, hook condition can be triggered if: + // memory_allocation - used_stable_memory - used_wasm_memory < wasm_memory_threshold + // Hence growing stable memory can trigger hook condition. + let wasm_memory_threshold = NumBytes::new(GIB as u64); + let wasm_memory_limit = None; + let memory_allocation = Some(NumBytes::new(3 * GIB as u64)); + let max_allowed_memory_size = 2 * GIB; + let grow_wasm_memory = false; + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_memory_size, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_memory_size + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); + + // Without `memory_allocation`, hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + None, + max_allowed_memory_size + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); +} + +#[test] +fn test_on_low_wasm_memory_without_memory_limitn() { + // When memory limit is not set, the default Wasm memory limit is 4 GIB. + let wasm_memory_threshold = NumBytes::new(GIB as u64); + // `max_allowed_wasm_memory` = `wasm_memory_limit` - `wasm_memory_threshold` + let max_allowed_wasm_memory = 3 * GIB as i64; + let wasm_memory_limit = None; + let memory_allocation = None; + let grow_wasm_memory = true; + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); +} + #[test] fn push_output_request_respects_memory_limits() { let subnet_available_memory_bytes = 1 << 30; From 1ad2cb5feaacfe36cc69dd9d75e16f08b9f9ea3b Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Thu, 12 Sep 2024 15:46:05 +0000 Subject: [PATCH 35/51] Clippy. --- rs/system_api/tests/system_api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 0a378a5a87b..af886e3fbd5 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -1583,7 +1583,7 @@ fn test_on_low_wasm_memory_without_memory_limitn() { // When memory limit is not set, the default Wasm memory limit is 4 GIB. let wasm_memory_threshold = NumBytes::new(GIB as u64); // `max_allowed_wasm_memory` = `wasm_memory_limit` - `wasm_memory_threshold` - let max_allowed_wasm_memory = 3 * GIB as i64; + let max_allowed_wasm_memory = 3 * GIB; let wasm_memory_limit = None; let memory_allocation = None; let grow_wasm_memory = true; From d3b0b1e1491a9fcb42a5952bb4a2d573ec822190 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 20 Sep 2024 12:29:06 +0000 Subject: [PATCH 36/51] remove unused on_low_wasm_memory_initialization --- rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs | 1 - rs/execution_environment/src/canister_manager.rs | 1 - rs/replicated_state/src/canister_state/system_state.rs | 3 +-- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs index 91289ccaec9..80af35d2f89 100644 --- a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs +++ b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs @@ -52,7 +52,6 @@ fn test_wasmtime_system_api() { Cycles::zero(), NumSeconds::from(0), Arc::new(TestPageAllocatorFileDescriptorImpl), - OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ); let api_type = ApiType::start(UNIX_EPOCH); let sandbox_safe_system_state = SandboxSafeSystemState::new( diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index b99df0a7394..af213c36382 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -1429,7 +1429,6 @@ impl CanisterManager { cycles, self.config.default_freeze_threshold, Arc::clone(&self.fd_factory), - OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ); system_state.remove_cycles(creation_fee, CyclesUseCase::CanisterCreation); diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 5e086c6bb1d..7374051ac4c 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -791,7 +791,6 @@ impl SystemState { initial_cycles: Cycles, freeze_threshold: NumSeconds, fd_factory: Arc, - on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, ) -> Self { Self::new_internal( canister_id, @@ -800,7 +799,7 @@ impl SystemState { freeze_threshold, CanisterStatus::new_running(), WasmChunkStore::new(fd_factory), - on_low_wasm_memory_hook_status, + OnLowWasmMemoryHookStatus::default(), ) } From 1e04e213b857354dd636a7d800fa3ba8e31f9d72 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 20 Sep 2024 12:31:59 +0000 Subject: [PATCH 37/51] remove unused initialization --- rs/replicated_state/src/canister_state/system_state.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 7374051ac4c..48c21eb46bf 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -799,7 +799,6 @@ impl SystemState { freeze_threshold, CanisterStatus::new_running(), WasmChunkStore::new(fd_factory), - OnLowWasmMemoryHookStatus::default(), ) } @@ -810,7 +809,6 @@ impl SystemState { freeze_threshold: NumSeconds, status: CanisterStatus, wasm_chunk_store: WasmChunkStore, - on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, ) -> Self { Self { canister_id, @@ -836,7 +834,7 @@ impl SystemState { wasm_memory_limit: None, next_snapshot_id: 0, snapshots_memory_usage: NumBytes::from(0), - on_low_wasm_memory_hook_status, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus::default(), } } @@ -966,7 +964,6 @@ impl SystemState { freeze_threshold, status, WasmChunkStore::new_for_testing(), - OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ) } From 6f66fe20edf7adde10d5725a029fdf785126296f Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 20 Sep 2024 12:34:19 +0000 Subject: [PATCH 38/51] write derrive in canonical order --- rs/replicated_state/src/canister_state/system_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 48c21eb46bf..496f06594eb 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -377,7 +377,7 @@ pub struct SystemState { } /// A wrapper around the different canister statuses. -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)] +#[derive(Clone, Eq, PartialEq, Debug, Default, Deserialize, Serialize)] pub enum OnLowWasmMemoryHookStatus { #[default] ConditionNotSatisfied, From 9e1fb928af35cef589b8232f3d56dac0788c6ac9 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 20 Sep 2024 12:41:24 +0000 Subject: [PATCH 39/51] refactor --- .../src/canister_state/system_state.rs | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 496f06594eb..192e4ea90b7 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -430,25 +430,23 @@ impl OnLowWasmMemoryHookStatus { // If wasm memory limit is not set, the default is 4 GiB. Wasm memory // limit is ignored for query methods, response callback handlers, // global timers, heartbeats, and canister pre_upgrade. - let wasm_memory_limit = if let Some(limit) = wasm_memory_limit { - limit - } else { - NumBytes::new(4 * 1024 * 1024 * 1024) - }; + let wasm_memory_limit = + wasm_memory_limit.unwrap_or_else(|| NumBytes::new(4 * 1024 * 1024 * 1024)); // If the canister has memory allocation, then it maximum allowed Wasm memory // can be calculated as min(memory_allocation - used_stable_memory, wasm_memory_limit). - let wasm_capacity = if let Some(memory_allocation) = memory_allocation { - debug_assert!( - used_stable_memory <= memory_allocation, - "Used stable memory: {:?}, is larger than memory allocation: {:?}.", - used_stable_memory, - memory_allocation - ); - std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) - } else { - wasm_memory_limit - }; + let wasm_capacity = memory_allocation.map_or_else( + || wasm_memory_limit, + |memory_allocation| { + debug_assert!( + used_stable_memory <= memory_allocation, + "Used stable memory: {:?} is larger than memory allocation: {:?}.", + used_stable_memory, + memory_allocation + ); + std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) + }, + ); // Conceptually we can think that the remaining Wasm memory is // equal to `wasm_capacity - used_wasm_memory` and that should From b7261513194ac36fb269c77b75688f19deae1a45 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 20 Sep 2024 12:46:36 +0000 Subject: [PATCH 40/51] refactor --- rs/replicated_state/src/canister_state/tests.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rs/replicated_state/src/canister_state/tests.rs b/rs/replicated_state/src/canister_state/tests.rs index 6cd3afeec97..449f75be667 100644 --- a/rs/replicated_state/src/canister_state/tests.rs +++ b/rs/replicated_state/src/canister_state/tests.rs @@ -998,17 +998,11 @@ fn helper_is_condition_satisfied_for_on_low_wasm_memory_hook( used_stable_memory: u64, used_wasm_memory: u64, ) -> bool { - let wasm_memory_limit = if let Some(wasm_memory_limit) = wasm_memory_limit { - wasm_memory_limit - } else { - 4 * GIB - }; + let wasm_memory_limit = wasm_memory_limit.unwrap_or(4 * GIB); - let wasm_capacity = if let Some(memory_allocation) = memory_allocation { + let wasm_capacity = memory_allocation.map_or(wasm_memory_limit, |memory_allocation| { std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) - } else { - wasm_memory_limit - }; + }); wasm_capacity < used_wasm_memory + wasm_memory_threshold } From d122dde41131e8ea475e10e5585685265c3ca4fa Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 20 Sep 2024 13:49:00 +0000 Subject: [PATCH 41/51] Fix clippy error. --- rs/execution_environment/src/canister_manager.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index af213c36382..a96ff3dc5af 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -34,7 +34,7 @@ use ic_replicated_state::{ execution_state::Memory, system_state::{ wasm_chunk_store::{self, WasmChunkStore}, - CyclesUseCase, OnLowWasmMemoryHookStatus, ReservationError, + CyclesUseCase, ReservationError, }, NextExecution, }, From 8e337aff97333a8a14843dd4ddd787938562cdc4 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Fri, 20 Sep 2024 14:22:14 +0000 Subject: [PATCH 42/51] Fix clippy error. --- rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs index 80af35d2f89..d8aaa0339cf 100644 --- a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs +++ b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs @@ -10,7 +10,6 @@ use ic_cycles_account_manager::ResourceSaturation; use ic_interfaces::execution_environment::{ExecutionMode, SubnetAvailableMemory}; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::canister_state::system_state::OnLowWasmMemoryHookStatus; use ic_replicated_state::page_map::TestPageAllocatorFileDescriptorImpl; use ic_replicated_state::{Memory, NetworkTopology, SystemState}; use ic_system_api::{ From 0d51f6343dd274ab5ad165de01af87e66e5729dd Mon Sep 17 00:00:00 2001 From: Dragoljub Djuric Date: Tue, 24 Sep 2024 17:27:27 +0200 Subject: [PATCH 43/51] Update rs/replicated_state/src/canister_state/system_state.rs Co-authored-by: Alin Sinpalean <58422065+alin-at-dfinity@users.noreply.github.com> --- rs/replicated_state/src/canister_state/system_state.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index dfe7466b67b..025728a20dc 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -387,12 +387,12 @@ pub enum OnLowWasmMemoryHookStatus { impl From<&OnLowWasmMemoryHookStatus> for pb::OnLowWasmMemoryHookStatus { fn from(item: &OnLowWasmMemoryHookStatus) -> Self { + use OnLowWasmMemoryHookStatus::*; + match *item { - OnLowWasmMemoryHookStatus::ConditionNotSatisfied => { - pb::OnLowWasmMemoryHookStatus::ConditionNotSatisfied - } - OnLowWasmMemoryHookStatus::Ready => pb::OnLowWasmMemoryHookStatus::Ready, - OnLowWasmMemoryHookStatus::Executed => pb::OnLowWasmMemoryHookStatus::Executed, + ConditionNotSatisfied => Self::ConditionNotSatisfied, + Ready => Self::Ready, + Executed => Self::Executed, } } } From 6a5b2134a2625bc367db4ba281b7f78ccef0dcdb Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 24 Sep 2024 15:28:56 +0000 Subject: [PATCH 44/51] adress comments --- .../src/wasmtime_embedder/wasmtime_embedder_tests.rs | 2 +- rs/embedders/tests/wasmtime_random_memory_writes.rs | 2 +- rs/replicated_state/src/canister_state/system_state.rs | 2 +- rs/system_api/tests/common/mod.rs | 2 +- rs/system_api/tests/system_api.rs | 8 ++++---- rs/test_utilities/embedders/src/lib.rs | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs index d8aaa0339cf..23aa658572b 100644 --- a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs +++ b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs @@ -92,7 +92,7 @@ fn test_wasmtime_system_api() { EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), - Memory::new_for_testing().size, + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); diff --git a/rs/embedders/tests/wasmtime_random_memory_writes.rs b/rs/embedders/tests/wasmtime_random_memory_writes.rs index 2ee92bcd3c9..1eb9729ce1e 100644 --- a/rs/embedders/tests/wasmtime_random_memory_writes.rs +++ b/rs/embedders/tests/wasmtime_random_memory_writes.rs @@ -111,7 +111,7 @@ fn test_api_for_update( EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), - Memory::new_for_testing().size, + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::new(instruction_limit)), log, ) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 025728a20dc..57dbe22c6a0 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -376,7 +376,7 @@ pub struct SystemState { on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, } -/// A wrapper around the different canister statuses. +/// A wrapper around the different statuses of `OnLowWasmMemory` hook execution. #[derive(Clone, Eq, PartialEq, Debug, Default, Deserialize, Serialize)] pub enum OnLowWasmMemoryHookStatus { #[default] diff --git a/rs/system_api/tests/common/mod.rs b/rs/system_api/tests/common/mod.rs index 7e3839a0fcb..c144c1c7a06 100644 --- a/rs/system_api/tests/common/mod.rs +++ b/rs/system_api/tests/common/mod.rs @@ -208,7 +208,7 @@ pub fn get_system_api( EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), - Memory::new_for_testing().size, + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ) diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index af886e3fbd5..4f2fbf209c9 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -1348,7 +1348,7 @@ fn growing_wasm_memory_updates_subnet_available_memory() { EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), - Memory::new_for_testing().size, + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); @@ -1426,7 +1426,7 @@ fn helper_test_on_low_wasm_memory( EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), - Memory::new_for_testing().size, + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); @@ -1663,7 +1663,7 @@ fn push_output_request_respects_memory_limits() { EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), - Memory::new_for_testing().size, + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); @@ -1776,7 +1776,7 @@ fn push_output_request_oversized_request_memory_limits() { EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), - Memory::new_for_testing().size, + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); diff --git a/rs/test_utilities/embedders/src/lib.rs b/rs/test_utilities/embedders/src/lib.rs index 2d4071d308d..f68fd75c14a 100644 --- a/rs/test_utilities/embedders/src/lib.rs +++ b/rs/test_utilities/embedders/src/lib.rs @@ -167,7 +167,7 @@ impl WasmtimeInstanceBuilder { embedder.config().feature_flags.canister_backtrace, embedder.config().max_sum_exported_function_name_lengths, Memory::new_for_testing(), - Memory::new_for_testing().size, + NumWasmPages::from(0), Rc::new(ic_system_api::DefaultOutOfInstructionsHandler::new( self.num_instructions, )), From f19ce55ca6182e1450906db821fdd2d96431547d Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 24 Sep 2024 15:31:53 +0000 Subject: [PATCH 45/51] Fix clippy. --- rs/replicated_state/src/canister_state/system_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 57dbe22c6a0..c582faa6c06 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -388,7 +388,7 @@ pub enum OnLowWasmMemoryHookStatus { impl From<&OnLowWasmMemoryHookStatus> for pb::OnLowWasmMemoryHookStatus { fn from(item: &OnLowWasmMemoryHookStatus) -> Self { use OnLowWasmMemoryHookStatus::*; - + match *item { ConditionNotSatisfied => Self::ConditionNotSatisfied, Ready => Self::Ready, From 1c1138d944a1191482ce90cb36d8b1bd6afda9d5 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 24 Sep 2024 15:45:22 +0000 Subject: [PATCH 46/51] Adress comments/ --- rs/replicated_state/src/canister_state/system_state.rs | 6 +++--- rs/state_manager/src/tip.rs | 3 +-- rs/system_api/src/sandbox_safe_system_state.rs | 2 +- rs/system_api/tests/common/mod.rs | 1 + rs/system_api/tests/system_api.rs | 5 +++-- rs/test_utilities/embedders/src/lib.rs | 1 + 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index c582faa6c06..62a6eddc61d 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -377,7 +377,7 @@ pub struct SystemState { } /// A wrapper around the different statuses of `OnLowWasmMemory` hook execution. -#[derive(Clone, Eq, PartialEq, Debug, Default, Deserialize, Serialize)] +#[derive(Clone, Copy, Eq, PartialEq, Debug, Default, Deserialize, Serialize)] pub enum OnLowWasmMemoryHookStatus { #[default] ConditionNotSatisfied, @@ -1655,8 +1655,8 @@ impl SystemState { self.on_low_wasm_memory_hook_status = status; } - pub fn get_on_low_wasm_memory_hook_status(&self) -> &OnLowWasmMemoryHookStatus { - &self.on_low_wasm_memory_hook_status + pub fn get_on_low_wasm_memory_hook_status(&self) -> OnLowWasmMemoryHookStatus { + self.on_low_wasm_memory_hook_status } } diff --git a/rs/state_manager/src/tip.rs b/rs/state_manager/src/tip.rs index 392458eb319..5c9bf330384 100644 --- a/rs/state_manager/src/tip.rs +++ b/rs/state_manager/src/tip.rs @@ -1079,8 +1079,7 @@ fn serialize_canister_to_tip( snapshots_memory_usage: canister_state.system_state.snapshots_memory_usage, on_low_wasm_memory_hook_status: canister_state .system_state - .get_on_low_wasm_memory_hook_status() - .clone(), + .get_on_low_wasm_memory_hook_status(), } .into(), )?; diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index ee9f816059a..69bb3d42f15 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -736,7 +736,7 @@ impl SandboxSafeSystemState { request_metadata, caller, system_state.canister_log.next_idx(), - system_state.get_on_low_wasm_memory_hook_status().clone(), + system_state.get_on_low_wasm_memory_hook_status(), ) } diff --git a/rs/system_api/tests/common/mod.rs b/rs/system_api/tests/common/mod.rs index c144c1c7a06..24d7bcc3eac 100644 --- a/rs/system_api/tests/common/mod.rs +++ b/rs/system_api/tests/common/mod.rs @@ -11,6 +11,7 @@ use ic_management_canister_types::IC_00; use ic_nns_constants::CYCLES_MINTING_CANISTER_ID; use ic_registry_routing_table::{CanisterIdRange, RoutingTable}; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::NumWasmPages; use ic_replicated_state::{CallOrigin, Memory, NetworkTopology, SubnetTopology, SystemState}; use ic_system_api::{ sandbox_safe_system_state::SandboxSafeSystemState, ApiType, DefaultOutOfInstructionsHandler, diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 4f2fbf209c9..ffd9d809ffa 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -12,6 +12,7 @@ use ic_interfaces::execution_environment::{ use ic_limits::SMALL_APP_SUBNET_MAX_SIZE; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::NumWasmPages; use ic_replicated_state::{ canister_state::system_state::OnLowWasmMemoryHookStatus, testing::CanisterQueuesTesting, CallOrigin, Memory, NetworkTopology, SystemState, @@ -1388,7 +1389,7 @@ fn helper_test_on_low_wasm_memory( let mut state_builder = SystemStateBuilder::default() .wasm_memory_threshold(wasm_memory_threshold) .wasm_memory_limit(wasm_memory_limit) - .on_low_wasm_memory_hook_status(start_status.clone()) + .on_low_wasm_memory_hook_status(start_status) .initial_cycles(Cycles::from(10_000_000_000_000_000u128)); if let Some(memory_allocation) = memory_allocation { @@ -1452,7 +1453,7 @@ fn helper_test_on_low_wasm_memory( .unwrap(); assert_eq!( - *system_state.get_on_low_wasm_memory_hook_status(), + system_state.get_on_low_wasm_memory_hook_status(), expected_status ); } diff --git a/rs/test_utilities/embedders/src/lib.rs b/rs/test_utilities/embedders/src/lib.rs index f68fd75c14a..efbd4c704df 100644 --- a/rs/test_utilities/embedders/src/lib.rs +++ b/rs/test_utilities/embedders/src/lib.rs @@ -9,6 +9,7 @@ use ic_interfaces::execution_environment::{ }; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::NumWasmPages; use ic_replicated_state::{Global, Memory, NetworkTopology, PageMap}; use ic_system_api::{ sandbox_safe_system_state::SandboxSafeSystemState, ExecutionParameters, InstructionLimits, From b2fa264d4126ecbecbb7b35135caafa9b3c24134 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 24 Sep 2024 16:07:11 +0000 Subject: [PATCH 47/51] Refactor remainings. --- rs/replicated_state/src/canister_state/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/replicated_state/src/canister_state/tests.rs b/rs/replicated_state/src/canister_state/tests.rs index 2a87bcd2145..eefa90b3805 100644 --- a/rs/replicated_state/src/canister_state/tests.rs +++ b/rs/replicated_state/src/canister_state/tests.rs @@ -1129,7 +1129,7 @@ fn helper_test_on_low_wasm_memory_hook( { for used_stable_memory in [0, GIB] { for used_wasm_memory in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { - let mut hook_status = start_status.clone(); + let mut hook_status = start_status; hook_status.update( wasm_memory_threshold.into(), @@ -1148,7 +1148,7 @@ fn helper_test_on_low_wasm_memory_hook( used_stable_memory, used_wasm_memory ) { - status_if_condition_satisfied.clone() + status_if_condition_satisfied } else { OnLowWasmMemoryHookStatus::ConditionNotSatisfied } From 6973cc45908804a4a95ae0535eb92119d3e5c460 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 24 Sep 2024 16:16:16 +0000 Subject: [PATCH 48/51] Add comment. --- rs/system_api/src/sandbox_safe_system_state.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 69bb3d42f15..5ce162473d9 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -1220,6 +1220,14 @@ impl SandboxSafeSystemState { } } + /// Check if the condition for triggering `OnLowWasmMemoryHook` + /// is satisfied: + /// 1. In the case of `memory_allocation` + /// `wasm_memory_threshold >= min(memory_allocation - used_stable_memory, wasm_memory_limit) - used_wasm_memory` + /// 2. Without memory allocation + /// `wasm_memory_threshold >= wasm_memory_limit - used_wasm_memory` + /// and updates its status accordingly. + /// Note: if `wasm_memory_limit` is not set, its default value is 4 GiB. pub fn update_on_low_wasm_memory_hook_status( &mut self, memory_allocation: Option, From 33592b05c4b7977d3eff4092a379a55c853d6f97 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 24 Sep 2024 16:46:06 +0000 Subject: [PATCH 49/51] Refactor. --- rs/system_api/src/lib.rs | 36 +++++++++---------- .../src/sandbox_safe_system_state.rs | 6 ++-- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index c3cbbe5feca..316a0b2b417 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -917,30 +917,12 @@ impl MemoryUsage { ) -> Result<(), HypervisorError> { match execution_memory_type { ExecutionMemoryType::WasmMemory => { - let (new_usage, overflow) = self - .wasm_memory_usage - .get() - .overflowing_add(execution_bytes.get()); - - if overflow { - return Err(HypervisorError::OutOfMemory); - } - - self.wasm_memory_usage = NumBytes::new(new_usage); + add_memory(&mut self.wasm_memory_usage, execution_bytes) } ExecutionMemoryType::StableMemory => { - let (new_usage, overflow) = self - .stable_memory_usage - .get() - .overflowing_add(execution_bytes.get()); - if overflow { - return Err(HypervisorError::OutOfMemory); - } - self.stable_memory_usage = NumBytes::new(new_usage); + add_memory(&mut self.stable_memory_usage, execution_bytes) } } - - Ok(()) } /// Tries to allocate the requested amount of message memory. @@ -1010,6 +992,20 @@ impl MemoryUsage { } } +fn add_memory( + memory_size: &mut NumBytes, + additional_memory: NumBytes, +) -> Result<(), HypervisorError> { + let (new_usage, overflow) = memory_size.get().overflowing_add(additional_memory.get()); + + if overflow { + return Err(HypervisorError::OutOfMemory); + } + + *memory_size = NumBytes::new(new_usage); + Ok(()) +} + /// Struct that implements the SystemApi trait. This trait enables a canister to /// have mediated access to its system state. pub struct SystemApiImpl { diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 5ce162473d9..e3592425683 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -1220,13 +1220,13 @@ impl SandboxSafeSystemState { } } - /// Check if the condition for triggering `OnLowWasmMemoryHook` - /// is satisfied: + /// Updates status `OnLowWasmMemoryHook` if the following condition is satisfied: + /// /// 1. In the case of `memory_allocation` /// `wasm_memory_threshold >= min(memory_allocation - used_stable_memory, wasm_memory_limit) - used_wasm_memory` /// 2. Without memory allocation /// `wasm_memory_threshold >= wasm_memory_limit - used_wasm_memory` - /// and updates its status accordingly. + /// /// Note: if `wasm_memory_limit` is not set, its default value is 4 GiB. pub fn update_on_low_wasm_memory_hook_status( &mut self, From c97b287d09e2f3d7f7ef560a05ae8ccf94a42864 Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 24 Sep 2024 18:12:21 +0000 Subject: [PATCH 50/51] Fix test. --- rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs index 23aa658572b..09faeed67a6 100644 --- a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs +++ b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs @@ -24,6 +24,7 @@ use ic_types::{ }; use ic_wasm_types::BinaryEncodedWasm; +use ic_replicated_state::NumWasmPages; use lazy_static::lazy_static; use wasmtime::{Engine, Module, Store, StoreLimits, Val}; From 29c75c19662384971df0747e2f88f3f340b38a8e Mon Sep 17 00:00:00 2001 From: Dragoljub Duric Date: Tue, 24 Sep 2024 18:32:03 +0000 Subject: [PATCH 51/51] Move update() logic to system_api. --- .../src/canister_state/system_state.rs | 45 ----- .../src/canister_state/tests.rs | 85 +-------- .../src/sandbox_safe_system_state.rs | 176 +++++++++++++++++- 3 files changed, 168 insertions(+), 138 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 62a6eddc61d..541ac4d0b3f 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -418,51 +418,6 @@ impl TryFrom for OnLowWasmMemoryHookStatus { } } -impl OnLowWasmMemoryHookStatus { - pub fn update( - &mut self, - wasm_memory_threshold: NumBytes, - memory_allocation: Option, - wasm_memory_limit: Option, - used_stable_memory: NumBytes, - used_wasm_memory: NumBytes, - ) { - // If wasm memory limit is not set, the default is 4 GiB. Wasm memory - // limit is ignored for query methods, response callback handlers, - // global timers, heartbeats, and canister pre_upgrade. - let wasm_memory_limit = - wasm_memory_limit.unwrap_or_else(|| NumBytes::new(4 * 1024 * 1024 * 1024)); - - // If the canister has memory allocation, then it maximum allowed Wasm memory - // can be calculated as min(memory_allocation - used_stable_memory, wasm_memory_limit). - let wasm_capacity = memory_allocation.map_or_else( - || wasm_memory_limit, - |memory_allocation| { - debug_assert!( - used_stable_memory <= memory_allocation, - "Used stable memory: {:?} is larger than memory allocation: {:?}.", - used_stable_memory, - memory_allocation - ); - std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) - }, - ); - - // Conceptually we can think that the remaining Wasm memory is - // equal to `wasm_capacity - used_wasm_memory` and that should - // be compared with `wasm_memory_threshold` when checking for - // the condition for the hook. However, since `wasm_memory_limit` - // is ignored in some executions as stated above it is possible - // that `used_wasm_memory` is greater than `wasm_capacity` to - // avoid overflowing subtraction we adopted inequality. - if wasm_capacity >= used_wasm_memory + wasm_memory_threshold { - *self = OnLowWasmMemoryHookStatus::ConditionNotSatisfied; - } else if *self != OnLowWasmMemoryHookStatus::Executed { - *self = OnLowWasmMemoryHookStatus::Ready; - } - } -} - /// A wrapper around the different canister statuses. #[derive(Clone, Eq, PartialEq, Debug)] pub enum CanisterStatus { diff --git a/rs/replicated_state/src/canister_state/tests.rs b/rs/replicated_state/src/canister_state/tests.rs index eefa90b3805..f55cb6bcc7d 100644 --- a/rs/replicated_state/src/canister_state/tests.rs +++ b/rs/replicated_state/src/canister_state/tests.rs @@ -6,7 +6,7 @@ use crate::canister_state::execution_state::CustomSection; use crate::canister_state::execution_state::CustomSectionType; use crate::canister_state::execution_state::WasmMetadata; use crate::canister_state::system_state::{ - CallContextManager, CanisterHistory, CanisterStatus, CyclesUseCase, OnLowWasmMemoryHookStatus, + CallContextManager, CanisterHistory, CanisterStatus, CyclesUseCase, MAX_CANISTER_HISTORY_CHANGES, }; use crate::metadata_state::subnet_call_context_manager::InstallCodeCallId; @@ -1100,86 +1100,3 @@ fn reverts_stopping_status_after_split() { assert_eq!(expected_state, canister_state); } - -const GIB: u64 = 1 << 30; - -fn helper_is_condition_satisfied_for_on_low_wasm_memory_hook( - wasm_memory_threshold: u64, - memory_allocation: Option, - wasm_memory_limit: Option, - used_stable_memory: u64, - used_wasm_memory: u64, -) -> bool { - let wasm_memory_limit = wasm_memory_limit.unwrap_or(4 * GIB); - - let wasm_capacity = memory_allocation.map_or(wasm_memory_limit, |memory_allocation| { - std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) - }); - - wasm_capacity < used_wasm_memory + wasm_memory_threshold -} - -fn helper_test_on_low_wasm_memory_hook( - start_status: OnLowWasmMemoryHookStatus, - status_if_condition_satisfied: OnLowWasmMemoryHookStatus, -) { - for wasm_memory_threshold in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { - for memory_allocation in [None, Some(GIB), Some(2 * GIB), Some(3 * GIB), Some(4 * GIB)] { - for wasm_memory_limit in [None, Some(GIB), Some(2 * GIB), Some(3 * GIB), Some(4 * GIB)] - { - for used_stable_memory in [0, GIB] { - for used_wasm_memory in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { - let mut hook_status = start_status; - - hook_status.update( - wasm_memory_threshold.into(), - memory_allocation.map(|m| m.into()), - wasm_memory_limit.map(|m| m.into()), - used_stable_memory.into(), - used_wasm_memory.into(), - ); - - assert_eq!( - hook_status, - if helper_is_condition_satisfied_for_on_low_wasm_memory_hook( - wasm_memory_threshold, - memory_allocation, - wasm_memory_limit, - used_stable_memory, - used_wasm_memory - ) { - status_if_condition_satisfied - } else { - OnLowWasmMemoryHookStatus::ConditionNotSatisfied - } - ); - } - } - } - } - } -} - -#[test] -fn test_on_low_wasm_memory_hook_start_status_condition_not_satisfied() { - helper_test_on_low_wasm_memory_hook( - OnLowWasmMemoryHookStatus::ConditionNotSatisfied, - OnLowWasmMemoryHookStatus::Ready, - ); -} - -#[test] -fn test_on_low_wasm_memory_hook_start_status_ready() { - helper_test_on_low_wasm_memory_hook( - OnLowWasmMemoryHookStatus::Ready, - OnLowWasmMemoryHookStatus::Ready, - ); -} - -#[test] -fn test_on_low_wasm_memory_hook_start_status_executed() { - helper_test_on_low_wasm_memory_hook( - OnLowWasmMemoryHookStatus::Executed, - OnLowWasmMemoryHookStatus::Executed, - ); -} diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index e3592425683..3e2c51dbc39 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -1235,15 +1235,42 @@ impl SandboxSafeSystemState { used_stable_memory: NumBytes, used_wasm_memory: NumBytes, ) { - self.system_state_changes - .on_low_wasm_memory_hook_status - .update( - self.wasm_memory_threshold, - memory_allocation, - wasm_memory_limit, - used_stable_memory, - used_wasm_memory, - ); + // If wasm memory limit is not set, the default is 4 GiB. Wasm memory + // limit is ignored for query methods, response callback handlers, + // global timers, heartbeats, and canister pre_upgrade. + let wasm_memory_limit = + wasm_memory_limit.unwrap_or_else(|| NumBytes::new(4 * 1024 * 1024 * 1024)); + + // If the canister has memory allocation, then it maximum allowed Wasm memory + // can be calculated as min(memory_allocation - used_stable_memory, wasm_memory_limit). + let wasm_capacity = memory_allocation.map_or_else( + || wasm_memory_limit, + |memory_allocation| { + debug_assert!( + used_stable_memory <= memory_allocation, + "Used stable memory: {:?} is larger than memory allocation: {:?}.", + used_stable_memory, + memory_allocation + ); + std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) + }, + ); + + let on_low_wasm_memory_hook_status = + &mut self.system_state_changes.on_low_wasm_memory_hook_status; + + // Conceptually we can think that the remaining Wasm memory is + // equal to `wasm_capacity - used_wasm_memory` and that should + // be compared with `wasm_memory_threshold` when checking for + // the condition for the hook. However, since `wasm_memory_limit` + // is ignored in some executions as stated above it is possible + // that `used_wasm_memory` is greater than `wasm_capacity` to + // avoid overflowing subtraction we adopted inequality. + if wasm_capacity >= used_wasm_memory + self.wasm_memory_threshold { + *on_low_wasm_memory_hook_status = OnLowWasmMemoryHookStatus::ConditionNotSatisfied; + } else if *on_low_wasm_memory_hook_status != OnLowWasmMemoryHookStatus::Executed { + *on_low_wasm_memory_hook_status = OnLowWasmMemoryHookStatus::Ready; + } } // Returns `true` if storage cycles need to be reserved for the given @@ -1445,4 +1472,135 @@ mod tests { // Otherwise the correct `deadline` is returned. assert_eq!(helper_msg_deadline(Some(deadline)), deadline); } + + fn helper_create_state_for_hook_status( + start_status: OnLowWasmMemoryHookStatus, + wasm_memory_threshold: u64, + ) -> SandboxSafeSystemState { + SandboxSafeSystemState::new_internal( + canister_test_id(0), + CanisterStatusView::Running, + NumSeconds::from(3600), + MemoryAllocation::BestEffort, + NumBytes::new(wasm_memory_threshold), + ComputeAllocation::default(), + Cycles::new(1_000_000), + Cycles::zero(), + None, + None, + None, + None, + CyclesAccountManager::new( + NumInstructions::from(1_000_000_000), + SubnetType::Application, + subnet_test_id(0), + CyclesAccountManagerConfig::application_subnet(), + ), + Some(0), + BTreeMap::new(), + 0, + BTreeSet::new(), + SMALL_APP_SUBNET_MAX_SIZE, + SchedulerConfig::application_subnet().dirty_page_overhead, + CanisterTimer::Inactive, + 0, + BTreeSet::new(), + RequestMetadata::new(0, Time::from_nanos_since_unix_epoch(0)), + None, + 0, + start_status, + ) + } + + const GIB: u64 = 1 << 30; + + fn helper_is_condition_satisfied_for_on_low_wasm_memory_hook( + wasm_memory_threshold: u64, + memory_allocation: Option, + wasm_memory_limit: Option, + used_stable_memory: u64, + used_wasm_memory: u64, + ) -> bool { + let wasm_memory_limit = wasm_memory_limit.unwrap_or(4 * GIB); + + let wasm_capacity = memory_allocation.map_or(wasm_memory_limit, |memory_allocation| { + std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) + }); + + wasm_capacity < used_wasm_memory + wasm_memory_threshold + } + + fn helper_test_on_low_wasm_memory_hook( + start_status: OnLowWasmMemoryHookStatus, + status_if_condition_satisfied: OnLowWasmMemoryHookStatus, + ) { + for wasm_memory_threshold in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { + for memory_allocation in [None, Some(GIB), Some(2 * GIB), Some(3 * GIB), Some(4 * GIB)] + { + for wasm_memory_limit in + [None, Some(GIB), Some(2 * GIB), Some(3 * GIB), Some(4 * GIB)] + { + for used_stable_memory in [0, GIB] { + for used_wasm_memory in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { + let mut state = helper_create_state_for_hook_status( + start_status, + wasm_memory_threshold, + ); + + assert_eq!( + state.system_state_changes.on_low_wasm_memory_hook_status, + start_status + ); + + state.update_on_low_wasm_memory_hook_status( + memory_allocation.map(|m| m.into()), + wasm_memory_limit.map(|m| m.into()), + used_stable_memory.into(), + used_wasm_memory.into(), + ); + + assert_eq!( + state.system_state_changes.on_low_wasm_memory_hook_status, + if helper_is_condition_satisfied_for_on_low_wasm_memory_hook( + wasm_memory_threshold, + memory_allocation, + wasm_memory_limit, + used_stable_memory, + used_wasm_memory + ) { + status_if_condition_satisfied + } else { + OnLowWasmMemoryHookStatus::ConditionNotSatisfied + } + ); + } + } + } + } + } + } + + #[test] + fn test_on_low_wasm_memory_hook_start_status_condition_not_satisfied() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); + } + + #[test] + fn test_on_low_wasm_memory_hook_start_status_ready() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::Ready, + OnLowWasmMemoryHookStatus::Ready, + ); + } + + #[test] + fn test_on_low_wasm_memory_hook_start_status_executed() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::Executed, + OnLowWasmMemoryHookStatus::Executed, + ); + } }