improve leb128 handling, move sidetable to using arrays, label error creation as cold paths

Author: InvalidPathException

bench.md

@@ -1,7 +1,12 @@
 This file documents the coremark bench results to document performance improvements over time.
 - ac6b813: avg ≈ 500 (introduced benchmarking without criterion)
-- current: avg = 529.773949, n = 20 (improved leb128 handling with unsafe)
+- 5f4af0c: avg = 529.773949, n = 20 (improved leb128 handling with unsafe)
     - we try to avoid unsafe code in module/validator/instance directly
+- current: avg = 810.367084, n = 20 (use a dense offset table instead of hashmap, better leb128, and cold path hinting)
+    - rust hashmaps are apparently very bad, this might be because of our memory access pattern, when we interpret the control op codes (corresponding to side table entries) they are kind of accessed sequentially, so even btreemap yields better results
+    - current design: two-level indirection (one array covering all possible code indices and directing them to a densely packed side table)
+    - we also try to not use nightly features... making error creation cold path is a really elegant solution in this regard
+    - repr(C) for the SideTableEntry struct caused mysterious improvements, not sure if it is a fluke
 
 
 Hardware Overview:

src/byte_iter.rs

@@ -1,4 +1,3 @@
-use crate::error::Error::*;
 use crate::error::*;
 
 #[derive(Clone, Copy)]
@@ -20,14 +19,14 @@ impl<'a> ByteIter<'a> {
     pub fn advance(&mut self, n: usize) { self.idx += n; }
     #[inline]
     pub fn read_u8(&mut self) -> Result<u8, Error> {
-        if self.idx >= self.bytes.len() { return Err(Malformed(UNEXPECTED_END)); }
+        if self.idx >= self.bytes.len() { return Err(Error::malformed(UNEXPECTED_END)); }
         let b = self.bytes[self.idx];
         self.idx += 1;
         Ok(b)
     }
     #[inline]
     pub fn peek_u8(&self) -> Result<u8, Error> {
-        if self.idx >= self.bytes.len() { return Err(Malformed(UNEXPECTED_END)); }
+        if self.idx >= self.bytes.len() { return Err(Error::malformed(UNEXPECTED_END)); }
         Ok(self.bytes[self.idx])
     }
 }

src/error.rs

