Chore: RUN-725: Cleanup old metering

Author: Maciej Kot

rs/config/src/embedders.rs

@@ -105,7 +105,6 @@ impl Default for FeatureFlags {
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
 pub enum MeteringType {
-    Old,
     New,
     /// for testing and benchmarking
     None,

rs/drun/src/main.rs

@@ -1,10 +1,10 @@
-use clap::{Arg, ArgAction, ArgMatches, Command};
+use clap::{Arg, ArgMatches, Command};
 use ic_canister_sandbox_backend_lib::{
     canister_sandbox_main, compiler_sandbox::compiler_sandbox_main,
     launcher::sandbox_launcher_main, RUN_AS_CANISTER_SANDBOX_FLAG, RUN_AS_COMPILER_SANDBOX_FLAG,
     RUN_AS_SANDBOX_LAUNCHER_FLAG,
 };
-use ic_config::{embedders::MeteringType, flag_status::FlagStatus, Config, ConfigSource};
+use ic_config::{flag_status::FlagStatus, Config, ConfigSource};
 use ic_drun::{run_drun, DrunOptions};
 use ic_registry_subnet_type::SubnetType;
 use std::path::PathBuf;
@@ -17,7 +17,6 @@ const ARG_MESSAGES: &str = "messages";
 const ARG_EXTRA_BATCHES: &str = "extra-batches";
 const ARG_INSTRUCTION_LIMIT: &str = "instruction-limit";
 const ARG_SUBNET_TYPE: &str = "subnet-type";
-const USE_OLD_METERING: &str = "use-old-metering";
 
 fn main() -> Result<(), String> {
     // Check if `drun` is running in the canister sandbox mode where it waits
@@ -56,7 +55,7 @@ async fn drun_main() -> Result<(), String> {
             .rate_limiting_of_debug_prints = FlagStatus::Disabled;
         default_config.hypervisor.rate_limiting_of_heap_delta = FlagStatus::Disabled;
         default_config.hypervisor.rate_limiting_of_instructions = FlagStatus::Disabled;
-        let mut cfg = Config::load_with_default(&source, default_config).unwrap_or_else(|err| {
+        let cfg = Config::load_with_default(&source, default_config).unwrap_or_else(|err| {
             eprintln!("Failed to load config:\n  {}", err);
             std::process::exit(1);
         });
@@ -90,13 +89,6 @@ async fn drun_main() -> Result<(), String> {
             })
             .unwrap_or(SubnetType::System);
 
-        let use_old_metering = matches.get_flag(USE_OLD_METERING);
-        cfg.hypervisor.embedders_config.metering_type = if use_old_metering {
-            MeteringType::Old
-        } else {
-            MeteringType::New
-        };
-
         let uo = DrunOptions {
             msg_filename: matches.value_of(ARG_MESSAGES).unwrap().to_string(),
             cfg,
@@ -167,11 +159,5 @@ fn get_arg_matches() -> ArgMatches {
                 .value_name("Subnet Type")
                 .takes_value(true),
         )
-        .arg(
-            Arg::new(USE_OLD_METERING)
-                .long(USE_OLD_METERING)
-                .help("Enable the old metering in the local canister execution environment.")
-                .action(ArgAction::SetTrue),
-        )
         .get_matches()
 }

rs/embedders/src/wasm_utils/instrumentation.rs

@@ -163,21 +163,6 @@ impl InjectedImports {
 
 // Gets the cost of an instruction.
 pub fn instruction_to_cost(i: &Operator) -> u64 {
-    match i {
-        // The following instructions are mostly signaling the start/end of code blocks,
-        // so we assign 0 cost to them.
-        Operator::Block { .. } => 0,
-        Operator::Else => 0,
-        Operator::End => 0,
-        Operator::Loop { .. } => 0,
-
-        // Default cost of an instruction is 1.
-        _ => 1,
-    }
-}
-
-// Gets the cost of an instruction.
-pub fn instruction_to_cost_new(i: &Operator) -> u64 {
     // This aims to be a complete list of all instructions that can be executed, with certain exceptions.
     // The exceptions are: SIMD instructions, atomic instructions, and the dynamic cost of
     // of operations such as table/memory fill, copy, init. This
@@ -788,7 +773,6 @@ pub(super) fn instrument(
             special_indices,
             subnet_type,
             dirty_page_overhead,
-            metering_type,
         )
     }
 
@@ -872,7 +856,6 @@ fn replace_system_api_functions(
     special_indices: SpecialIndices,
     subnet_type: SubnetType,
     dirty_page_overhead: NumInstructions,
-    metering_type: MeteringType,
 ) {
     let api_indexes = calculate_api_indexes(module);
     let number_of_func_imports = module
@@ -884,12 +867,9 @@ fn replace_system_api_functions(
     // Collect a single map of all the function indexes that need to be
     // replaced.
     let mut func_index_replacements = BTreeMap::new();
-    for (api, (ty, body)) in replacement_functions(
-        special_indices,
-        subnet_type,
-        dirty_page_overhead,
-        metering_type,
-    ) {
+    for (api, (ty, body)) in
+        replacement_functions(special_indices, subnet_type, dirty_page_overhead)
+    {
         if let Some(old_index) = api_indexes.get(&api) {
             let type_idx = add_func_type(module, ty);
             let new_index = (number_of_func_imports + module.functions.len()) as u32;
@@ -1198,9 +1178,8 @@ fn inject_metering(
     metering_type: MeteringType,
 ) {
     let points = match metering_type {
-        MeteringType::Old => injections_old(code),
         MeteringType::None => Vec::new(),
-        MeteringType::New => injections_new(code),
+        MeteringType::New => injections(code),
     };
     let points = points.iter().filter(|point| match point.cost_detail {
         InjectionPointCostDetail::StaticCost {
@@ -1531,79 +1510,20 @@ fn inject_try_grow_wasm_memory(
     }
 }
 
-// This function scans through the Wasm code and creates an injection point
-// at the beginning of every basic block (straight-line sequence of instructions
-// with no branches) and before each bulk memory instruction. An injection point
-// contains a "hint" about the context of every basic block, specifically if
-// it's re-entrant or not. This version over-estimates the cost of code with
-// returns and jumps.
-fn injections_old(code: &[Operator]) -> Vec<InjectionPoint> {
-    let mut res = Vec::new();
-    let mut stack = Vec::new();
-    use Operator::*;
-    // The function itself is a re-entrant code block.
-    let mut curr = InjectionPoint::new_static_cost(0, Scope::ReentrantBlockStart, 0);
-    for (position, i) in code.iter().enumerate() {
-        curr.cost_detail.increment_cost(instruction_to_cost(i));
-        match i {
-            // Start of a re-entrant code block.
-            Loop { .. } => {
-                stack.push(curr);
-                curr = InjectionPoint::new_static_cost(position + 1, Scope::ReentrantBlockStart, 0);
-            }
-            // Start of a non re-entrant code block.
-            If { .. } | Block { .. } => {
-                stack.push(curr);
-                curr =
-                    InjectionPoint::new_static_cost(position + 1, Scope::NonReentrantBlockStart, 0);
-            }
-            // End of a code block but still more code left.
-            Else | Br { .. } | BrIf { .. } | BrTable { .. } => {
-                res.push(curr);
-                curr = InjectionPoint::new_static_cost(position + 1, Scope::BlockEnd, 0);
-            }
-            // `End` signals the end of a code block. If there's nothing more on the stack, we've
-            // gone through all the code.
-            End => {
-                res.push(curr);
-                curr = match stack.pop() {
-                    Some(val) => val,
-                    None => break,
-                };
-            }
-            // Bulk memory instructions require injected metering __before__ the instruction
-            // executes so that size arguments can be read from the stack at runtime.
-            MemoryFill { .. }
-            | MemoryCopy { .. }
-            | MemoryInit { .. }
-            | TableCopy { .. }
-            | TableInit { .. }
-            | TableFill { .. } => {
-                res.push(InjectionPoint::new_dynamic_cost(position));
-            }
-            // Nothing special to be done for other instructions.
-            _ => (),
-        }
-    }
-
-    res.sort_by_key(|k| k.position);
-    res
-}
-
 // This function scans through the Wasm code and creates an injection point
 // at the beginning of every basic block (straight-line sequence of instructions
 // with no branches) and before each bulk memory instruction. An injection point
 // contains a "hint" about the context of every basic block, specifically if
 // it's re-entrant or not.
-fn injections_new(code: &[Operator]) -> Vec<InjectionPoint> {
+fn injections(code: &[Operator]) -> Vec<InjectionPoint> {
     let mut res = Vec::new();
     use Operator::*;
     // The function itself is a re-entrant code block.
     // Start with at least one fuel being consumed because even empty
     // functions should consume at least some fuel.
     let mut curr = InjectionPoint::new_static_cost(0, Scope::ReentrantBlockStart, 1);
     for (position, i) in code.iter().enumerate() {
-        curr.cost_detail.increment_cost(instruction_to_cost_new(i));
+        curr.cost_detail.increment_cost(instruction_to_cost(i));
         match i {
             // Start of a re-entrant code block.
             Loop { .. } => {

rs/embedders/src/wasm_utils/system_api_replacements.rs

@@ -12,10 +12,9 @@
 //!
 
 use crate::{
-    wasm_utils::instrumentation::InjectedImports, wasmtime_embedder::system_api_complexity,
-    InternalErrorCode,
+    wasm_utils::instrumentation::InjectedImports,
+    wasmtime_embedder::system_api_complexity::overhead_native, InternalErrorCode,
 };
-use ic_config::embedders::MeteringType;
 use ic_interfaces::execution_environment::StableMemoryApi;
 use ic_registry_subnet_type::SubnetType;
 use ic_sys::PAGE_SIZE;
@@ -26,15 +25,12 @@ use wasmtime_environ::WASM_PAGE_SIZE;
 
 use super::{instrumentation::SpecialIndices, SystemApiFunc};
 
-use crate::wasmtime_embedder::system_api_complexity::system_api;
-
 const MAX_32_BIT_STABLE_MEMORY_IN_PAGES: i64 = 64 * 1024; // 4GiB
 
 pub(super) fn replacement_functions(
     special_indices: SpecialIndices,
     subnet_type: SubnetType,
     dirty_page_overhead: NumInstructions,
-    metering_type: MeteringType,
 ) -> Vec<(SystemApiFunc, (FuncType, Body<'static>))> {
     let count_clean_pages_fn_index = special_indices.count_clean_pages_fn.unwrap();
     let dirty_pages_counter_index = special_indices.dirty_pages_counter_ix.unwrap();
@@ -243,11 +239,7 @@ pub(super) fn replacement_functions(
                             },
                             I64ExtendI32U,
                             I64Const {
-                                value: system_api::complexity_overhead_native!(
-                                    STABLE_READ,
-                                    metering_type
-                                )
-                                .get() as i64,
+                                value: overhead_native::STABLE_READ.get() as i64,
                             },
                             I64Add,
                             Call {
@@ -505,11 +497,7 @@ pub(super) fn replacement_functions(
                                 }
                             },
                             I64Const {
-                                value: system_api::complexity_overhead_native!(
-                                    STABLE64_READ,
-                                    metering_type
-                                )
-                                .get() as i64,
+                                value: overhead_native::STABLE64_READ.get() as i64,
                             },
                             I64Add,
                             Call {
@@ -795,11 +783,7 @@ pub(super) fn replacement_functions(
                             },
                             I64ExtendI32U,
                             I64Const {
-                                value: system_api::complexity_overhead_native!(
-                                    STABLE_WRITE,
-                                    metering_type
-                                )
-                                .get() as i64,
+                                value: overhead_native::STABLE_WRITE.get() as i64,
                             },
                             I64Add,
                             Call {
@@ -1026,11 +1010,7 @@ pub(super) fn replacement_functions(
                                 }
                             },
                             I64Const {
-                                value: system_api::complexity_overhead_native!(
-                                    STABLE64_WRITE,
-                                    metering_type
-                                )
-                                .get() as i64,
+                                value: overhead_native::STABLE64_WRITE.get() as i64,
                             },
                             I64Add,
                             Call {

rs/embedders/src/wasmtime_embedder.rs

@@ -244,7 +244,6 @@ impl WasmtimeEmbedder {
             self.config.feature_flags,
             self.config.stable_memory_dirty_page_limit,
             self.config.stable_memory_accessed_page_limit,
-            self.config.metering_type,
             main_memory_type,
         );