Optimize ValueStack (#404)

* optimize ValueStack methods by removing unncessary bounds checks

This resulted in a performance boost of approx 8-10% across the board.
In debug mode we still do bounds checking which helps with testing.

* optimize ValueStack::peek{_mut} slightly

No longer adds and subtracts depth by 1.

* add #[cold] UntypedError::invalid_len and make use of it

* replace type name with Self in match

* simplify and clean up impls for {i32,i64}::{div,rem}

* fix compile error

* apply rustfmt

core/src/trap.rs

@@ -159,15 +159,15 @@ impl TrapCode {
     /// other uses since it avoid heap memory allocation in certain cases.
     pub fn trap_message(&self) -> &'static str {
         match self {
-            TrapCode::Unreachable => "unreachable",
-            TrapCode::MemoryAccessOutOfBounds => "out of bounds memory access",
-            TrapCode::TableAccessOutOfBounds => "undefined element",
-            TrapCode::ElemUninitialized => "uninitialized element",
-            TrapCode::DivisionByZero => "integer divide by zero",
-            TrapCode::IntegerOverflow => "integer overflow",
-            TrapCode::InvalidConversionToInt => "invalid conversion to integer",
-            TrapCode::StackOverflow => "call stack exhausted",
-            TrapCode::UnexpectedSignature => "indirect call type mismatch",
+            Self::Unreachable => "unreachable",
+            Self::MemoryAccessOutOfBounds => "out of bounds memory access",
+            Self::TableAccessOutOfBounds => "undefined element",
+            Self::ElemUninitialized => "uninitialized element",
+            Self::DivisionByZero => "integer divide by zero",
+            Self::IntegerOverflow => "integer overflow",
+            Self::InvalidConversionToInt => "invalid conversion to integer",
+            Self::StackOverflow => "call stack exhausted",
+            Self::UnexpectedSignature => "indirect call type mismatch",
         }
     }
 }

core/src/untyped.rs

@@ -882,6 +882,14 @@ pub enum UntypedError {
     },
 }
 
+impl UntypedError {
+    /// Creates a new `InvalidLen` [`UntypedError`].
+    #[cold]
+    pub fn invalid_len(expected: usize, found: usize) -> Self {
+        Self::InvalidLen { expected, found }
+    }
+}
+
 impl Display for UntypedError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
@@ -953,10 +961,7 @@ where
 {
     fn decode_untyped_slice(results: &[UntypedValue]) -> Result<Self, UntypedError> {
         if results.len() != 1 {
-            return Err(UntypedError::InvalidLen {
-                expected: 1,
-                found: results.len(),
-            });
+            return Err(UntypedError::invalid_len(1, results.len()));
         }
         Ok(<T1 as From<UntypedValue>>::from(results[0]))
     }
@@ -979,10 +984,7 @@ macro_rules! impl_decode_untyped_slice {
                         )*
                     )),
                     _unexpected => {
-                        Err(UntypedError::InvalidLen {
-                            expected: $n,
-                            found: results.len(),
-                        })
+                        Err(UntypedError::invalid_len($n, results.len()))
                     }
                 }
             }