@@ -23,6 +23,19 @@ impl Display for Error {
 
 impl std::error::Error for Error {}
 
+impl Error {
+    #[cold] #[inline(never)]
+    pub fn malformed(msg: &'static str) -> Self { Error::Malformed(msg) }
+    #[cold] #[inline(never)]
+    pub fn validation(msg: &'static str) -> Self { Error::Validation(msg) }
+    #[cold] #[inline(never)]
+    pub fn trap(msg: &'static str) -> Self { Error::Trap(msg) }
+    #[cold] #[inline(never)]
+    pub fn link(msg: &'static str) -> Self { Error::Link(msg) }
+    #[cold] #[inline(never)]
+    pub fn uninstantiable(msg: &'static str) -> Self { Error::Uninstantiable(msg) }
+}
+
 // Malformed errors
 pub const END_EXPECTED: &str = "END opcode expected";
 pub const FUNC_CODE_INCONSISTENT: &str = "function and code section have inconsistent lengths";

src/instance.rs

@@ -1,4 +1,3 @@
-use crate::error::Error::*;
 use crate::error::*;
 
 #[inline]
@@ -10,7 +9,7 @@ where T: TryFrom<u64> {
     let mut byte: u8;
     unsafe {
         loop {
-            if end >= bytes.len() { return Err(Malformed(UNEXPECTED_END)); }
+            if end >= bytes.len() { return Err(Error::malformed(UNEXPECTED_END)); }
             byte = *bytes.get_unchecked(end);
             end += 1;
             result |= ((byte & 0x7f) as u64) << shift;
@@ -19,16 +18,16 @@ where T: TryFrom<u64> {
         }
     }
     let consumed = end - *pc;
-    if consumed > (bits as usize).div_ceil(7) { return Err(Malformed(INT_TOO_LONG)); }
+    if consumed > (bits as usize).div_ceil(7) { return Err(Error::malformed(INT_TOO_LONG)); }
 
     // Only bits=1 and bits=32 are used
-    if (bits == 1 && result > 1) || (bits == 32 && result > 0xFFFFFFFF) { return Err(Malformed(INT_TOO_LARGE)); }
+    if (bits == 1 && result > 1) || (bits == 32 && result > 0xFFFFFFFF) { return Err(Error::malformed(INT_TOO_LARGE)); }
 
     if consumed > 1 {
         let used = (consumed - 1) * 7;
         if used < bits as usize {
             let rem = bits as usize - used;
-            if rem < 32 && (bytes[end - 1] as u32) >> rem != 0 { return Err(Malformed(INT_TOO_LARGE)); }
+            if rem < 32 && (bytes[end - 1] as u32) >> rem != 0 { return Err(Error::malformed(INT_TOO_LARGE)); }
         }
     }
     *pc = end;
@@ -44,7 +43,7 @@ where T: TryFrom<i64> {
     let mut byte: u8;
     unsafe {
         loop {
-            if end >= bytes.len() { return Err(Malformed(UNEXPECTED_END)); }
+            if end >= bytes.len() { return Err(Error::malformed(UNEXPECTED_END)); }
             byte = *bytes.get_unchecked(end);
             end += 1;
             if shift < 63 {
@@ -63,17 +62,17 @@ where T: TryFrom<i64> {
         32 | 33 => {
             const MIN_I32: i128 = -(1i128 << 31);
             const MAX_I32: i128 = (1i128 << 31) - 1;
-            if (result as i128) < MIN_I32 || (result as i128) > MAX_I32 { return Err(Malformed(INT_TOO_LARGE)); }
+            if (result as i128) < MIN_I32 || (result as i128) > MAX_I32 { return Err(Error::malformed(INT_TOO_LARGE)); }
         }
         64 => {} // Already i64
         _ => unreachable!()
     }
 
-    if consumed > (bits as usize).div_ceil(7) { return Err(Malformed(INT_TOO_LONG)); }
+    if consumed > (bits as usize).div_ceil(7) { return Err(Error::malformed(INT_TOO_LONG)); }
     if consumed >= 1 {
         let last = bytes[end - 1];
         if ((last != 0 && last != 127) as usize + (consumed - 1) * 7) >= bits as usize {
-            return Err(Malformed(INT_TOO_LARGE));
+            return Err(Error::malformed(INT_TOO_LARGE));
         }
     }
     *pc = end;
@@ -92,7 +91,7 @@ where T: TryFrom<u64> {
             *pc += 1;
             result |= ((byte & 0x7f) as u64) << shift;
             if byte & 0x80 == 0 {
-                return Ok(T::try_from(result).ok().unwrap_unchecked());
+                return Ok(T::try_from(result).unwrap_unchecked());
             }
             shift += 7;
         }
@@ -109,15 +108,13 @@ where T: TryFrom<i64> {
         loop {
             byte = *bytes.get_unchecked(*pc);
             *pc += 1;
-            if shift < 63 {
-                result |= ((byte & 0x7f) as i64) << shift;
-            }
-            shift = (shift + 7).min(63);
+            result |= ((byte & 0x7f) as i64) << shift;
+            shift += 7;
             if byte & 0x80 == 0 { break; }
         }
         if shift < 64 && (byte & 0x40) != 0 {
             result |= (!0i64) << shift;
         }
-        Ok(T::try_from(result).ok().unwrap_unchecked())
+        Ok(T::try_from(result).unwrap_unchecked())
     }
 }

src/leb128.rs

@@ -16,7 +16,8 @@ mod error;
 pub use signature::{Signature, ValType};
 
 // Runtime types
-pub use instance::{ExportValue, RuntimeFunction, Imports, Instance, RuntimeSignature, WasmGlobal, WasmTable, WasmValue};
+pub use instance::{ExportValue, RuntimeFunction, Imports, Instance, WasmGlobal, WasmTable, WasmValue};
+pub use signature::RuntimeSignature;
 
 // Main API types
 pub use module::Module;