From b1b532a8fc2c8f5417337ec9da03b395b0d6f9fd Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sat, 14 Jan 2023 20:31:42 +0100 Subject: [PATCH 01/10] re-export inner field of Store to rest of the crate --- crates/wasmi/src/lib.rs | 2 +- crates/wasmi/src/store.rs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/wasmi/src/lib.rs b/crates/wasmi/src/lib.rs index a5388bbaf7..4986153355 100644 --- a/crates/wasmi/src/lib.rs +++ b/crates/wasmi/src/lib.rs @@ -158,6 +158,6 @@ use self::{ global::{GlobalEntity, GlobalIdx}, instance::{InstanceEntity, InstanceEntityBuilder, InstanceIdx}, memory::{MemoryEntity, MemoryIdx}, - store::Stored, + store::{StoreInner, Stored}, table::{TableEntity, TableIdx}, }; diff --git a/crates/wasmi/src/store.rs b/crates/wasmi/src/store.rs index 786570fe7a..a1d250054b 100644 --- a/crates/wasmi/src/store.rs +++ b/crates/wasmi/src/store.rs @@ -62,7 +62,12 @@ pub type Stored = GuardedEntity; #[derive(Debug)] pub struct Store { /// All data that is not associated to `T`. - inner: StoreInner, + /// + /// # Note + /// + /// This is re-exported to the rest of the crate since + /// it is used directly by the engine's executor. + pub(crate) inner: StoreInner, /// Stored Wasm or host functions. funcs: Arena>, /// User provided state. From c4d9a98555265f62722d8ce37e9df7a1465ccd9f Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sat, 14 Jan 2023 21:01:59 +0100 Subject: [PATCH 02/10] make engine cache use StoreInner This is in preparation to making the engine executor itself use StoreInner. --- crates/wasmi/src/engine/cache.rs | 69 ++++++++++++++--------------- crates/wasmi/src/engine/executor.rs | 17 ++++--- crates/wasmi/src/store.rs | 4 +- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/crates/wasmi/src/engine/cache.rs b/crates/wasmi/src/engine/cache.rs index fb3a575827..e56693972c 100644 --- a/crates/wasmi/src/engine/cache.rs +++ b/crates/wasmi/src/engine/cache.rs @@ -1,7 +1,6 @@ use crate::{ module::{DEFAULT_MEMORY_INDEX, DEFAULT_TABLE_INDEX}, - AsContext, - AsContextMut, + store::StoreInner, Func, Instance, Memory, @@ -69,10 +68,10 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a default linear memory. - fn load_default_memory(&mut self, ctx: impl AsContext) -> Memory { - let default_memory = self - .instance() - .get_memory(ctx.as_context(), DEFAULT_MEMORY_INDEX) + fn load_default_memory(&mut self, ctx: &StoreInner) -> Memory { + let default_memory = ctx + .resolve_instance(self.instance()) + .get_memory(DEFAULT_MEMORY_INDEX) .unwrap_or_else(|| { panic!( "missing default linear memory for instance: {:?}", @@ -88,10 +87,10 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a default table. - fn load_default_table(&mut self, ctx: impl AsContext) -> Table { - let default_table = self - .instance() - .get_table(ctx.as_context(), DEFAULT_TABLE_INDEX) + fn load_default_table(&mut self, ctx: &StoreInner) -> Table { + let default_table = ctx + .resolve_instance(self.instance()) + .get_table(DEFAULT_TABLE_INDEX) .unwrap_or_else(|| panic!("missing default table for instance: {:?}", self.instance)); self.default_table = Some(default_table); default_table @@ -102,7 +101,7 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a default linear memory. - pub fn default_memory(&mut self, ctx: impl AsContext) -> Memory { + pub fn default_memory(&mut self, ctx: &StoreInner) -> Memory { match self.default_memory { Some(default_memory) => default_memory, None => self.load_default_memory(ctx), @@ -114,7 +113,7 @@ impl InstanceCache { /// # Note /// /// This avoids one indirection compared to using the `default_memory`. - pub fn default_memory_bytes(&mut self, ctx: impl AsContextMut) -> &mut CachedMemoryBytes { + pub fn default_memory_bytes(&mut self, ctx: &mut StoreInner) -> &mut CachedMemoryBytes { match self.default_memory_bytes { Some(ref mut cached) => cached, None => self.load_default_memory_bytes(ctx), @@ -124,8 +123,8 @@ impl InstanceCache { /// Loads and populates the cached default memory instance. /// /// Returns an exclusive reference to the cached default memory. - fn load_default_memory_bytes(&mut self, ctx: impl AsContextMut) -> &mut CachedMemoryBytes { - let memory = self.default_memory(&ctx); + fn load_default_memory_bytes(&mut self, ctx: &mut StoreInner) -> &mut CachedMemoryBytes { + let memory = self.default_memory(ctx); self.default_memory_bytes = Some(CachedMemoryBytes::new(ctx, memory)); self.default_memory_bytes .as_mut() @@ -150,7 +149,7 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a default table. - pub fn default_table(&mut self, ctx: impl AsContext) -> Table { + pub fn default_table(&mut self, ctx: &StoreInner) -> Table { match self.default_table { Some(default_table) => default_table, None => self.load_default_table(ctx), @@ -162,10 +161,10 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a default table. - fn load_func_at(&mut self, ctx: impl AsContext, index: u32) -> Func { - let func = self - .instance() - .get_func(ctx.as_context(), index) + fn load_func_at(&mut self, ctx: &StoreInner, index: u32) -> Func { + let func = ctx + .resolve_instance(self.instance()) + .get_func(index) .unwrap_or_else(|| { panic!( "missing func at index {index} for instance: {:?}", @@ -181,7 +180,7 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a [`Func`] at the index. - pub fn get_func(&mut self, ctx: impl AsContext, func_idx: u32) -> Func { + pub fn get_func(&mut self, ctx: &StoreInner, func_idx: u32) -> Func { match self.last_func { Some((index, func)) if index == func_idx => func, _ => self.load_func_at(ctx, func_idx), @@ -194,19 +193,17 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a default table. - fn load_global_at(&mut self, ctx: impl AsContextMut, index: u32) -> NonNull { - let global = self - .instance() - .get_global(ctx.as_context(), index) - .map_or_else( - || { - panic!( - "missing global variable at index {index} for instance: {:?}", - self.instance - ) - }, - |g| g.get_untyped_ptr(ctx), - ); + fn load_global_at(&mut self, ctx: &mut StoreInner, index: u32) -> NonNull { + let global = ctx + .resolve_instance(self.instance()) + .get_global(index) + .map(|global| ctx.resolve_global_mut(global).get_untyped_ptr()) + .unwrap_or_else(|| { + panic!( + "missing global variable at index {index} for instance: {:?}", + self.instance + ) + }); self.last_global = Some((index, global)); global } @@ -217,7 +214,7 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a [`Func`] at the index. - pub fn get_global(&mut self, ctx: impl AsContextMut, global_idx: u32) -> &mut UntypedValue { + pub fn get_global(&mut self, ctx: &mut StoreInner, global_idx: u32) -> &mut UntypedValue { let mut ptr = match self.last_global { Some((index, global)) if index == global_idx => global, _ => self.load_global_at(ctx, global_idx), @@ -239,9 +236,9 @@ pub struct CachedMemoryBytes { impl CachedMemoryBytes { /// Creates a new [`CachedMemoryBytes`] from the given [`Memory`]. #[inline] - pub fn new(mut ctx: impl AsContextMut, memory: Memory) -> Self { + pub fn new(ctx: &mut StoreInner, memory: Memory) -> Self { Self { - data: memory.data_mut(&mut ctx).into(), + data: ctx.resolve_memory_mut(memory).data().into(), } } diff --git a/crates/wasmi/src/engine/executor.rs b/crates/wasmi/src/engine/executor.rs index 361c4da3aa..6ad820f8f0 100644 --- a/crates/wasmi/src/engine/executor.rs +++ b/crates/wasmi/src/engine/executor.rs @@ -288,7 +288,8 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { /// If there exists is no linear memory for the instance. #[inline] fn default_memory(&mut self) -> Memory { - self.cache.default_memory(self.ctx.as_context()) + self.cache + .default_memory(&self.ctx.as_context().store.inner) } /// Returns the default table. @@ -298,7 +299,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { /// If there exists is no table for the instance. #[inline] fn default_table(&mut self) -> Table { - self.cache.default_table(self.ctx.as_context()) + self.cache.default_table(&self.ctx.as_context().store.inner) } /// Returns the global variable at the given index. @@ -307,8 +308,10 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { /// /// If there is no global variable at the given index. fn global(&mut self, global_index: GlobalIdx) -> &mut UntypedValue { - self.cache - .get_global(self.ctx.as_context_mut(), global_index.into_inner()) + self.cache.get_global( + &mut self.ctx.as_context_mut().store.inner, + global_index.into_inner(), + ) } /// Executes a generic Wasm `store[N_{s|u}]` operation. @@ -332,7 +335,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { self.value_stack.try_eval_top(|address| { let memory = self .cache - .default_memory_bytes(self.ctx.as_context_mut()) + .default_memory_bytes(&mut self.ctx.as_context_mut().store.inner) .data(); let value = load_extend(memory, address, offset.into_inner())?; Ok(value) @@ -359,7 +362,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { let (address, value) = self.value_stack.pop2(); let memory = self .cache - .default_memory_bytes(self.ctx.as_context_mut()) + .default_memory_bytes(&mut self.ctx.as_context_mut().store.inner) .data_mut(); store_wrap(memory, address, offset.into_inner(), value)?; self.next_instr(); @@ -526,7 +529,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { fn visit_call(&mut self, func_index: FuncIdx) -> Result { let callee = self .cache - .get_func(self.ctx.as_context_mut(), func_index.into_inner()); + .get_func(&self.ctx.as_context().store.inner, func_index.into_inner()); self.call_func(callee) } diff --git a/crates/wasmi/src/store.rs b/crates/wasmi/src/store.rs index a1d250054b..0837c1d9e4 100644 --- a/crates/wasmi/src/store.rs +++ b/crates/wasmi/src/store.rs @@ -62,9 +62,9 @@ pub type Stored = GuardedEntity; #[derive(Debug)] pub struct Store { /// All data that is not associated to `T`. - /// + /// /// # Note - /// + /// /// This is re-exported to the rest of the crate since /// it is used directly by the engine's executor. pub(crate) inner: StoreInner, From 99990c13690191b7ef6739cc27caa2248eddd20b Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sat, 14 Jan 2023 21:10:26 +0100 Subject: [PATCH 03/10] use StoreInner from crate root --- crates/wasmi/src/engine/cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasmi/src/engine/cache.rs b/crates/wasmi/src/engine/cache.rs index e56693972c..70762321bf 100644 --- a/crates/wasmi/src/engine/cache.rs +++ b/crates/wasmi/src/engine/cache.rs @@ -1,9 +1,9 @@ use crate::{ module::{DEFAULT_MEMORY_INDEX, DEFAULT_TABLE_INDEX}, - store::StoreInner, Func, Instance, Memory, + StoreInner, Table, }; use core::ptr::NonNull; From fe435eaa3f37cecd352f602d710d84a08daed07e Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sat, 14 Jan 2023 21:10:58 +0100 Subject: [PATCH 04/10] remove no longer needed APIs These APIs were pub(crate) and just used by the engine before which is now using the StoreInner directly instead. --- crates/wasmi/src/global.rs | 8 ---- crates/wasmi/src/instance.rs | 39 -------------------- crates/wasmi/src/module/instantiate/tests.rs | 28 +++++++------- 3 files changed, 14 insertions(+), 61 deletions(-) diff --git a/crates/wasmi/src/global.rs b/crates/wasmi/src/global.rs index c1ffdd6985..e16cd686ea 100644 --- a/crates/wasmi/src/global.rs +++ b/crates/wasmi/src/global.rs @@ -292,12 +292,4 @@ impl Global { pub fn get(&self, ctx: impl AsContext) -> Value { ctx.as_context().store.resolve_global(*self).get() } - - /// Returns a pointer to the untyped value of the global variable. - pub(crate) fn get_untyped_ptr(&self, mut ctx: impl AsContextMut) -> NonNull { - ctx.as_context_mut() - .store - .resolve_global_mut(*self) - .get_untyped_ptr() - } } diff --git a/crates/wasmi/src/instance.rs b/crates/wasmi/src/instance.rs index 1fd529bd88..d169fc9f30 100644 --- a/crates/wasmi/src/instance.rs +++ b/crates/wasmi/src/instance.rs @@ -353,45 +353,6 @@ impl Instance { self.0 } - /// Returns the linear memory at the `index` if any. - /// - /// # Panics - /// - /// Panics if `store` does not own this [`Instance`]. - pub(crate) fn get_memory(&self, store: impl AsContext, index: u32) -> Option { - store - .as_context() - .store - .resolve_instance(*self) - .get_memory(index) - } - - /// Returns the table at the `index` if any. - /// - /// # Panics - /// - /// Panics if `store` does not own this [`Instance`]. - pub(crate) fn get_table(&self, store: impl AsContext, index: u32) -> Option { - store - .as_context() - .store - .resolve_instance(*self) - .get_table(index) - } - - /// Returns the global variable at the `index` if any. - /// - /// # Panics - /// - /// Panics if `store` does not own this [`Instance`]. - pub(crate) fn get_global(&self, store: impl AsContext, index: u32) -> Option { - store - .as_context() - .store - .resolve_instance(*self) - .get_global(index) - } - /// Returns the function at the `index` if any. /// /// # Panics diff --git a/crates/wasmi/src/module/instantiate/tests.rs b/crates/wasmi/src/module/instantiate/tests.rs index 76689bc153..ef5c3f4a34 100644 --- a/crates/wasmi/src/module/instantiate/tests.rs +++ b/crates/wasmi/src/module/instantiate/tests.rs @@ -35,8 +35,8 @@ fn instantiate_from_wat(wat: &str) -> (Store<()>, Instance) { } fn assert_no_duplicates(store: &Store<()>, instance: Instance) { - assert!(instance.get_memory(store, 1).is_none()); - assert!(instance.get_table(store, 1).is_none()); + assert!(store.resolve_instance(instance).get_memory(1).is_none()); + assert!(store.resolve_instance(instance).get_table(1).is_none()); } #[test] @@ -47,8 +47,8 @@ fn test_import_memory_and_table() { (import "env" "table" (table 4 funcref)) )"#; let (store, instance) = instantiate_from_wat(wat); - assert!(instance.get_memory(&store, 0).is_some()); - assert!(instance.get_table(&store, 0).is_some()); + assert!(store.resolve_instance(instance).get_memory(0).is_some()); + assert!(store.resolve_instance(instance).get_table(0).is_some()); assert_no_duplicates(&store, instance); } @@ -59,8 +59,8 @@ fn test_import_memory() { (import "env" "memory" (memory 4)) )"#; let (store, instance) = instantiate_from_wat(wat); - assert!(instance.get_memory(&store, 0).is_some()); - assert!(instance.get_table(&store, 0).is_none()); + assert!(store.resolve_instance(instance).get_memory(0).is_some()); + assert!(store.resolve_instance(instance).get_table(0).is_none()); assert_no_duplicates(&store, instance); } @@ -71,8 +71,8 @@ fn test_import_table() { (import "env" "table" (table 4 funcref)) )"#; let (store, instance) = instantiate_from_wat(wat); - assert!(instance.get_memory(&store, 0).is_none()); - assert!(instance.get_table(&store, 0).is_some()); + assert!(store.resolve_instance(instance).get_memory(0).is_none()); + assert!(store.resolve_instance(instance).get_table(0).is_some()); assert_no_duplicates(&store, instance); } @@ -80,8 +80,8 @@ fn test_import_table() { fn test_no_memory_no_table() { let wat = "(module)"; let (store, instance) = instantiate_from_wat(wat); - assert!(instance.get_memory(&store, 0).is_none()); - assert!(instance.get_table(&store, 0).is_none()); + assert!(store.resolve_instance(instance).get_memory(0).is_none()); + assert!(store.resolve_instance(instance).get_table(0).is_none()); assert_no_duplicates(&store, instance); } @@ -89,8 +89,8 @@ fn test_no_memory_no_table() { fn test_internal_memory() { let wat = "(module (memory 1 10) )"; let (store, instance) = instantiate_from_wat(wat); - assert!(instance.get_memory(&store, 0).is_some()); - assert!(instance.get_table(&store, 0).is_none()); + assert!(store.resolve_instance(instance).get_memory(0).is_some()); + assert!(store.resolve_instance(instance).get_table(0).is_none()); assert_no_duplicates(&store, instance); } @@ -98,7 +98,7 @@ fn test_internal_memory() { fn test_internal_table() { let wat = "(module (table 4 funcref) )"; let (store, instance) = instantiate_from_wat(wat); - assert!(instance.get_memory(&store, 0).is_none()); - assert!(instance.get_table(&store, 0).is_some()); + assert!(store.resolve_instance(instance).get_memory(0).is_none()); + assert!(store.resolve_instance(instance).get_table(0).is_some()); assert_no_duplicates(&store, instance); } From 7b6e0b3b1df1d704b14760a6ed3c895ab69189d5 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sat, 14 Jan 2023 23:08:40 +0100 Subject: [PATCH 05/10] add wasmi_arena::ComponentVec data structure --- crates/arena/src/component_vec.rs | 151 ++++++++++++++++++++++++++++++ crates/arena/src/lib.rs | 3 +- 2 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 crates/arena/src/component_vec.rs diff --git a/crates/arena/src/component_vec.rs b/crates/arena/src/component_vec.rs new file mode 100644 index 0000000000..7bc7be209a --- /dev/null +++ b/crates/arena/src/component_vec.rs @@ -0,0 +1,151 @@ +use crate::ArenaIndex; +use core::{ + fmt::{self, Debug}, + marker::PhantomData, + ops::{Index, IndexMut}, +}; + +/// Stores components for entities backed by a [`Vec`]. +pub struct ComponentVec { + components: Vec>, + marker: PhantomData Idx>, +} + +/// [`ComponentVec`] does not store `Idx` therefore it is `Send` without its bound. +unsafe impl Send for ComponentVec where T: Send {} + +/// [`ComponentVec`] does not store `Idx` therefore it is `Sync` without its bound. +unsafe impl Sync for ComponentVec where T: Send {} + +impl Debug for ComponentVec +where + T: Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ComponentVec") + .field("components", &DebugComponents(&self.components)) + .field("marker", &self.marker) + .finish() + } +} + +struct DebugComponents<'a, T>(&'a [Option]); + +impl<'a, T> Debug for DebugComponents<'a, T> +where + T: Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut map = f.debug_map(); + let components = self + .0 + .iter() + .enumerate() + .filter_map(|(n, component)| component.as_ref().map(|c| (n, c))); + for (idx, component) in components { + map.entry(&idx, component); + } + map.finish() + } +} + +impl Default for ComponentVec { + fn default() -> Self { + Self::new() + } +} + +impl PartialEq for ComponentVec +where + T: PartialEq, +{ + fn eq(&self, other: &Self) -> bool { + self.components.eq(&other.components) + } +} + +impl Eq for ComponentVec where T: Eq {} + +impl ComponentVec { + /// Creates a new empty [`ComponentVec`]. + pub fn new() -> Self { + Self { + components: Vec::new(), + marker: PhantomData, + } + } + + /// Clears all components from the [`ComponentVec`]. + pub fn clear(&mut self) { + self.components.clear(); + } +} + +impl ComponentVec +where + Idx: ArenaIndex, +{ + /// Sets the `component` for the entity at `index`. + /// + /// Returns the old component of the same entity if any. + pub fn set(&mut self, index: Idx, component: T) -> Option { + let index = index.into_usize(); + if index >= self.components.len() { + // The underlying vector does not have enough capacity + // and is required to be enlarged. + self.components.resize_with(index + 1, || None); + } + self.components[index].replace(component) + } + + /// Unsets the component for the entity at `index` and returns it if any. + pub fn unset(&mut self, index: Idx) -> Option { + self.components + .get_mut(index.into_usize()) + .and_then(Option::take) + } + + /// Returns a shared reference to the component at the `index` if any. + /// + /// Returns `None` if no component is stored under the `index`. + #[inline] + pub fn get(&self, index: Idx) -> Option<&T> { + self.components + .get(index.into_usize()) + .and_then(Option::as_ref) + } + + /// Returns an exclusive reference to the component at the `index` if any. + /// + /// Returns `None` if no component is stored under the `index`. + #[inline] + pub fn get_mut(&mut self, index: Idx) -> Option<&mut T> { + self.components + .get_mut(index.into_usize()) + .and_then(Option::as_mut) + } +} + +impl Index for ComponentVec +where + Idx: ArenaIndex, +{ + type Output = T; + + #[inline] + fn index(&self, index: Idx) -> &Self::Output { + self.get(index) + .unwrap_or_else(|| panic!("missing component at index: {}", index.into_usize())) + } +} + +impl IndexMut for ComponentVec +where + Idx: ArenaIndex, +{ + #[inline] + fn index_mut(&mut self, index: Idx) -> &mut Self::Output { + self.get_mut(index) + .unwrap_or_else(|| panic!("missing component at index: {}", index.into_usize())) + } +} diff --git a/crates/arena/src/lib.rs b/crates/arena/src/lib.rs index ae68740672..785dba07d1 100644 --- a/crates/arena/src/lib.rs +++ b/crates/arena/src/lib.rs @@ -22,13 +22,14 @@ extern crate alloc; #[cfg(feature = "std")] extern crate std as alloc; +mod component_vec; mod dedup; mod guarded; #[cfg(test)] mod tests; -pub use self::{dedup::DedupArena, guarded::GuardedEntity}; +pub use self::{component_vec::ComponentVec, dedup::DedupArena, guarded::GuardedEntity}; use alloc::vec::Vec; use core::{ iter::{DoubleEndedIterator, Enumerate, ExactSizeIterator}, From 6b2dc7cffed76514304eef0410b599be33135404 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sat, 14 Jan 2023 23:10:41 +0100 Subject: [PATCH 06/10] make Engine executor use &mut StoreInner This step required us to save a mapping from Func -> DedupFuncType in the StoreInner itself. So this information is duplicated now between StoreInner and Store since Store already stores a mapping from Func -> FuncEntity where FuncEntity has information about the underlying DedupFuncType. I think this minor information duplication is O.K. for the big gain of a non-generic Engine executor. --- crates/wasmi/src/engine/executor.rs | 69 ++++++++++++++--------------- crates/wasmi/src/instance.rs | 13 ------ crates/wasmi/src/store.rs | 42 ++++++++++++++++-- 3 files changed, 72 insertions(+), 52 deletions(-) diff --git a/crates/wasmi/src/engine/executor.rs b/crates/wasmi/src/engine/executor.rs index 6ad820f8f0..290ef7dfd5 100644 --- a/crates/wasmi/src/engine/executor.rs +++ b/crates/wasmi/src/engine/executor.rs @@ -10,7 +10,7 @@ use super::{ FuncFrame, ValueStack, }; -use crate::{core::TrapCode, AsContext, Func, StoreContextMut}; +use crate::{core::TrapCode, Func, StoreInner}; use core::cmp; use wasmi_core::{Pages, UntypedValue}; @@ -31,7 +31,13 @@ pub fn execute_frame<'engine>( cache: &'engine mut InstanceCache, frame: &mut FuncFrame, ) -> Result { - Executor::new(value_stack, ctx.as_context_mut(), cache, frame).execute() + Executor::new( + value_stack, + &mut ctx.as_context_mut().store.inner, + cache, + frame, + ) + .execute() } /// The function signature of Wasm load operations. @@ -48,27 +54,27 @@ type WasmStoreOp = fn( /// An execution context for executing a `wasmi` function frame. #[derive(Debug)] -struct Executor<'ctx, 'engine, 'func, HostData> { +struct Executor<'ctx, 'engine, 'func> { /// The pointer to the currently executed instruction. ip: InstructionPtr, /// Stores the value stack of live values on the Wasm stack. value_stack: ValueStackRef<'engine>, - /// A mutable [`Store`] context. + /// A mutable [`StoreInner`] context. /// - /// [`Store`]: [`crate::Store`] - ctx: StoreContextMut<'ctx, HostData>, + /// [`StoreInner`]: [`crate::StoreInner`] + ctx: &'ctx mut StoreInner, /// Stores frequently used instance related data. cache: &'engine mut InstanceCache, /// The function frame that is being executed. frame: &'func mut FuncFrame, } -impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { +impl<'ctx, 'engine, 'func> Executor<'ctx, 'engine, 'func> { /// Creates a new [`Executor`] for executing a `wasmi` function frame. #[inline(always)] pub fn new( value_stack: &'engine mut ValueStack, - ctx: StoreContextMut<'ctx, HostData>, + ctx: &'ctx mut StoreInner, cache: &'engine mut InstanceCache, frame: &'func mut FuncFrame, ) -> Self { @@ -288,8 +294,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { /// If there exists is no linear memory for the instance. #[inline] fn default_memory(&mut self) -> Memory { - self.cache - .default_memory(&self.ctx.as_context().store.inner) + self.cache.default_memory(self.ctx) } /// Returns the default table. @@ -299,7 +304,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { /// If there exists is no table for the instance. #[inline] fn default_table(&mut self) -> Table { - self.cache.default_table(&self.ctx.as_context().store.inner) + self.cache.default_table(self.ctx) } /// Returns the global variable at the given index. @@ -308,10 +313,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { /// /// If there is no global variable at the given index. fn global(&mut self, global_index: GlobalIdx) -> &mut UntypedValue { - self.cache.get_global( - &mut self.ctx.as_context_mut().store.inner, - global_index.into_inner(), - ) + self.cache.get_global(self.ctx, global_index.into_inner()) } /// Executes a generic Wasm `store[N_{s|u}]` operation. @@ -333,10 +335,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { load_extend: WasmLoadOp, ) -> Result<(), TrapCode> { self.value_stack.try_eval_top(|address| { - let memory = self - .cache - .default_memory_bytes(&mut self.ctx.as_context_mut().store.inner) - .data(); + let memory = self.cache.default_memory_bytes(self.ctx).data(); let value = load_extend(memory, address, offset.into_inner())?; Ok(value) })?; @@ -360,10 +359,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { store_wrap: WasmStoreOp, ) -> Result<(), TrapCode> { let (address, value) = self.value_stack.pop2(); - let memory = self - .cache - .default_memory_bytes(&mut self.ctx.as_context_mut().store.inner) - .data_mut(); + let memory = self.cache.default_memory_bytes(self.ctx).data_mut(); store_wrap(memory, address, offset.into_inner(), value)?; self.next_instr(); Ok(()) @@ -441,7 +437,7 @@ pub enum MaybeReturn { Continue, } -impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { +impl<'ctx, 'engine, 'func> Executor<'ctx, 'engine, 'func> { fn visit_unreachable(&mut self) -> Result<(), TrapCode> { Err(TrapCode::UnreachableCodeReached).map_err(Into::into) } @@ -527,9 +523,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { } fn visit_call(&mut self, func_index: FuncIdx) -> Result { - let callee = self - .cache - .get_func(&self.ctx.as_context().store.inner, func_index.into_inner()); + let callee = self.cache.get_func(self.ctx, func_index.into_inner()); self.call_func(callee) } @@ -539,15 +533,17 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { ) -> Result { let func_index: u32 = self.value_stack.pop_as(); let table = self.default_table(); - let func = table - .get(self.ctx.as_context(), func_index as usize) + let func = self + .ctx + .resolve_table(table) + .get(func_index as usize) .map_err(|_| TrapCode::TableOutOfBounds)? .ok_or(TrapCode::IndirectCallToNull)?; - let actual_signature = func.signature(self.ctx.as_context()); + let actual_signature = self.ctx.get_func_type(func); let expected_signature = self - .frame - .instance() - .get_signature(self.ctx.as_context(), signature_index.into_inner()) + .ctx + .resolve_instance(self.frame.instance()) + .get_signature(signature_index.into_inner()) .unwrap_or_else(|| { panic!("missing signature for call_indirect at index: {signature_index:?}") }); @@ -581,7 +577,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { fn visit_current_memory(&mut self) { let memory = self.default_memory(); - let result: u32 = memory.current_pages(self.ctx.as_context()).into(); + let result: u32 = self.ctx.resolve_memory(memory).current_pages().into(); self.value_stack.push(result); self.next_instr() } @@ -592,8 +588,9 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { const ERR_VALUE: u32 = u32::MAX; let memory = self.default_memory(); let result = Pages::new(self.value_stack.pop_as()).map_or(ERR_VALUE, |additional| { - memory - .grow(self.ctx.as_context_mut(), additional) + self.ctx + .resolve_memory_mut(memory) + .grow(additional) .map(u32::from) .unwrap_or(ERR_VALUE) }); diff --git a/crates/wasmi/src/instance.rs b/crates/wasmi/src/instance.rs index d169fc9f30..add98aa624 100644 --- a/crates/wasmi/src/instance.rs +++ b/crates/wasmi/src/instance.rs @@ -366,19 +366,6 @@ impl Instance { .get_func(index) } - /// Returns the signature at the `index` if any. - /// - /// # Panics - /// - /// Panics if `store` does not own this [`Instance`]. - pub(crate) fn get_signature(&self, store: impl AsContext, index: u32) -> Option { - store - .as_context() - .store - .resolve_instance(*self) - .get_signature(index) - } - /// Returns the value exported to the given `name` if any. /// /// # Panics diff --git a/crates/wasmi/src/store.rs b/crates/wasmi/src/store.rs index 0837c1d9e4..f82180fd2d 100644 --- a/crates/wasmi/src/store.rs +++ b/crates/wasmi/src/store.rs @@ -22,7 +22,7 @@ use core::{ fmt::Debug, sync::atomic::{AtomicU32, Ordering}, }; -use wasmi_arena::{Arena, ArenaIndex, GuardedEntity}; +use wasmi_arena::{Arena, ArenaIndex, ComponentVec, GuardedEntity}; /// A unique store index. /// @@ -81,6 +81,13 @@ pub struct StoreInner { /// /// Used to protect against invalid entity indices. store_idx: StoreIdx, + /// Stores the function type for each function. + /// + /// # Note + /// + /// This is required so that the [`Engine`] can work entirely + /// with a `&mut StoreInner` reference. + func_types: ComponentVec, /// Stored linear memories. memories: Arena, /// Stored tables. @@ -111,6 +118,7 @@ impl StoreInner { StoreInner { engine: engine.clone(), store_idx: StoreIdx::new(), + func_types: ComponentVec::new(), memories: Arena::new(), tables: Arena::new(), globals: Arena::new(), @@ -159,6 +167,31 @@ impl StoreInner { }) } + /// Registers the `func_type` for the given `func`. + /// + /// # Note + /// + /// This is required so that the [`Engine`] can work entirely + /// with a `&mut StoreInner` reference. + pub fn register_func_type(&mut self, func: Func, func_type: DedupFuncType) { + let idx = self.unwrap_stored(func.into_inner()); + let previous = self.func_types.set(idx, func_type); + debug_assert!(previous.is_none()); + } + + /// Returns the [`DedupFuncType`] for the given [`Func`]. + /// + /// # Note + /// + /// Panics if no [`DedupFuncType`] for the given [`Func`] was registered. + pub fn get_func_type(&self, func: Func) -> DedupFuncType { + let idx = self.unwrap_stored(func.into_inner()); + self.func_types + .get(idx) + .copied() + .unwrap_or_else(|| panic!("missing function type for func: {:?}", func)) + } + /// Allocates a new [`GlobalEntity`] and returns a [`Global`] reference to it. pub fn alloc_global(&mut self, global: GlobalEntity) -> Global { let global = self.globals.alloc(global); @@ -435,8 +468,11 @@ impl Store { /// Allocates a new Wasm or host [`FuncEntity`] and returns a [`Func`] reference to it. pub(super) fn alloc_func(&mut self, func: FuncEntity) -> Func { - let func = self.funcs.alloc(func); - Func::from_inner(self.wrap_stored(func)) + let func_type = func.signature(); + let idx = self.funcs.alloc(func); + let func = Func::from_inner(self.wrap_stored(idx)); + self.inner.register_func_type(func, func_type); + func } /// Allocates a new uninitialized [`InstanceEntity`] and returns an [`Instance`] reference to it. From b718ba0d5bcc9a19f75773ad59cbbac9d755f97b Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sat, 14 Jan 2023 23:25:09 +0100 Subject: [PATCH 07/10] add missing import for no_std envs --- crates/arena/src/component_vec.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/arena/src/component_vec.rs b/crates/arena/src/component_vec.rs index 7bc7be209a..6c60939920 100644 --- a/crates/arena/src/component_vec.rs +++ b/crates/arena/src/component_vec.rs @@ -1,4 +1,5 @@ use crate::ArenaIndex; +use alloc::vec::Vec; use core::{ fmt::{self, Debug}, marker::PhantomData, From 08a43034d2c244186dbc113f6bc424a4f8491add Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sat, 14 Jan 2023 23:26:10 +0100 Subject: [PATCH 08/10] apply clippy suggestion --- crates/wasmi/src/store.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasmi/src/store.rs b/crates/wasmi/src/store.rs index f82180fd2d..b4f0cedb79 100644 --- a/crates/wasmi/src/store.rs +++ b/crates/wasmi/src/store.rs @@ -189,7 +189,7 @@ impl StoreInner { self.func_types .get(idx) .copied() - .unwrap_or_else(|| panic!("missing function type for func: {:?}", func)) + .unwrap_or_else(|| panic!("missing function type for func: {func:?}")) } /// Allocates a new [`GlobalEntity`] and returns a [`Global`] reference to it. From c12b003980dd6081af47fb30c7917cbbef8deb4a Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sun, 15 Jan 2023 00:25:55 +0100 Subject: [PATCH 09/10] move StoreInner conversion a bit --- crates/wasmi/src/engine/executor.rs | 11 ++--------- crates/wasmi/src/engine/mod.rs | 10 ++++++++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/wasmi/src/engine/executor.rs b/crates/wasmi/src/engine/executor.rs index 290ef7dfd5..4ac6651a72 100644 --- a/crates/wasmi/src/engine/executor.rs +++ b/crates/wasmi/src/engine/executor.rs @@ -4,7 +4,6 @@ use super::{ cache::InstanceCache, code_map::InstructionPtr, stack::ValueStackRef, - AsContextMut, CallOutcome, DropKeep, FuncFrame, @@ -26,18 +25,12 @@ use wasmi_core::{Pages, UntypedValue}; /// - If the execution of the function `frame` trapped. #[inline(always)] pub fn execute_frame<'engine>( - mut ctx: impl AsContextMut, + ctx: &mut StoreInner, value_stack: &'engine mut ValueStack, cache: &'engine mut InstanceCache, frame: &mut FuncFrame, ) -> Result { - Executor::new( - value_stack, - &mut ctx.as_context_mut().store.inner, - cache, - frame, - ) - .execute() + Executor::new(value_stack, ctx, cache, frame).execute() } /// The function signature of Wasm load operations. diff --git a/crates/wasmi/src/engine/mod.rs b/crates/wasmi/src/engine/mod.rs index 95a82f59e8..e4467879d1 100644 --- a/crates/wasmi/src/engine/mod.rs +++ b/crates/wasmi/src/engine/mod.rs @@ -714,7 +714,7 @@ impl<'engine> EngineExecutor<'engine> { #[inline(always)] fn execute_frame( &mut self, - ctx: impl AsContextMut, + mut ctx: impl AsContextMut, frame: &mut FuncFrame, cache: &mut InstanceCache, ) -> Result { @@ -729,6 +729,12 @@ impl<'engine> EngineExecutor<'engine> { } let value_stack = &mut self.stack.values; - execute_frame(ctx, value_stack, cache, frame).map_err(make_trap) + execute_frame( + &mut ctx.as_context_mut().store.inner, + value_stack, + cache, + frame, + ) + .map_err(make_trap) } } From 31199562e131d56349ac965fa75bb0401412c470 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sun, 15 Jan 2023 09:28:00 +0100 Subject: [PATCH 10/10] add some #[inline] annotations to InstanceCache methods --- crates/wasmi/src/engine/cache.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/wasmi/src/engine/cache.rs b/crates/wasmi/src/engine/cache.rs index 70762321bf..167a6b67d3 100644 --- a/crates/wasmi/src/engine/cache.rs +++ b/crates/wasmi/src/engine/cache.rs @@ -101,6 +101,7 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a default linear memory. + #[inline] pub fn default_memory(&mut self, ctx: &StoreInner) -> Memory { match self.default_memory { Some(default_memory) => default_memory, @@ -113,6 +114,7 @@ impl InstanceCache { /// # Note /// /// This avoids one indirection compared to using the `default_memory`. + #[inline] pub fn default_memory_bytes(&mut self, ctx: &mut StoreInner) -> &mut CachedMemoryBytes { match self.default_memory_bytes { Some(ref mut cached) => cached, @@ -140,6 +142,7 @@ impl InstanceCache { /// - Conservatively it is also recommended to reset default memory bytes /// when calling a host function since that might invalidate linear memory /// without the Wasm engine knowing. + #[inline] pub fn reset_default_memory_bytes(&mut self) { self.default_memory_bytes = None; } @@ -149,6 +152,7 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a default table. + #[inline] pub fn default_table(&mut self, ctx: &StoreInner) -> Table { match self.default_table { Some(default_table) => default_table, @@ -180,6 +184,7 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a [`Func`] at the index. + #[inline] pub fn get_func(&mut self, ctx: &StoreInner, func_idx: u32) -> Func { match self.last_func { Some((index, func)) if index == func_idx => func, @@ -214,6 +219,7 @@ impl InstanceCache { /// # Panics /// /// If the currently used [`Instance`] does not have a [`Func`] at the index. + #[inline] pub fn get_global(&mut self, ctx: &mut StoreInner, global_idx: u32) -> &mut UntypedValue { let mut ptr = match self.last_global { Some((index, global)) if index == global_idx => global,