From b1bdfed38f73f003463cbd5f8a3d828ee550f558 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sun, 2 Oct 2022 20:10:32 +0200 Subject: [PATCH 1/3] bail out early when a jump has to keep no values around The procedures to keep values is pretty expensive when in the case where no values shall be kept so we bail out early for some nice and easy performance gains in some cases. --- crates/wasmi/src/engine/stack/values/vref.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/wasmi/src/engine/stack/values/vref.rs b/crates/wasmi/src/engine/stack/values/vref.rs index c716ea9c13..47287df642 100644 --- a/crates/wasmi/src/engine/stack/values/vref.rs +++ b/crates/wasmi/src/engine/stack/values/vref.rs @@ -111,6 +111,11 @@ impl<'a> ValueStackRef<'a> { return; } let keep = drop_keep.keep(); + if keep == 0 { + // Bail out early when there are no values to keep. + self.stack_ptr -= drop; + return; + } // Copy kept values over to their new place on the stack. // Note: We cannot use `memcpy` since the slices may overlap. let src = self.stack_ptr - keep; From e2588d5a8feb4d2a946e292610e96fdeb374384e Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sun, 2 Oct 2022 20:10:56 +0200 Subject: [PATCH 2/3] clean-up implementation of the wasmi executor a bit --- crates/wasmi/src/engine/executor.rs | 47 ++++++++++++----------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/crates/wasmi/src/engine/executor.rs b/crates/wasmi/src/engine/executor.rs index 707eed1032..76d748de8b 100644 --- a/crates/wasmi/src/engine/executor.rs +++ b/crates/wasmi/src/engine/executor.rs @@ -86,9 +86,9 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { Instr::LocalGet { local_depth } => self.visit_local_get(local_depth), Instr::LocalSet { local_depth } => self.visit_local_set(local_depth), Instr::LocalTee { local_depth } => self.visit_local_tee(local_depth), - Instr::Br(target) => self.visit_br(target), - Instr::BrIfEqz(target) => self.visit_br_if_eqz(target), - Instr::BrIfNez(target) => self.visit_br_if_nez(target), + Instr::Br(params) => self.visit_br(params), + Instr::BrIfEqz(params) => self.visit_br_if_eqz(params), + Instr::BrIfNez(params) => self.visit_br_if_nez(params), Instr::ReturnIfNez(drop_keep) => { if let MaybeReturn::Return = self.visit_return_if_nez(drop_keep) { return Ok(CallOutcome::Return); @@ -447,7 +447,8 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { f: fn(UntypedValue) -> Result, ) -> Result<(), TrapCode> { self.value_stack.try_eval_top(f)?; - self.try_next_instr() + self.next_instr(); + Ok(()) } fn execute_binary(&mut self, f: fn(UntypedValue, UntypedValue) -> UntypedValue) { @@ -460,7 +461,8 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { f: fn(UntypedValue, UntypedValue) -> Result, ) -> Result<(), TrapCode> { self.value_stack.try_eval_top2(f)?; - self.try_next_instr() + self.next_instr(); + Ok(()) } fn execute_reinterpret(&mut self) @@ -472,25 +474,16 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { self.next_instr() } - fn try_next_instr(&mut self) -> Result<(), TrapCode> { - self.next_instr(); - Ok(()) + fn next_instr(&mut self) { + self.ip_add(1) } - fn next_instr(&mut self) { - // Safety: This is safe since we carefully constructed the `wasmi` - // bytecode in conjunction with Wasm validation so that the - // offsets of the instruction pointer within the sequence of - // instructions never make the instruction pointer point out - // of bounds of the instructions that belong to the function - // that is currently executed. - unsafe { - self.ip.offset(1); - }; + fn branch_to(&mut self, params: BranchParams) { + self.value_stack.drop_keep(params.drop_keep()); + self.ip_add(params.offset().into_i32() as isize) } - fn branch_to(&mut self, target: BranchParams) { - self.value_stack.drop_keep(target.drop_keep()); + fn ip_add(&mut self, delta: isize) { // Safety: This is safe since we carefully constructed the `wasmi` // bytecode in conjunction with Wasm validation so that the // offsets of the instruction pointer within the sequence of @@ -498,7 +491,7 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { // of bounds of the instructions that belong to the function // that is currently executed. unsafe { - self.ip.offset(target.offset().into_i32() as isize); + self.ip.offset(delta); } } @@ -530,23 +523,23 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { Err(TrapCode::Unreachable).map_err(Into::into) } - fn visit_br(&mut self, target: BranchParams) { - self.branch_to(target) + fn visit_br(&mut self, params: BranchParams) { + self.branch_to(params) } - fn visit_br_if_eqz(&mut self, target: BranchParams) { + fn visit_br_if_eqz(&mut self, params: BranchParams) { let condition = self.value_stack.pop_as(); if condition { self.next_instr() } else { - self.branch_to(target) + self.branch_to(params) } } - fn visit_br_if_nez(&mut self, target: BranchParams) { + fn visit_br_if_nez(&mut self, params: BranchParams) { let condition = self.value_stack.pop_as(); if condition { - self.branch_to(target) + self.branch_to(params) } else { self.next_instr() } From edcd58054a519ca8bd43d01cbb26bb7af202b441 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Sun, 2 Oct 2022 22:08:29 +0200 Subject: [PATCH 3/3] add special case for keep==1 in ValueStackRef::drop_keep --- crates/wasmi/src/engine/stack/values/vref.rs | 21 +++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/wasmi/src/engine/stack/values/vref.rs b/crates/wasmi/src/engine/stack/values/vref.rs index 47287df642..8b797be3f7 100644 --- a/crates/wasmi/src/engine/stack/values/vref.rs +++ b/crates/wasmi/src/engine/stack/values/vref.rs @@ -113,15 +113,18 @@ impl<'a> ValueStackRef<'a> { let keep = drop_keep.keep(); if keep == 0 { // Bail out early when there are no values to keep. - self.stack_ptr -= drop; - return; - } - // Copy kept values over to their new place on the stack. - // Note: We cannot use `memcpy` since the slices may overlap. - let src = self.stack_ptr - keep; - let dst = self.stack_ptr - keep - drop; - for i in 0..keep { - *self.get_release_unchecked_mut(dst + i) = self.get_release_unchecked(src + i); + } else if keep == 1 { + // Bail out early when there is only one value to copy. + *self.get_release_unchecked_mut(self.stack_ptr - 1 - drop) = + self.get_release_unchecked(self.stack_ptr - 1); + } else { + // Copy kept values over to their new place on the stack. + // Note: We cannot use `memcpy` since the slices may overlap. + let src = self.stack_ptr - keep; + let dst = self.stack_ptr - keep - drop; + for i in 0..keep { + *self.get_release_unchecked_mut(dst + i) = self.get_release_unchecked(src + i); + } } self.stack_ptr -= drop; }