@@ -1012,10 +1014,7 @@ where
 {
     fn encode_untyped_slice(self, results: &mut [UntypedValue]) -> Result<(), UntypedError> {
         if results.len() != 1 {
-            return Err(UntypedError::InvalidLen {
-                expected: 1,
-                found: results.len(),
-            });
+            return Err(UntypedError::invalid_len(1, results.len()));
         }
         results[0] = self.into();
         Ok(())
@@ -1033,10 +1032,7 @@ macro_rules! impl_encode_untyped_slice {
             #[allow(non_snake_case)]
             fn encode_untyped_slice(self, results: &mut [UntypedValue]) -> Result<(), UntypedError> {
                 if results.len() != $n {
-                    return Err(UntypedError::InvalidLen {
-                        expected: $n,
-                        found: results.len(),
-                    })
+                    return Err(UntypedError::invalid_len($n, results.len()))
                 }
                 let ($($tuple,)*) = self;
                 let converted: [UntypedValue; $n] = [

core/src/value.rs

@@ -780,14 +780,11 @@ macro_rules! impl_integer_arithmetic_ops {
             #[inline]
             fn div(self, other: $type) -> Result<$type, TrapCode> {
                 if other == 0 {
-                    Err(TrapCode::DivisionByZero)
-                } else {
-                    let (result, overflow) = self.overflowing_div(other);
-                    if overflow {
-                        Err(TrapCode::IntegerOverflow)
-                    } else {
-                        Ok(result)
-                    }
+                    return Err(TrapCode::DivisionByZero);
+                }
+                match self.overflowing_div(other) {
+                    (result, false) => Ok(result),
+                    (_, true) => Err(TrapCode::IntegerOverflow),
                 }
             }
         }
@@ -853,10 +850,9 @@ macro_rules! impl_integer {
             #[inline]
             fn rem(self, other: $type) -> Result<$type, TrapCode> {
                 if other == 0 {
-                    Err(TrapCode::DivisionByZero)
-                } else {
-                    Ok(self.wrapping_rem(other))
+                    return Err(TrapCode::DivisionByZero);
                 }
+                Ok(self.wrapping_rem(other))
             }
         }
     };

wasmi_v1/src/engine/exec_context.rs

@@ -344,8 +344,7 @@ where
 
     /// Returns the local depth as `usize`.
     fn convert_local_depth(local_depth: LocalIdx) -> usize {
-        // TODO: calculate the -1 offset at module compilation time.
-        (local_depth.into_inner() - 1) as usize
+        local_depth.into_inner() as usize
     }
 
     /// Calculates the effective address of a linear memory access.

wasmi_v1/src/engine/value_stack.rs

@@ -95,6 +95,54 @@ impl ValueStack {
         }
     }
 
+    /// Returns the [`UntypedValue`] at the given `index`.
+    ///
+    /// # Note
+    ///
+    /// This is an optimized convenience method that only asserts
+    /// that the index is within bounds in `debug` mode.
+    ///
+    /// # Safety
+    ///
+    /// This is safe since all wasmi bytecode has been validated
+    /// during translation and therefore cannot result in out of
+    /// bounds accesses.
+    ///
+    /// # Panics (Debug)
+    ///
+    /// If the `index` is out of bounds.
+    fn get_release_unchecked(&self, index: usize) -> UntypedValue {
+        debug_assert!(index < self.entries.len());
+        // Safety: This is safe since all wasmi bytecode has been validated
+        //         during translation and therefore cannot result in out of
+        //         bounds accesses.
+        unsafe { *self.entries.get_unchecked(index) }
+    }
+
+    /// Returns the [`UntypedValue`] at the given `index`.
+    ///
+    /// # Note
+    ///
+    /// This is an optimized convenience method that only asserts
+    /// that the index is within bounds in `debug` mode.
+    ///
+    /// # Safety
+    ///
+    /// This is safe since all wasmi bytecode has been validated
+    /// during translation and therefore cannot result in out of
+    /// bounds accesses.
+    ///
+    /// # Panics (Debug)
+    ///
+    /// If the `index` is out of bounds.
+    fn get_release_unchecked_mut(&mut self, index: usize) -> &mut UntypedValue {
+        debug_assert!(index < self.entries.len());
+        // Safety: This is safe since all wasmi bytecode has been validated
+        //         during translation and therefore cannot result in out of
+        //         bounds accesses.
+        unsafe { self.entries.get_unchecked_mut(index) }
+    }
+
     /// Extends the value stack by the `additional` amount of zeros.
     ///
     /// # Errors
@@ -136,7 +184,7 @@ impl ValueStack {
         let src = self.stack_ptr - keep;
         let dst = self.stack_ptr - keep - drop;
         for i in 0..keep {
-            self.entries[dst + i] = self.entries[src + i];
+            *self.get_release_unchecked_mut(dst + i) = self.get_release_unchecked(src + i);
         }
         self.stack_ptr -= drop;
     }
@@ -147,7 +195,7 @@ impl ValueStack {
     ///
     /// This has the same effect as [`ValueStack::peek`]`(0)`.
     pub fn last(&self) -> UntypedValue {
-        self.entries[self.stack_ptr - 1]
+        self.get_release_unchecked(self.stack_ptr - 1)
     }
 
     /// Returns the last stack entry of the [`ValueStack`].
@@ -156,25 +204,29 @@ impl ValueStack {
     ///
     /// This has the same effect as [`ValueStack::peek`]`(0)`.
     pub fn last_mut(&mut self) -> &mut UntypedValue {
-        &mut self.entries[self.stack_ptr - 1]
+        self.get_release_unchecked_mut(self.stack_ptr - 1)
     }
 
     /// Peeks the entry at the given depth from the last entry.
     ///
     /// # Note
     ///
-    /// Given a `depth` of 0 has the same effect as [`ValueStack::last`].
+    /// Given a `depth` of 1 has the same effect as [`ValueStack::last`].
+    ///
+    /// A `depth` of 0 is invalid and undefined.
     pub fn peek(&self, depth: usize) -> UntypedValue {
-        self.entries[self.stack_ptr - depth - 1]
+        self.get_release_unchecked(self.stack_ptr - depth)
     }
 
     /// Peeks the `&mut` entry at the given depth from the last entry.
     ///
     /// # Note
     ///
-    /// Given a `depth` of 0 has the same effect as [`ValueStack::last_mut`].
+    /// Given a `depth` of 1 has the same effect as [`ValueStack::last_mut`].
+    ///
+    /// A `depth` of 0 is invalid and undefined.
     pub fn peek_mut(&mut self, depth: usize) -> &mut UntypedValue {
-        &mut self.entries[self.stack_ptr - depth - 1]
+        self.get_release_unchecked_mut(self.stack_ptr - depth)
     }
 
     /// Pops the last [`UntypedValue`] from the [`ValueStack`].
@@ -185,9 +237,10 @@ impl ValueStack {
     /// the executed WebAssembly bytecode for correctness.
     pub fn pop(&mut self) -> UntypedValue {
         self.stack_ptr -= 1;
-        self.entries[self.stack_ptr]
+        self.get_release_unchecked(self.stack_ptr)
     }
 
+    /// Drops the last value on the [`ValueStack`].
     pub fn drop(&mut self, depth: usize) {
         self.stack_ptr -= depth;
     }
@@ -211,8 +264,8 @@ impl ValueStack {
     pub fn pop2(&mut self) -> (UntypedValue, UntypedValue) {
         self.stack_ptr -= 2;
         (
-            self.entries[self.stack_ptr],
-            self.entries[self.stack_ptr + 1],
+            self.get_release_unchecked(self.stack_ptr),
+            self.get_release_unchecked(self.stack_ptr + 1),
         )
     }
 
@@ -246,7 +299,7 @@ impl ValueStack {
     where
         T: Into<UntypedValue>,
     {
-        self.entries[self.stack_ptr] = entry.into();
+        *self.get_release_unchecked_mut(self.stack_ptr) = entry.into();
         self.stack_ptr += 1;
     }