refactor: move log_memory_store_feature to embedders::feature_flags

rs/config/src/embedders.rs

@@ -1,14 +1,12 @@
-use std::time::Duration;
-
+use crate::{execution_environment::LOG_MEMORY_STORE_FEATURE_ENABLED, flag_status::FlagStatus};
 use ic_base_types::NumBytes;
 use ic_sys::PAGE_SIZE;
 use ic_types::{
     MAX_STABLE_MEMORY_IN_BYTES, MAX_WASM_MEMORY_IN_BYTES, MAX_WASM64_MEMORY_IN_BYTES,
     NumInstructions, NumOsPages,
 };
 use serde::{Deserialize, Serialize};
-
-use crate::flag_status::FlagStatus;
+use std::time::Duration;
 
 // Defining 100000 globals in a module can result in significant overhead in
 // each message's execution time (about 40x), so set a limit 3 orders of
@@ -122,10 +120,15 @@ pub struct FeatureFlags {
     /// If this flag is enabled, then the output of the `debug_print` system-api
     /// call will be skipped based on heuristics.
     pub rate_limiting_of_debug_prints: FlagStatus,
+
     /// If this flag is enabled, then the environment variables are supported.
     pub environment_variables: FlagStatus,
+
     /// Use deterministic memory tracker.
     pub deterministic_memory_tracker: FlagStatus,
+
+    /// Enables the log memory store feature.
+    pub log_memory_store_feature: FlagStatus,
 }
 
 impl FeatureFlags {
@@ -134,6 +137,10 @@ impl FeatureFlags {
             rate_limiting_of_debug_prints: FlagStatus::Enabled,
             environment_variables: FlagStatus::Enabled,
             deterministic_memory_tracker: FlagStatus::Disabled,
+            log_memory_store_feature: match LOG_MEMORY_STORE_FEATURE_ENABLED {
+                true => FlagStatus::Enabled,
+                false => FlagStatus::Disabled,
+            },
         }
     }
 }

rs/config/src/execution_environment.rs

@@ -12,11 +12,6 @@ const TIB: u64 = 1024 * GIB;
 
 // TODO(DSM-105): remove after the feature is enabled by default.
 pub const LOG_MEMORY_STORE_FEATURE_ENABLED: bool = false;
-pub const LOG_MEMORY_STORE_FEATURE: FlagStatus = if LOG_MEMORY_STORE_FEATURE_ENABLED {
-    FlagStatus::Enabled
-} else {
-    FlagStatus::Disabled
-};
 pub const TEST_DEFAULT_LOG_MEMORY_LIMIT: u64 = if LOG_MEMORY_STORE_FEATURE_ENABLED {
     4 * KIB
 } else {
@@ -378,9 +373,6 @@ pub struct Config {
 
     /// Enables the replicated inter-canister calls to `fetch_canister_logs`.
     pub replicated_inter_canister_log_fetch: FlagStatus,
-
-    /// Enables the log memory store feature.
-    pub log_memory_store_feature: FlagStatus,
 }
 
 impl Default for Config {
@@ -467,7 +459,6 @@ impl Default for Config {
             max_environment_variable_name_length: MAX_ENVIRONMENT_VARIABLE_NAME_LENGTH,
             max_environment_variable_value_length: MAX_ENVIRONMENT_VARIABLE_VALUE_LENGTH,
             replicated_inter_canister_log_fetch: FlagStatus::Disabled,
-            log_memory_store_feature: LOG_MEMORY_STORE_FEATURE,
         }
     }
 }

rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs

@@ -5,7 +5,7 @@ use super::{
     routing::ResolveDestinationError,
 };
 use ic_base_types::{CanisterId, NumBytes, NumOsPages, NumSeconds, PrincipalId, SubnetId};
-use ic_config::execution_environment::LOG_MEMORY_STORE_FEATURE_ENABLED;
+use ic_config::{embedders::FeatureFlags, flag_status::FlagStatus};
 use ic_cycles_account_manager::{
     CyclesAccountManager, CyclesAccountManagerError, ResourceSaturation,
 };
