chore: Revert unified page instruction charging

rs/bitcoin/checker/tests/tests.rs

@@ -38,7 +38,7 @@ const TEST_SUBNET_NODES: u16 = 34;
 // and checking the actual consumptions. It is both big enough to allow tests to
 // succeed, and small enough not to interfere with the expected cycle cost we
 // are testing for.
-const UNIVERSAL_CANISTER_CYCLE_MARGIN: u128 = 46_000_000;
+const UNIVERSAL_CANISTER_CYCLE_MARGIN: u128 = 43_000_000;
 
 struct Setup {
     // Owner of canisters created for the setup.

rs/canister_sandbox/src/protocol/sbxsvc.rs

@@ -495,6 +495,7 @@ mod tests {
                         CyclesAccountManagerConfig::application_subnet(),
                     ),
                     std::sync::Arc::new(NetworkTopology::default()),
+                    NumInstructions::new(42),
                     ComputeAllocation::zero(),
                     123,
                     RequestMetadata::new(0, Time::from_nanos_since_unix_epoch(10)),

rs/canister_sandbox/src/sandbox_server.rs

@@ -126,7 +126,9 @@ mod tests {
     };
     use ic_base_types::{NumSeconds, PrincipalId};
     use ic_config::embedders::Config as EmbeddersConfig;
-    use ic_config::subnet_config::{CyclesAccountManagerConfig, DEFAULT_REFERENCE_SUBNET_SIZE};
+    use ic_config::subnet_config::{
+        CyclesAccountManagerConfig, DEFAULT_REFERENCE_SUBNET_SIZE, SchedulerConfig,
+    };
     use ic_cycles_account_manager::{
         CyclesAccountManager, CyclesAccountManagerSubnetConfig, ResourceSaturation,
     };
@@ -220,6 +222,7 @@ mod tests {
                 CanisterCyclesCostSchedule::Normal,
                 DEFAULT_REFERENCE_SUBNET_SIZE,
             ),
+            SchedulerConfig::application_subnet().dirty_page_overhead,
             CanisterTimer::Inactive,
             0,
             BTreeSet::from([controller]),

rs/config/src/embedders.rs

@@ -86,6 +86,9 @@ pub(crate) const DEFAULT_MAX_DIRTY_PAGES_WITHOUT_OPTIMIZATION: usize = (GIB as u
 /// Scheduling overhead for copying dirty pages, in instructions.
 pub(crate) const DIRTY_PAGE_COPY_OVERHEAD: NumInstructions = NumInstructions::new(3_000);
 
+/// The overhead for dirty pages in Wasm64.
+pub const WASM64_DIRTY_PAGE_OVERHEAD_MULTIPLIER: u64 = 4;
+
 const KIB: u64 = 1024;
 const GIB: u64 = KIB * KIB * KIB;
 
@@ -242,6 +245,9 @@ pub struct Config {
     /// The dirty page copying overhead, in instructions.
     pub dirty_page_copy_overhead: NumInstructions,
 
+    /// The dirty page overhead factor for Wasm64.
+    pub wasm64_dirty_page_overhead_multiplier: u64,
+
     /// The maximum allowed size for an uncompressed canister Wasm module.
     pub wasm_max_size: NumBytes,
 
@@ -292,6 +298,7 @@ impl Config {
             max_wasm_memory_size: NumBytes::new(MAX_WASM_MEMORY_IN_BYTES),
             max_wasm64_memory_size: NumBytes::new(MAX_WASM64_MEMORY_IN_BYTES),
             max_stable_memory_size: NumBytes::new(MAX_STABLE_MEMORY_IN_BYTES),
+            wasm64_dirty_page_overhead_multiplier: WASM64_DIRTY_PAGE_OVERHEAD_MULTIPLIER,
         }
     }
 }

rs/config/src/subnet_config.rs

@@ -139,7 +139,7 @@ pub const DEFAULT_REFERENCE_SUBNET_SIZE: usize = 13;
 pub const SEV_REFERENCE_SUBNET_SIZE: usize = 7;
 
 /// Costs for each newly created dirty page in stable memory.
-pub const DEFAULT_DIRTY_PAGE_OVERHEAD: NumInstructions = NumInstructions::new(5_000);
+const DEFAULT_DIRTY_PAGE_OVERHEAD: NumInstructions = NumInstructions::new(1_000);
 
 /// Accumulated priority reset interval, rounds.
 ///

rs/embedders/BUILD.bazel

@@ -92,7 +92,6 @@ rust_test(
     crate = ":embedders",
     deps = [
         # Keep sorted.
-        "//rs/config",
         "//rs/interfaces",
         "//rs/test_utilities",
         "//rs/test_utilities/types",

rs/embedders/Cargo.toml

@@ -62,7 +62,6 @@ assert_matches = { workspace = true }
 canister-test = { path = "../rust_canisters/canister_test" }
 criterion = { workspace = true }
 embedders_bench = { path = "benches/embedders_bench" }
-ic-config = {path = "../config"}
 ic-registry-routing-table = { path = "../registry/routing_table" }
 ic-test-utilities = { path = "../test_utilities" }
 ic-test-utilities-embedders = { path = "../test_utilities/embedders" }

rs/embedders/fuzz/src/wasm_executor.rs

@@ -1,7 +1,8 @@
 use crate::ic_wasm::{ICWasmModule, get_system_api_type_for_wasm_method};
 use ic_config::{
-    embedders::Config as EmbeddersConfig, execution_environment::Config as HypervisorConfig,
-    subnet_config::DEFAULT_REFERENCE_SUBNET_SIZE,
+    embedders::Config as EmbeddersConfig,
+    execution_environment::Config as HypervisorConfig,
+    subnet_config::{DEFAULT_REFERENCE_SUBNET_SIZE, SchedulerConfig},
 };
 use ic_cycles_account_manager::{CyclesAccountManagerSubnetConfig, ResourceSaturation};
 use ic_embedders::{
@@ -152,12 +153,14 @@ pub(crate) fn get_sandbox_safe_system_state(
     api_type: ApiType,
 ) -> SandboxSafeSystemState {
     let cycles_account_manager = CyclesAccountManagerBuilder::new().build();
+    let dirty_page_overhead = SchedulerConfig::application_subnet().dirty_page_overhead;
     let network_topology = NetworkTopology::default();
 
     SandboxSafeSystemState::new_for_testing(
         system_state,
         cycles_account_manager,
         Arc::new(network_topology),
+        dirty_page_overhead,
         ComputeAllocation::default(),
         HypervisorConfig::default().subnet_callback_soft_limit as u64,
         Default::default(),

rs/embedders/src/wasm_utils.rs

@@ -222,6 +222,7 @@ fn validate_and_instrument(
         module,
         config.cost_to_compile_wasm_instruction,
         config.metering_type,
+        config.dirty_page_overhead,
         max_wasm_memory_size,
         config.max_stable_memory_size,
     )?;

rs/embedders/src/wasm_utils/instrumentation.rs

@@ -1476,6 +1476,7 @@ fn replace_system_api_functions(
     injected_functions: &InjectedFunctions,
     injected_counters: &InjectedCounters,
     stable_memory_index: u32,
+    dirty_page_overhead: NumInstructions,
     main_memory_type: WasmMemoryType,
     max_wasm_memory_size: NumBytes,
 ) {
@@ -1487,6 +1488,7 @@ fn replace_system_api_functions(
         injected_functions,
         injected_counters,
         stable_memory_index,
+        dirty_page_overhead,
         main_memory_type,
         max_wasm_memory_size,
     ) {
@@ -1562,6 +1564,7 @@ pub(super) fn instrument(
     mut module: wirm::Module<'_>,
     cost_to_compile_wasm_instruction: NumInstructions,
     metering_type: MeteringType,
+    dirty_page_overhead: NumInstructions,
     max_wasm_memory_size: NumBytes,
     max_stable_memory_size: NumBytes,
 ) -> Result<InstrumentationOutput, WasmInstrumentationError> {
@@ -1630,6 +1633,7 @@ pub(super) fn instrument(
         &injected_functions,
         &injected_counters,
         stable_memory_index,
+        dirty_page_overhead,
         main_memory_type,
         max_wasm_memory_size,
     );

rs/embedders/src/wasm_utils/system_api_replacements.rs

@@ -18,6 +18,7 @@ use crate::{
 };
 use ic_interfaces::execution_environment::StableMemoryApi;
 use ic_sys::PAGE_SIZE;
+use ic_types::NumInstructions;
 use wirm::{DataType, ir::types::Instructions, wasmparser::BlockType};
 
 use ic_types::NumBytes;
@@ -53,6 +54,7 @@ pub(super) fn replacement_functions(
     injected_functions: &InjectedFunctions,
     injected_counters: &InjectedCounters,
     stable_memory_index: u32,
+    dirty_page_overhead: NumInstructions,
     main_memory_type: WasmMemoryType,
     max_wasm_memory_size: NumBytes,
 ) -> Vec<(SystemApiFunc, ReplacementFunction)> {
@@ -910,6 +912,21 @@ pub(super) fn replacement_functions(
                             function_index: injected_functions.internal_trap,
                         },
                         End,
+                        // Decrement instruction counter to charge for dirty pages
+                        LocalGet {
+                            local_index: DIRTY_PAGE_COUNT,
+                        },
+                        I64ExtendI32U,
+                        I64Const {
+                            value: dirty_page_overhead.get().try_into().unwrap(),
+                        },
+                        I64Mul,
+                        // Bounds check above should guarantee that we don't
+                        // overflow as the over head is a small constant.
+                        Call {
+                            function_index: decr_instruction_counter_fn,
+                        },
+                        Drop,
                         // perform memory fill
                         LocalGet {
                             local_index: BYTEMAP_START,
@@ -1138,6 +1155,21 @@ pub(super) fn replacement_functions(
                             function_index: injected_functions.internal_trap,
                         },
                         End,
+                        // Decrement instruction counter to charge for dirty pages
+                        LocalGet {
+                            local_index: DIRTY_PAGE_COUNT,
+                        },
+                        I64ExtendI32U,
+                        I64Const {
+                            value: dirty_page_overhead.get().try_into().unwrap(),
+                        },
+                        I64Mul,
+                        // Bounds check above should guarantee that we don't
+                        // overflow as the over head is a small constant.
+                        Call {
+                            function_index: decr_instruction_counter_fn,
+                        },
+                        Drop,
                         // perform memory fill
                         LocalGet {
                             local_index: BYTEMAP_START,

rs/embedders/src/wasmtime_embedder.rs

@@ -415,12 +415,9 @@ impl WasmtimeEmbedder {
             bytemap_name: Some(STABLE_BYTEMAP_MEMORY_NAME),
             memory: stable_memory.clone(),
             memory_type: CanisterMemoryType::Stable,
-            // Wasm native stable memory is tracked by a
-            // bytemap within the wasm module, but that's used
-            // only for limiting the memory. The deterministic
-            // memory tracker accounts for page fault charging,
-            // same as for the heap pages.
-            dirty_page_tracking,
+            // Wasm native stable memory will always be tracked by a
+            // bytemap within the wasm module.
+            dirty_page_tracking: DirtyPageTracking::Ignore,
         });
 
         result
@@ -643,12 +640,20 @@ impl WasmtimeEmbedder {
             main_memory_type = WasmMemoryType::Wasm64;
         }
 
+        let dirty_page_overhead = match main_memory_type {
+            WasmMemoryType::Wasm32 => self.config.dirty_page_overhead,
+            WasmMemoryType::Wasm64 => NumInstructions::from(
+                self.config.dirty_page_overhead.get()
+                    * self.config.wasm64_dirty_page_overhead_multiplier,
+            ),
+        };
+
         let memory_trackers = sigsegv_memory_tracker(
             memories,
             &mut *store,
             self.log.clone(),
             self.config.feature_flags.deterministic_memory_tracker,
-            self.config.dirty_page_overhead,
+            /*dirty_page_overhead*/ NumInstructions::new(1),
             subtract_instruction_counter,
         );
 
@@ -660,6 +665,7 @@ impl WasmtimeEmbedder {
             instance_stats: InstanceStats::default(),
             store,
             modification_tracking,
+            dirty_page_overhead,
             #[cfg(debug_assertions)]
             stable_memory_dirty_page_limit: current_dirty_page_limit,
             stable_memory_page_access_limit: current_accessed_limit,
@@ -941,6 +947,7 @@ pub struct WasmtimeInstance {
     instance_stats: InstanceStats,
     store: Pin<Box<wasmtime::Store<StoreData>>>,
     modification_tracking: ModificationTracking,
+    dirty_page_overhead: NumInstructions,
     #[cfg(debug_assertions)]
     #[allow(dead_code)]
     stable_memory_dirty_page_limit: ic_types::NumOsPages,
@@ -1211,7 +1218,13 @@ impl WasmtimeInstance {
         let access = self.page_accesses()?;
         self.set_instance_stats(&access);
 
-        // No need to charge for dirty wasm heap pages anymore: The DMT charges directly.
+        // Charge for dirty wasm heap pages.
+        let x = self.instruction_counter().saturating_sub_unsigned(
+            self.dirty_page_overhead
+                .get()
+                .saturating_mul(access.wasm_dirty_pages.len() as u64),
+        );
+        self.set_instruction_counter(x);
 
         match result {
             Ok(_) => Ok(InstanceRunResult {

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

@@ -4,7 +4,7 @@ use super::{
     ApiType, CERTIFIED_DATA_MAX_LENGTH, cycles_balance_change::CyclesBalanceChange, routing,
     routing::ResolveDestinationError,
 };
-use ic_base_types::{CanisterId, NumBytes, NumSeconds, PrincipalId, SubnetId};
+use ic_base_types::{CanisterId, NumBytes, NumOsPages, NumSeconds, PrincipalId, SubnetId};
 use ic_cycles_account_manager::{
     CyclesAccountManager, CyclesAccountManagerError, ResourceSaturation,
 };
@@ -26,7 +26,7 @@ use ic_replicated_state::{
 };
 use ic_types::canister_log::CanisterLogMetrics;
 use ic_types::{
-    CanisterLog, CanisterTimer, ComputeAllocation, MemoryAllocation, Time,
+    CanisterLog, CanisterTimer, ComputeAllocation, MemoryAllocation, NumInstructions, Time,
     messages::{CallContextId, NO_DEADLINE, RejectContext, RequestMetadata},
     time::CoarseTime,
 };
@@ -634,6 +634,7 @@ pub struct SandboxSafeSystemState {
     pub(super) status: CanisterStatusView,
     pub(super) subnet_type: SubnetType,
     pub(super) subnet_cycles_config: CyclesAccountManagerSubnetConfig,
+    dirty_page_overhead: NumInstructions,
     freeze_threshold: NumSeconds,
     memory_allocation: MemoryAllocation,
     wasm_memory_threshold: NumBytes,
@@ -686,6 +687,7 @@ impl SandboxSafeSystemState {
         ic00_available_request_slots: usize,
         ic00_aliases: BTreeSet<CanisterId>,
         subnet_cycles_config: CyclesAccountManagerSubnetConfig,
+        dirty_page_overhead: NumInstructions,
         global_timer: CanisterTimer,
         canister_version: u64,
         controllers: BTreeSet<PrincipalId>,
@@ -701,6 +703,7 @@ impl SandboxSafeSystemState {
             status,
             subnet_type: cycles_account_manager.subnet_type(),
             subnet_cycles_config,
+            dirty_page_overhead,
             freeze_threshold,
             memory_allocation,
             environment_variables,
@@ -740,6 +743,7 @@ impl SandboxSafeSystemState {
         system_state: &SystemState,
         cycles_account_manager: CyclesAccountManager,
         network_topology: Arc<NetworkTopology>,
+        dirty_page_overhead: NumInstructions,
         compute_allocation: ComputeAllocation,
         available_callbacks: u64,
         request_metadata: RequestMetadata,
@@ -751,6 +755,7 @@ impl SandboxSafeSystemState {
             system_state,
             cycles_account_manager,
             network_topology,
+            dirty_page_overhead,
             compute_allocation,
             available_callbacks,
             request_metadata,
@@ -766,6 +771,7 @@ impl SandboxSafeSystemState {
         system_state: &SystemState,
         cycles_account_manager: CyclesAccountManager,
         network_topology: Arc<NetworkTopology>,
+        dirty_page_overhead: NumInstructions,
         compute_allocation: ComputeAllocation,
         available_callbacks: u64,
         request_metadata: RequestMetadata,
@@ -838,6 +844,7 @@ impl SandboxSafeSystemState {
             ic00_available_request_slots,
             ic00_aliases,
             subnet_cycles_config,
+            dirty_page_overhead,
             system_state.global_timer,
             system_state.canister_version(),
             system_state.controllers.clone(),
@@ -1168,6 +1175,23 @@ impl SandboxSafeSystemState {
         Ok(())
     }
 
+    /// Calculate the cost for newly created dirty pages.
+    pub fn dirty_page_cost(&self, dirty_pages: NumOsPages) -> HypervisorResult<NumInstructions> {
+        let (inst, overflow) = dirty_pages
+            .get()
+            .overflowing_mul(self.dirty_page_overhead.get());
+        if overflow {
+            Err(HypervisorError::ToolchainContractViolation {
+                error: format!(
+                    "Overflow calculating instruction cost for dirty pages - conversion rate: {}, dirty_pages: {}",
+                    self.dirty_page_overhead, dirty_pages
+                ),
+            })
+        } else {
+            Ok(NumInstructions::from(inst))
+        }
+    }
+
     pub fn is_controller(&self, principal_id: &PrincipalId) -> bool {
         self.controllers.contains(principal_id)
     }
@@ -1426,7 +1450,7 @@ mod tests {
     use std::collections::{BTreeMap, BTreeSet};
 
     use ic_base_types::NumSeconds;
-    use ic_config::subnet_config::CyclesAccountManagerConfig;
+    use ic_config::subnet_config::{CyclesAccountManagerConfig, SchedulerConfig};
     use ic_cycles_account_manager::{CyclesAccountManager, CyclesAccountManagerSubnetConfig};
     use ic_limits::SMALL_APP_SUBNET_MAX_SIZE;
     use ic_registry_subnet_type::SubnetType;
@@ -1566,6 +1590,7 @@ mod tests {
                 CanisterCyclesCostSchedule::Normal,
                 ic_config::subnet_config::DEFAULT_REFERENCE_SUBNET_SIZE,
             ),
+            SchedulerConfig::application_subnet().dirty_page_overhead,
             CanisterTimer::Inactive,
             0,
             BTreeSet::new(),