Skip to content

Commit

Permalink
Use references where possible in Store and StoreInner (#634)
Browse files Browse the repository at this point in the history
* use references where possible in Store and StoreInner

* apply clippy suggestions
  • Loading branch information
Robbepop committed Jan 25, 2023
1 parent 75d52f4 commit 77e574c
Show file tree
Hide file tree
Showing 18 changed files with 119 additions and 115 deletions.
21 changes: 11 additions & 10 deletions crates/wasmi/src/engine/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ pub struct InstanceCache {
default_memory_bytes: Option<CachedMemoryBytes>,
}

impl From<Instance> for InstanceCache {
fn from(instance: Instance) -> Self {
impl From<&'_ Instance> for InstanceCache {
fn from(instance: &Instance) -> Self {
Self {
instance,
instance: *instance,
default_memory: None,
default_table: None,
last_func: None,
Expand All @@ -41,13 +41,13 @@ impl From<Instance> for InstanceCache {

impl InstanceCache {
/// Resolves the instances.
fn instance(&self) -> Instance {
self.instance
fn instance(&self) -> &Instance {
&self.instance
}

/// Updates the cached [`Instance`].
fn set_instance(&mut self, instance: Instance) {
self.instance = instance;
fn set_instance(&mut self, instance: &Instance) {
self.instance = *instance;
self.default_memory = None;
self.default_table = None;
self.last_func = None;
Expand All @@ -56,7 +56,7 @@ impl InstanceCache {
}

/// Updates the currently used instance resetting all cached entities.
pub fn update_instance(&mut self, instance: Instance) {
pub fn update_instance(&mut self, instance: &Instance) {
if instance == self.instance() {
return;
}
Expand Down Expand Up @@ -127,7 +127,7 @@ impl InstanceCache {
/// Returns an exclusive reference to the cached default memory.
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 = Some(CachedMemoryBytes::new(ctx, &memory));
self.default_memory_bytes
.as_mut()
.expect("cached_memory was just set to Some")
Expand Down Expand Up @@ -202,6 +202,7 @@ impl InstanceCache {
let global = ctx
.resolve_instance(self.instance())
.get_global(index)
.as_ref()
.map(|global| ctx.resolve_global_mut(global).get_untyped_ptr())
.unwrap_or_else(|| {
panic!(
Expand Down Expand Up @@ -242,7 +243,7 @@ pub struct CachedMemoryBytes {
impl CachedMemoryBytes {
/// Creates a new [`CachedMemoryBytes`] from the given [`Memory`].
#[inline]
pub fn new(ctx: &mut StoreInner, memory: Memory) -> Self {
pub fn new(ctx: &mut StoreInner, memory: &Memory) -> Self {
Self {
data: ctx.resolve_memory_mut(memory).data().into(),
}
Expand Down
6 changes: 3 additions & 3 deletions crates/wasmi/src/engine/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ impl<'ctx, 'engine, 'func> Executor<'ctx, 'engine, 'func> {
let table = self.default_table();
let func = self
.ctx
.resolve_table(table)
.resolve_table(&table)
.get(func_index)
.map_err(|_| TrapCode::TableOutOfBounds)?
.ok_or(TrapCode::IndirectCallToNull)?;
Expand Down Expand Up @@ -570,7 +570,7 @@ impl<'ctx, 'engine, 'func> Executor<'ctx, 'engine, 'func> {

fn visit_current_memory(&mut self) {
let memory = self.default_memory();
let result: u32 = self.ctx.resolve_memory(memory).current_pages().into();
let result: u32 = self.ctx.resolve_memory(&memory).current_pages().into();
self.value_stack.push(result);
self.next_instr()
}
Expand All @@ -582,7 +582,7 @@ impl<'ctx, 'engine, 'func> Executor<'ctx, 'engine, 'func> {
let memory = self.default_memory();
let result = Pages::new(self.value_stack.pop_as()).map_or(ERR_VALUE, |additional| {
self.ctx
.resolve_memory_mut(memory)
.resolve_memory_mut(&memory)
.grow(additional)
.map(u32::from)
.unwrap_or(ERR_VALUE)
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmi/src/engine/func_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl FuncTypeRegistry {
///
/// - If the deduplicated function type is not owned by the engine.
/// - If the deduplicated function type cannot be resolved to its entity.
pub(crate) fn resolve_func_type(&self, func_type: DedupFuncType) -> &FuncType {
pub(crate) fn resolve_func_type(&self, func_type: &DedupFuncType) -> &FuncType {
let entity_index = self.unwrap_index(func_type.into_inner());
self.func_types
.get(entity_index)
Expand Down
4 changes: 2 additions & 2 deletions crates/wasmi/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl Engine {
///
/// - If the deduplicated function type is not owned by the engine.
/// - If the deduplicated function type cannot be resolved to its entity.
pub(super) fn resolve_func_type<F, R>(&self, func_type: DedupFuncType, f: F) -> R
pub(super) fn resolve_func_type<F, R>(&self, func_type: &DedupFuncType, f: F) -> R
where
F: FnOnce(&FuncType) -> R,
{
Expand Down Expand Up @@ -371,7 +371,7 @@ impl EngineInner {
.alloc(len_locals, max_stack_height, insts)
}

fn resolve_func_type<F, R>(&self, func_type: DedupFuncType, f: F) -> R
fn resolve_func_type<F, R>(&self, func_type: &DedupFuncType, f: F) -> R
where
F: FnOnce(&FuncType) -> R,
{
Expand Down
4 changes: 2 additions & 2 deletions crates/wasmi/src/engine/stack/frames.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ impl FuncFrame {
}

/// Returns the instance of the [`FuncFrame`].
pub fn instance(&self) -> Instance {
self.instance
pub fn instance(&self) -> &Instance {
&self.instance
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/wasmi/src/engine/stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl Stack {
&mut self,
mut ctx: C,
host_func: HostFuncEntity<<C as AsContext>::UserState>,
instance: Option<Instance>,
instance: Option<&Instance>,
func_types: &FuncTypeRegistry,
) -> Result<(), Trap>
where
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmi/src/engine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn assert_func_body<E>(
actual,
expected,
"encountered instruction mismatch for {} at position {index}",
engine.resolve_func_type(func_type, Clone::clone),
engine.resolve_func_type(&func_type, Clone::clone),
);
}
if let Some(unexpected) = engine.resolve_inst(func_body, len_expected) {
Expand Down
4 changes: 2 additions & 2 deletions crates/wasmi/src/func/caller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ pub struct Caller<'a, T> {

impl<'a, T> Caller<'a, T> {
/// Creates a new [`Caller`] from the given store context and [`Instance`] handle.
pub(crate) fn new<C>(ctx: &'a mut C, instance: Option<Instance>) -> Self
pub(crate) fn new<C>(ctx: &'a mut C, instance: Option<&Instance>) -> Self
where
C: AsContextMut<UserState = T>,
{
Self {
store: ctx.as_context_mut(),
instance,
instance: instance.copied(),
}
}

Expand Down
25 changes: 14 additions & 11 deletions crates/wasmi/src/func/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl<T> FuncEntity<T> {
}

/// Returns the signature of the Wasm function.
pub fn ty_dedup(&self) -> DedupFuncType {
pub fn ty_dedup(&self) -> &DedupFuncType {
match self.as_internal() {
FuncEntityInternal::Wasm(func) => func.ty_dedup(),
FuncEntityInternal::Host(func) => func.ty_dedup(),
Expand Down Expand Up @@ -138,8 +138,8 @@ impl WasmFuncEntity {
}

/// Returns the signature of the Wasm function.
pub fn ty_dedup(&self) -> DedupFuncType {
self.signature
pub fn ty_dedup(&self) -> &DedupFuncType {
&self.signature
}

/// Returns the instance where the [`Func`] belong to.
Expand Down Expand Up @@ -216,8 +216,8 @@ impl<T> HostFuncEntity<T> {
}

/// Returns the signature of the host function.
pub fn ty_dedup(&self) -> DedupFuncType {
self.signature
pub fn ty_dedup(&self) -> &DedupFuncType {
&self.signature
}

/// Calls the host function with the given inputs.
Expand All @@ -226,7 +226,7 @@ impl<T> HostFuncEntity<T> {
pub fn call(
&self,
mut ctx: impl AsContextMut<UserState = T>,
instance: Option<Instance>,
instance: Option<&Instance>,
params: FuncParams,
) -> Result<FuncResults, Trap> {
let caller = <Caller<T>>::new(&mut ctx, instance);
Expand All @@ -246,8 +246,8 @@ impl Func {
}

/// Returns the underlying stored representation.
pub(super) fn into_inner(self) -> Stored<FuncIdx> {
self.0
pub(super) fn as_inner(&self) -> &Stored<FuncIdx> {
&self.0
}

/// Creates a new host function from the given closure.
Expand All @@ -260,8 +260,11 @@ impl Func {
}

/// Returns the signature of the function.
pub(crate) fn ty_dedup(&self, ctx: impl AsContext) -> DedupFuncType {
ctx.as_context().store.resolve_func(*self).ty_dedup()
pub(crate) fn ty_dedup<'a, T: 'a>(
&self,
ctx: impl Into<StoreContext<'a, T>>,
) -> &'a DedupFuncType {
ctx.into().store.resolve_func(self).ty_dedup()
}

/// Returns the function type of the [`Func`].
Expand Down Expand Up @@ -406,6 +409,6 @@ impl Func {
&self,
ctx: impl Into<StoreContext<'a, T>>,
) -> &'a FuncEntityInternal<T> {
ctx.into().store.resolve_func(*self).as_internal()
ctx.into().store.resolve_func(self).as_internal()
}
}
10 changes: 5 additions & 5 deletions crates/wasmi/src/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ impl Global {
}

/// Returns the underlying stored representation.
pub(super) fn into_inner(self) -> Stored<GlobalIdx> {
self.0
pub(super) fn as_inner(&self) -> &Stored<GlobalIdx> {
&self.0
}

/// Creates a new global variable to the store.
Expand All @@ -240,7 +240,7 @@ impl Global {

/// Returns the [`GlobalType`] of the global variable.
pub fn ty(&self, ctx: impl AsContext) -> GlobalType {
ctx.as_context().store.inner.resolve_global(*self).ty()
ctx.as_context().store.inner.resolve_global(self).ty()
}

/// Sets a new value to the global variable.
Expand All @@ -257,7 +257,7 @@ impl Global {
ctx.as_context_mut()
.store
.inner
.resolve_global_mut(*self)
.resolve_global_mut(self)
.set(new_value)
}

Expand All @@ -267,6 +267,6 @@ impl Global {
///
/// Panics if `ctx` does not own this [`Global`].
pub fn get(&self, ctx: impl AsContext) -> Value {
ctx.as_context().store.inner.resolve_global(*self).get()
ctx.as_context().store.inner.resolve_global(self).get()
}
}
10 changes: 5 additions & 5 deletions crates/wasmi/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,8 @@ impl Instance {
}

/// Returns the underlying stored representation.
pub(super) fn into_inner(self) -> Stored<InstanceIdx> {
self.0
pub(super) fn as_inner(&self) -> &Stored<InstanceIdx> {
&self.0
}

/// Returns the function at the `index` if any.
Expand All @@ -439,7 +439,7 @@ impl Instance {
.as_context()
.store
.inner
.resolve_instance(*self)
.resolve_instance(self)
.get_func(index)
}

Expand All @@ -453,7 +453,7 @@ impl Instance {
.as_context()
.store
.inner
.resolve_instance(*self)
.resolve_instance(self)
.get_export(name)
}

Expand Down Expand Up @@ -545,6 +545,6 @@ impl Instance {
&self,
store: impl Into<StoreContext<'ctx, T>>,
) -> ExportsIter<'ctx> {
store.into().store.inner.resolve_instance(*self).exports()
store.into().store.inner.resolve_instance(self).exports()
}
}
20 changes: 10 additions & 10 deletions crates/wasmi/src/memory/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ impl Memory {
}

/// Returns the underlying stored representation.
pub(super) fn into_inner(self) -> Stored<MemoryIdx> {
self.0
pub(super) fn as_inner(&self) -> &Stored<MemoryIdx> {
&self.0
}

/// Creates a new linear memory to the store.
Expand All @@ -281,7 +281,7 @@ impl Memory {
///
/// Panics if `ctx` does not own this [`Memory`].
pub fn ty(&self, ctx: impl AsContext) -> MemoryType {
ctx.as_context().store.inner.resolve_memory(*self).ty()
ctx.as_context().store.inner.resolve_memory(self).ty()
}

/// Returns the amount of pages in use by the linear memory.
Expand All @@ -293,7 +293,7 @@ impl Memory {
ctx.as_context()
.store
.inner
.resolve_memory(*self)
.resolve_memory(self)
.current_pages()
}

Expand All @@ -317,7 +317,7 @@ impl Memory {
ctx.as_context_mut()
.store
.inner
.resolve_memory_mut(*self)
.resolve_memory_mut(self)
.grow(additional)
}

Expand All @@ -327,7 +327,7 @@ impl Memory {
///
/// Panics if `ctx` does not own this [`Memory`].
pub fn data<'a, T: 'a>(&self, ctx: impl Into<StoreContext<'a, T>>) -> &'a [u8] {
ctx.into().store.inner.resolve_memory(*self).data()
ctx.into().store.inner.resolve_memory(self).data()
}

/// Returns an exclusive slice to the bytes underlying the [`Memory`].
Expand All @@ -336,7 +336,7 @@ impl Memory {
///
/// Panics if `ctx` does not own this [`Memory`].
pub fn data_mut<'a, T: 'a>(&self, ctx: impl Into<StoreContextMut<'a, T>>) -> &'a mut [u8] {
ctx.into().store.inner.resolve_memory_mut(*self).data_mut()
ctx.into().store.inner.resolve_memory_mut(self).data_mut()
}

/// Returns an exclusive slice to the bytes underlying the [`Memory`], and an exclusive
Expand All @@ -349,7 +349,7 @@ impl Memory {
&self,
ctx: impl Into<StoreContextMut<'a, T>>,
) -> (&'a mut [u8], &'a mut T) {
let (memory, store) = ctx.into().store.resolve_memory_and_state_mut(*self);
let (memory, store) = ctx.into().store.resolve_memory_and_state_mut(self);
(memory.data_mut(), store)
}

Expand All @@ -372,7 +372,7 @@ impl Memory {
ctx.as_context()
.store
.inner
.resolve_memory(*self)
.resolve_memory(self)
.read(offset, buffer)
}

Expand All @@ -395,7 +395,7 @@ impl Memory {
ctx.as_context_mut()
.store
.inner
.resolve_memory_mut(*self)
.resolve_memory_mut(self)
.write(offset, buffer)
}
}
Loading

0 comments on commit 77e574c

Please sign in to comment.