@@ -333,10 +333,11 @@ impl SystemStateModifications {
         network_topology: &NetworkTopology,
         own_subnet_id: SubnetId,
         is_composite_query: bool,
+        feature_flags: &FeatureFlags,
         logger: &ReplicaLogger,
     ) -> HypervisorResult<RequestMetadataStats> {
         // Append delta logs.
-        if LOG_MEMORY_STORE_FEATURE_ENABLED {
+        if feature_flags.log_memory_store_feature == FlagStatus::Enabled {
             let log_memory_store = &mut system_state.log_memory_store;
             // TODO(DSM-11): cleanup population logic after migration is done.
             // We need to copy existing canister_log to log_memory_store in order

rs/embedders/tests/sandbox_safe_system_state.rs

@@ -295,6 +295,7 @@ fn correct_charging_source_canister_for_a_request() {
             &default_network_topology(),
             subnet_test_id(1),
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();
@@ -512,6 +513,7 @@ fn call_increases_cycles_consumed_metric() {
             &default_network_topology(),
             subnet_test_id(1),
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();
@@ -620,6 +622,7 @@ fn test_inter_canister_call(
             topo,
             subnet_id,
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();

rs/embedders/tests/system_api.rs

@@ -1230,6 +1230,7 @@ fn certified_data_set() {
             &default_network_topology(),
             subnet_test_id(1),
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();
@@ -1417,6 +1418,7 @@ fn call_perform_not_enough_cycles_does_not_trap() {
             &default_network_topology(),
             subnet_test_id(1),
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();
@@ -1569,6 +1571,7 @@ fn helper_test_on_low_wasm_memory(
             &default_network_topology(),
             subnet_test_id(1),
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();
@@ -1790,6 +1793,7 @@ fn push_output_request_respects_memory_limits() {
             &default_network_topology(),
             subnet_test_id(1),
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();
@@ -1906,6 +1910,7 @@ fn push_output_request_oversized_request_memory_limits() {
             &default_network_topology(),
             subnet_test_id(1),
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();
@@ -1943,6 +1948,7 @@ fn ic0_global_timer_set_is_propagated_from_sandbox() {
             &default_network_topology(),
             subnet_test_id(1),
             false,
+            &Default::default(),
             &no_op_logger(),
         )
         .unwrap();
@@ -2192,6 +2198,7 @@ fn ic0_call_with_best_effort_response() {
                 &default_network_topology(),
                 own_subnet_id,
                 false,
+                &Default::default(),
                 &no_op_logger(),
             )
             .unwrap();

rs/execution_environment/src/canister_manager.rs

@@ -1,19 +1,17 @@
-use crate::as_round_instructions;
-use crate::execution::{common::validate_controller, install_code::OriginalContext};
-use crate::execution::{install::execute_install, upgrade::execute_upgrade};
-use crate::execution_environment::{
-    CompilationCostHandling, RoundContext, RoundCounters, RoundLimits,
-};
-use crate::util::MIGRATION_CANISTER_ID;
 use crate::{
+    as_round_instructions,
     canister_settings::{CanisterSettings, ValidatedCanisterSettings},
+    execution::{
+        common::validate_controller, install::execute_install, install_code::OriginalContext,
+        upgrade::execute_upgrade,
+    },
+    execution_environment::{CompilationCostHandling, RoundContext, RoundCounters, RoundLimits},
     hypervisor::Hypervisor,
     types::{IngressResponse, Response},
-    util::GOVERNANCE_CANISTER_ID,
+    util::{GOVERNANCE_CANISTER_ID, MIGRATION_CANISTER_ID},
 };
 use ic_base_types::NumSeconds;
-use ic_config::embedders::Config as EmbeddersConfig;
-use ic_config::flag_status::FlagStatus;
+use ic_config::{embedders::Config as EmbeddersConfig, flag_status::FlagStatus};
 use ic_cycles_account_manager::{CyclesAccountManager, ResourceSaturation};
 use ic_embedders::{
     wasm_utils::decoding::decode_wasm, wasmtime_embedder::system_api::ExecutionParameters,
@@ -556,11 +554,14 @@ impl CanisterManager {
         if let Some(log_visibility) = settings.log_visibility() {
             canister.system_state.log_visibility = log_visibility.clone();
         }
-        if let Some(log_memory_limit) = settings.log_memory_limit() {
+        if let (Some(limit), FlagStatus::Enabled) = (
+            settings.log_memory_limit(),
+            self.hypervisor.feature_flags().log_memory_store_feature,
+        ) {
             canister
                 .system_state
                 .log_memory_store
-                .resize(log_memory_limit.get() as usize, self.fd_factory.clone());
+                .resize(limit.get() as usize, self.fd_factory.clone());
         }
         if let Some(wasm_memory_limit) = settings.wasm_memory_limit() {
             canister.system_state.wasm_memory_limit = Some(wasm_memory_limit);

rs/execution_environment/src/execution/call_or_task.rs

@@ -11,7 +11,7 @@ use crate::execution_environment::{
 };
 use crate::metrics::CallTreeMetrics;
 use ic_base_types::CanisterId;
-use ic_config::flag_status::FlagStatus;
+use ic_config::{embedders::FeatureFlags, flag_status::FlagStatus};
 use ic_embedders::{
     wasm_executor::{CanisterStateChanges, PausedWasmExecution, WasmExecutionResult},
     wasmtime_embedder::system_api::{ApiType, ExecutionParameters},
@@ -33,6 +33,7 @@ use ic_types::methods::{FuncRef, SystemMethod, WasmMethod};
 use ic_types::{CanisterTimer, Cycles, NumBytes, NumInstructions, Time};
 use ic_utils_thread::deallocator_thread::DeallocationSender;
 use ic_wasm_types::WasmEngineError::FailedToApplySystemChanges;
+use std::sync::Arc;
 
 #[cfg(test)]
 mod tests;
@@ -50,6 +51,7 @@ pub fn execute_call_or_task(
     round_limits: &mut RoundLimits,
     subnet_size: usize,
     call_tree_metrics: &dyn CallTreeMetrics,
+    feature_flags: &FeatureFlags,
     log_dirty_pages: FlagStatus,
     deallocation_sender: &DeallocationSender,
 ) -> ExecuteMessageResult {
@@ -149,6 +151,7 @@ pub fn execute_call_or_task(
         freezing_threshold,
         canister_id: clean_canister.canister_id(),
         log_dirty_pages,
+        feature_flags: Arc::new(*feature_flags),
     };
 
     let helper = match CallOrTaskHelper::new(&clean_canister, &original, deallocation_sender) {
@@ -324,6 +327,7 @@ struct OriginalContext {
     freezing_threshold: Cycles,
     canister_id: CanisterId,
     log_dirty_pages: FlagStatus,
+    feature_flags: Arc<FeatureFlags>,
 }
 
 /// Contains fields of `CallOrTaskHelper` that are necessary for resuming an update
@@ -567,6 +571,7 @@ impl CallOrTaskHelper {
                     call_tree_metrics,
                     original.time,
                     is_composite_query,
+                    &original.feature_flags,
                     &|system_state| self.deallocation_sender.send(Box::new(system_state)),
                 );
 
@@ -586,6 +591,7 @@ impl CallOrTaskHelper {
                         round.network_topology,
                         round.hypervisor.subnet_id(),
                         is_composite_query,
+                        &original.feature_flags,
                         round.log,
                     )
                 {

rs/execution_environment/src/execution/common.rs

@@ -1,12 +1,13 @@
 // This module defines common helper functions.
 // TODO(RUN-60): Move helper functions here.
 
-use crate::execution_environment::ExecutionResponse;
 use crate::{
     ExecuteMessageResult, RoundLimits, as_round_instructions,
-    canister_manager::types::CanisterManagerError, metrics::CallTreeMetrics,
+    canister_manager::types::CanisterManagerError, execution_environment::ExecutionResponse,
+    metrics::CallTreeMetrics,
 };
 use ic_base_types::{CanisterId, NumBytes, SubnetId};
+use ic_config::embedders::FeatureFlags;
 use ic_embedders::{
     wasm_executor::{CanisterStateChanges, ExecutionStateChanges, SliceExecutionOutput},
     wasmtime_embedder::system_api::sandbox_safe_system_state::{
@@ -444,6 +445,7 @@ fn try_apply_canister_state_changes(
     network_topology: &NetworkTopology,
     subnet_id: SubnetId,
     is_composite_query: bool,
+    feature_flags: &FeatureFlags,
     log: &ReplicaLogger,
 ) -> HypervisorResult<RequestMetadataStats> {
     subnet_available_memory
@@ -460,6 +462,7 @@ fn try_apply_canister_state_changes(
         network_topology,
         subnet_id,
         is_composite_query,
+        feature_flags,
         log,
     )
 }
@@ -488,6 +491,7 @@ pub fn apply_canister_state_changes(
     call_tree_metrics: &dyn CallTreeMetrics,
     call_context_creation_time: Time,
     is_composite_query: bool,
+    feature_flags: &FeatureFlags,
     deallocate: &dyn Fn(SystemState),
 ) {
     let CanisterStateChanges {
@@ -509,6 +513,7 @@ pub fn apply_canister_state_changes(
         network_topology,
         subnet_id,
         is_composite_query,
+        feature_flags,
         log,
     ) {
         Ok(request_stats) => {

rs/execution_environment/src/execution/install_code.rs

@@ -665,6 +665,7 @@ impl InstallCodeHelper {
                 round.network_topology,
                 round.hypervisor.subnet_id(),
                 false, // Install cannot happen in composite_query.
+                round.hypervisor.feature_flags(),
                 round.log,
             );