From 6dbbada8099511e538fb2a26e583012df0c90b5c Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Mon, 6 Mar 2023 12:18:22 +0100 Subject: [PATCH] Add fuel consumption modes (#706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add fuel consumption mode to Config * make Config::wasm_features crate private This API was never intened to be public. * add doc note to Config::fuel_consumption_mode * implement both fuel consumption modes in executor * fix internal doc link * add tests for fuel consumption modes * Update crates/wasmi/src/engine/config.rs Co-authored-by: Alexander Theißen * try fix performance regressions * add some inline annotations in executor * deduplicate some code * Revert "deduplicate some code" This reverts commit 165d94dce4187cc7e7eb57d6534ef25e8a188f4e. * try to fix perf regressions (take 2) * remove some inline annotations * refactor code * refactor code (split into more funcs) * apply #[cold] attribute * Revert "apply #[cold] attribute" This reverts commit 13e017e835bd9aad5d01418ba9743e17d23ddd52. * try fix regression (take 3) * replace inlines * remove cold attribute again * replace some inline(always) with inline * Revert "replace some inline(always) with inline" This reverts commit 482abc16893e3220176386ec65247eeec2f33a19. * add cold attribute again ... * Revert "add cold attribute again ..." This reverts commit 925cc223115ced7594796e1fb62651898bb7bca0. * put inline on all UntypedValue public methods * Revert "put inline on all UntypedValue public methods" This reverts commit df19822e91624e0f6afdaa74ceaabfc699d43f76. * put inline(always) on ret method * refactor code * apply rustfmt * try to fix Wasm regressions * Revert "try to fix Wasm regressions" This reverts commit 267666f65b32e3c94c46f7ea694661b5ea35d621. * deduplicate some control flow code in executor * apply rustfmt * use Default impl * try to mutate inline annotations a bit * adjust inline * use inline * remove one inline * next try * go back to stage 1 ... --------- Co-authored-by: Alexander Theißen --- crates/wasmi/src/engine/config.rs | 75 +++++++- crates/wasmi/src/engine/executor.rs | 170 +++++++++++------- crates/wasmi/src/engine/mod.rs | 2 +- crates/wasmi/src/lib.rs | 1 + .../tests/e2e/v1/fuel_consumption_mode.rs | 92 ++++++++++ crates/wasmi/tests/e2e/v1/mod.rs | 1 + 6 files changed, 275 insertions(+), 66 deletions(-) create mode 100644 crates/wasmi/tests/e2e/v1/fuel_consumption_mode.rs diff --git a/crates/wasmi/src/engine/config.rs b/crates/wasmi/src/engine/config.rs index 0bb076007c..3245fff6b5 100644 --- a/crates/wasmi/src/engine/config.rs +++ b/crates/wasmi/src/engine/config.rs @@ -33,10 +33,60 @@ pub struct Config { floats: bool, /// Is `true` if `wasmi` executions shall consume fuel. consume_fuel: bool, + /// The fuel consumption mode of the `wasmi` [`Engine`](crate::Engine). + fuel_consumption_mode: FuelConsumptionMode, /// The configured fuel costs of all `wasmi` bytecode instructions. fuel_costs: FuelCosts, } +/// The fuel consumption mode of the `wasmi` [`Engine`]. +/// +/// This mode affects when fuel is charged for Wasm bulk-operations. +/// Affected Wasm instructions are: +/// +/// - `memory.{grow, copy, fill}` +/// - `data.init` +/// - `table.{grow, copy, fill}` +/// - `element.init` +/// +/// The default fuel consumption mode is [`FuelConsumptionMode::Lazy`]. +/// +/// [`Engine`]: crate::Engine +#[derive(Debug, Default, Copy, Clone)] +pub enum FuelConsumptionMode { + /// Fuel consumption for bulk-operations is lazy. + /// + /// Lazy fuel consumption means that fuel for bulk-operations + /// is checked before executing the instruction but only consumed + /// if the executed instruction suceeded. The reason for this is + /// that bulk-operations fail fast and therefore do not cost + /// a lot of compute power in case of failure. + /// + /// # Note + /// + /// Lazy fuel consumption makes sense as default mode since the + /// affected bulk-operations usually are very costly if they are + /// successful. Therefore users generally want to avoid having to + /// using more fuel than what was actually used, especially if there + /// is an underlying cost model associated to the used fuel. + #[default] + Lazy, + /// Fuel consumption for bulk-operations is eager. + /// + /// Eager fuel consumption means that fuel for bulk-operations + /// is always consumed before executing the instruction independent + /// of it suceeding or failing. + /// + /// # Note + /// + /// A use case for when a user might prefer eager fuel consumption + /// is when the fuel **required** to perform an execution should be identical + /// to the actual fuel **consumed** by an execution. Otherwise it can be confusing + /// that the execution consumed `x` gas while it needs `x + gas_for_bulk_op` to + /// not run out of fuel. + Eager, +} + /// Type storing all kinds of fuel costs of instructions. #[derive(Debug, Copy, Clone)] pub struct FuelCosts { @@ -154,6 +204,7 @@ impl Default for Config { floats: true, consume_fuel: false, fuel_costs: FuelCosts::default(), + fuel_consumption_mode: FuelConsumptionMode::default(), } } } @@ -312,8 +363,30 @@ impl Config { &self.fuel_costs } + /// Configures the [`FuelConsumptionMode`] for the [`Engine`]. + /// + /// # Note + /// + /// This has no effect if fuel metering is disabled for the [`Engine`]. + /// + /// [`Engine`]: crate::Engine + pub fn fuel_consumption_mode(&mut self, mode: FuelConsumptionMode) -> &mut Self { + self.fuel_consumption_mode = mode; + self + } + + /// Returns the [`FuelConsumptionMode`] for the [`Engine`]. + /// + /// Returns `None` if fuel metering is disabled for the [`Engine`]. + /// + /// [`Engine`]: crate::Engine + pub(crate) fn get_fuel_consumption_mode(&self) -> Option { + self.get_consume_fuel() + .then_some(self.fuel_consumption_mode) + } + /// Returns the [`WasmFeatures`] represented by the [`Config`]. - pub fn wasm_features(&self) -> WasmFeatures { + pub(crate) fn wasm_features(&self) -> WasmFeatures { WasmFeatures { multi_value: self.multi_value, mutable_global: self.mutable_global, diff --git a/crates/wasmi/src/engine/executor.rs b/crates/wasmi/src/engine/executor.rs index dcb03fae0e..213819a1c4 100644 --- a/crates/wasmi/src/engine/executor.rs +++ b/crates/wasmi/src/engine/executor.rs @@ -23,6 +23,7 @@ use crate::{ }, func::FuncEntity, table::TableEntity, + FuelConsumptionMode, Func, FuncRef, Instance, @@ -164,6 +165,21 @@ struct Executor<'ctx, 'engine> { code_map: &'engine CodeMap, } +macro_rules! forward_call { + ($expr:expr) => {{ + if let CallOutcome::Call { + host_func, + instance, + } = $expr? + { + return Ok(WasmOutcome::Call { + host_func, + instance, + }); + } + }}; +} + impl<'ctx, 'engine> Executor<'ctx, 'engine> { /// Creates a new [`Executor`] for executing a `wasmi` function frame. #[inline(always)] @@ -214,56 +230,18 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { } } Instr::ReturnCall { drop_keep, func } => { - if let CallOutcome::Call { - host_func, - instance, - } = self.visit_return_call(drop_keep, func)? - { - return Ok(WasmOutcome::Call { - host_func, - instance, - }); - } + forward_call!(self.visit_return_call(drop_keep, func)) } Instr::ReturnCallIndirect { drop_keep, table, func_type, } => { - if let CallOutcome::Call { - host_func, - instance, - } = self.visit_return_call_indirect(drop_keep, table, func_type)? - { - return Ok(WasmOutcome::Call { - host_func, - instance, - }); - } - } - Instr::Call(func) => { - if let CallOutcome::Call { - host_func, - instance, - } = self.visit_call(func)? - { - return Ok(WasmOutcome::Call { - host_func, - instance, - }); - } + forward_call!(self.visit_return_call_indirect(drop_keep, table, func_type)) } + Instr::Call(func) => forward_call!(self.visit_call(func)), Instr::CallIndirect { table, func_type } => { - if let CallOutcome::Call { - host_func, - instance, - } = self.visit_call_indirect(table, func_type)? - { - return Ok(WasmOutcome::Call { - host_func, - instance, - }); - } + forward_call!(self.visit_call_indirect(table, func_type)) } Instr::Drop => self.visit_drop(), Instr::Select => self.visit_select(), @@ -599,7 +577,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { /// /// This also modifies the stack as the caller would expect it /// and synchronizes the execution state with the outer structures. - #[inline] + #[inline(always)] fn ret(&mut self, drop_keep: DropKeep) -> ReturnOutcome { self.sp.drop_keep(drop_keep); self.sync_stack_ptr(); @@ -622,14 +600,12 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { /// for amount of required fuel determined by `delta` or if /// fuel metering is disabled. /// - /// Only if `exec` runs successfully and fuel metering - /// is enabled the fuel determined by `delta` is charged. - /// /// # Errors /// /// - If the [`StoreInner`] ran out of fuel. /// - If the `exec` closure traps. - fn consume_fuel_on_success( + #[inline(always)] + fn consume_fuel_with( &mut self, delta: impl FnOnce(&FuelCosts) -> u64, exec: impl FnOnce(&mut Self) -> Result, @@ -637,34 +613,100 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { where E: From, { - if !self.is_fuel_metering_enabled() { - return exec(self); + match self.get_fuel_consumption_mode() { + None => exec(self), + Some(mode) => self.consume_fuel_with_mode(mode, delta, exec), } - // At this point we know that fuel metering is enabled. + } + + /// Consume an amount of fuel specified by `delta` and executes `exec`. + /// + /// The `mode` determines when and if the fuel determined by `delta` is charged. + /// + /// # Errors + /// + /// - If the [`StoreInner`] ran out of fuel. + /// - If the `exec` closure traps. + #[inline(always)] + fn consume_fuel_with_mode( + &mut self, + mode: FuelConsumptionMode, + delta: impl FnOnce(&FuelCosts) -> u64, + exec: impl FnOnce(&mut Self) -> Result, + ) -> Result + where + E: From, + { let delta = delta(self.fuel_costs()); + match mode { + FuelConsumptionMode::Lazy => self.consume_fuel_with_lazy(delta, exec), + FuelConsumptionMode::Eager => self.consume_fuel_with_eager(delta, exec), + } + } + + /// Consume an amount of fuel specified by `delta` if `exec` succeeds. + /// + /// Prior to executing `exec` it is checked if enough fuel is remaining + /// determined by `delta`. The fuel is charged only after `exec` has been + /// finished successfully. + /// + /// # Errors + /// + /// - If the [`StoreInner`] ran out of fuel. + /// - If the `exec` closure traps. + #[inline(always)] + fn consume_fuel_with_lazy( + &mut self, + delta: u64, + exec: impl FnOnce(&mut Self) -> Result, + ) -> Result + where + E: From, + { self.ctx.fuel().sufficient_fuel(delta)?; let result = exec(self)?; self.ctx .fuel_mut() .consume_fuel(delta) - .unwrap_or_else(|error| { - panic!("remaining fuel has already been approved prior but encountered: {error}") - }); + .expect("remaining fuel has already been approved prior"); Ok(result) } - /// Returns `true` if fuel metering is enabled. - fn is_fuel_metering_enabled(&self) -> bool { - self.ctx.engine().config().get_consume_fuel() + /// Consume an amount of fuel specified by `delta` and executes `exec`. + /// + /// # Errors + /// + /// - If the [`StoreInner`] ran out of fuel. + /// - If the `exec` closure traps. + #[inline(always)] + fn consume_fuel_with_eager( + &mut self, + delta: u64, + exec: impl FnOnce(&mut Self) -> Result, + ) -> Result + where + E: From, + { + self.ctx.fuel_mut().consume_fuel(delta)?; + exec(self) } /// Returns a shared reference to the [`FuelCosts`] of the [`Engine`]. /// /// [`Engine`]: crate::Engine + #[inline] fn fuel_costs(&self) -> &FuelCosts { self.ctx.engine().config().fuel_costs() } + /// Returns the [`FuelConsumptionMode`] of the [`Engine`]. + /// + /// [`Engine`]: crate::Engine + #[inline] + fn get_fuel_consumption_mode(&self) -> Option { + self.ctx.engine().config().get_fuel_consumption_mode() + } + /// Executes a `call` or `return_call` instruction. #[inline(always)] fn execute_call( @@ -892,7 +934,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { return self.try_next_instr(); } }; - let result = self.consume_fuel_on_success( + let result = self.consume_fuel_with( |costs| { let delta_in_bytes = delta.to_bytes().unwrap_or(0) as u64; costs.fuel_for_bytes(delta_in_bytes) @@ -928,7 +970,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { let n = i32::from(n) as usize; let offset = i32::from(d) as usize; let byte = u8::from(val); - self.consume_fuel_on_success( + self.consume_fuel_with( |costs| costs.fuel_for_bytes(n as u64), |this| { let memory = this @@ -951,7 +993,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { let n = i32::from(n) as usize; let src_offset = i32::from(s) as usize; let dst_offset = i32::from(d) as usize; - self.consume_fuel_on_success( + self.consume_fuel_with( |costs| costs.fuel_for_bytes(n as u64), |this| { let data = this.cache.default_memory_bytes(this.ctx); @@ -976,7 +1018,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { let n = i32::from(n) as usize; let src_offset = i32::from(s) as usize; let dst_offset = i32::from(d) as usize; - self.consume_fuel_on_success( + self.consume_fuel_with( |costs| costs.fuel_for_bytes(n as u64), |this| { let (memory, data) = this @@ -1018,7 +1060,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { fn visit_table_grow(&mut self, table_index: TableIdx) -> Result<(), TrapCode> { let (init, delta) = self.sp.pop2(); let delta: u32 = delta.into(); - let result = self.consume_fuel_on_success( + let result = self.consume_fuel_with( |costs| costs.fuel_for_elements(u64::from(delta)), |this| { let table = this.cache.get_table(this.ctx, table_index); @@ -1043,7 +1085,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { let (i, val, n) = self.sp.pop3(); let dst: u32 = i.into(); let len: u32 = n.into(); - self.consume_fuel_on_success( + self.consume_fuel_with( |costs| costs.fuel_for_elements(u64::from(len)), |this| { let table = this.cache.get_table(this.ctx, table_index); @@ -1088,7 +1130,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { let len = u32::from(n); let src_index = u32::from(s); let dst_index = u32::from(d); - self.consume_fuel_on_success( + self.consume_fuel_with( |costs| costs.fuel_for_elements(u64::from(len)), |this| { // Query both tables and check if they are the same: @@ -1120,7 +1162,7 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> { let len = u32::from(n); let src_index = u32::from(s); let dst_index = u32::from(d); - self.consume_fuel_on_success( + self.consume_fuel_with( |costs| costs.fuel_for_elements(u64::from(len)), |this| { let (instance, table, element) = this diff --git a/crates/wasmi/src/engine/mod.rs b/crates/wasmi/src/engine/mod.rs index 8d1e598bba..b65adb25ce 100644 --- a/crates/wasmi/src/engine/mod.rs +++ b/crates/wasmi/src/engine/mod.rs @@ -18,7 +18,7 @@ mod tests; pub use self::{ bytecode::DropKeep, code_map::FuncBody, - config::Config, + config::{Config, FuelConsumptionMode}, func_builder::{ FuncBuilder, FuncTranslatorAllocations, diff --git a/crates/wasmi/src/lib.rs b/crates/wasmi/src/lib.rs index e468836e0c..0e17b8d653 100644 --- a/crates/wasmi/src/lib.rs +++ b/crates/wasmi/src/lib.rs @@ -121,6 +121,7 @@ pub use self::{ engine::{ Config, Engine, + FuelConsumptionMode, ResumableCall, ResumableInvocation, StackLimits, diff --git a/crates/wasmi/tests/e2e/v1/fuel_consumption_mode.rs b/crates/wasmi/tests/e2e/v1/fuel_consumption_mode.rs new file mode 100644 index 0000000000..4f8179e82c --- /dev/null +++ b/crates/wasmi/tests/e2e/v1/fuel_consumption_mode.rs @@ -0,0 +1,92 @@ +//! Tests to check if wasmi's fuel metering works as intended. + +use wasmi::{Config, Engine, FuelConsumptionMode, Func, Linker, Module, Store}; +use wasmi_core::Trap; + +/// Setup [`Engine`] and [`Store`] for fuel metering. +fn test_setup(mode: FuelConsumptionMode) -> (Store<()>, Linker<()>) { + let mut config = Config::default(); + config.consume_fuel(true).fuel_consumption_mode(mode); + let engine = Engine::new(&config); + let store = Store::new(&engine, ()); + let linker = Linker::new(&engine); + (store, linker) +} + +/// Converts the `wat` string source into `wasm` encoded byte. +fn wat2wasm(wat: &str) -> Vec { + wat::parse_str(wat).unwrap() +} + +/// Compiles the `wasm` encoded bytes into a [`Module`]. +/// +/// # Panics +/// +/// If an error occurred upon module compilation, validation or translation. +fn create_module(store: &Store<()>, bytes: &[u8]) -> Module { + Module::new(store.engine(), bytes).unwrap() +} + +/// Setup [`Store`] and [`Instance`] for fuel metering. +fn default_test_setup(mode: FuelConsumptionMode, wasm: &[u8]) -> (Store<()>, Func) { + let (mut store, linker) = test_setup(mode); + let module = create_module(&store, wasm); + let instance = linker + .instantiate(&mut store, &module) + .unwrap() + .start(&mut store) + .unwrap(); + let func = instance.get_func(&store, "test").unwrap(); + (store, func) +} + +/// Asserts the the call was successful. +/// +/// # Note +/// +/// We just check if the call succeeded, not if the results are correct. +/// That is to be determined by another kind of test. +fn assert_success(call_result: Result) { + assert!(call_result.is_ok()); + assert_eq!(call_result.unwrap(), -1); +} + +/// The test module exporting a function as `"test"`. +/// +/// # Note +/// +/// The module's `memory` has one pages minimum and one page maximum +/// and thus cannot grow. Therefore the `memory.grow` operation in +/// the `test` function will fail and only consume a small amount of +/// fuel in lazy mode but will still consume a large amount in eager +/// mode. +fn test_module() -> &'static str { + r#" + (module + (memory 1 1) + (func (export "test") (result i32) + (memory.grow (i32.const 1)) + ) + )"# +} + +fn check_consumption_mode(mode: FuelConsumptionMode, given_fuel: u64, consumed_fuel: u64) { + assert!(given_fuel >= consumed_fuel); + let wasm = wat2wasm(test_module()); + let (mut store, func) = default_test_setup(mode, &wasm); + let func = func.typed::<(), i32>(&store).unwrap(); + // Now add enough fuel, so execution should succeed. + store.add_fuel(given_fuel).unwrap(); // this is just enough fuel for a successful `memory.grow` + assert_success(func.call(&mut store, ())); + assert_eq!(store.fuel_consumed(), Some(consumed_fuel)); +} + +#[test] +fn lazy_consumption_mode() { + check_consumption_mode(FuelConsumptionMode::Lazy, 1030, 4); +} + +#[test] +fn eager_consumption_mode() { + check_consumption_mode(FuelConsumptionMode::Eager, 1030, 1028); +} diff --git a/crates/wasmi/tests/e2e/v1/mod.rs b/crates/wasmi/tests/e2e/v1/mod.rs index 1ed533dc55..1375eec594 100644 --- a/crates/wasmi/tests/e2e/v1/mod.rs +++ b/crates/wasmi/tests/e2e/v1/mod.rs @@ -1,3 +1,4 @@ +mod fuel_consumption_mode; mod fuel_metering; mod func; mod host_calls_wasm;