More VM opcode optimizations & cleanups

  * Make opcode dispatch faster by not having an Option return with unwrap. We panic anyways, no need to make that decision inside execute. 10% increase in raw opcode dispatch performance.
  * Some more stack-manip optimizations
  * Go through opcodes and util functions and do a little cleanup

crates/kernel/src/vm/activation.rs

@@ -319,13 +319,11 @@ impl Activation {
     }
 
     #[inline]
-    pub fn next_op(&mut self) -> Option<Op> {
-        if !self.pc < self.program.main_vector.len() {
-            return None;
-        }
+    pub fn next_op(&mut self) -> Op {
+        assert!(self.pc < self.program.main_vector.len(), "pc out of bounds");
         let op = self.program.main_vector[self.pc].clone();
         self.pc += 1;
-        Some(op)
+        op
     }
 
     #[inline]
@@ -354,11 +352,16 @@ impl Activation {
     }
 
     #[inline]
-    pub fn peek(&self, width: usize) -> Vec<Var> {
+    pub fn peek_range(&self, width: usize) -> Vec<Var> {
         let l = self.valstack.len();
         Vec::from(&self.valstack[l - width..])
     }
 
+    #[inline]
+    pub(crate) fn peek_abs(&self, amt: usize) -> &Var {
+        return &self.valstack[amt];
+    }
+
     #[inline]
     pub fn peek2(&self) -> (Var, Var) {
         let l = self.valstack.len();

crates/kernel/src/vm/exec_state.rs

@@ -151,10 +151,19 @@ impl VMExecState {
         self.top_mut().push(v.clone())
     }
 
-    /// Non-destructively peek in the value stack at the given offset.
+    /// Non-destructively peek a range of values from the value stack.
+    /// Returns top N values, where N is the given amount.
     #[inline]
-    pub(crate) fn peek(&self, amt: usize) -> Vec<Var> {
-        self.top().peek(amt)
+    pub(crate) fn peek_range(&self, amt: usize) -> Vec<Var> {
+        self.top().peek_range(amt)
+    }
+
+    /// Non-destructively return a value at a specific absolute offset in the value stack.
+    /// Not computed relative to the top of the stack, used by the Length opcode, which
+    /// has specific knowledge of the stack layout.
+    #[inline]
+    pub(crate) fn peek_abs(&self, amt: usize) -> &Var {
+        self.top().peek_abs(amt)
     }
 
     /// Return the top two values on the value stack.
@@ -177,7 +186,7 @@ impl VMExecState {
 
     /// Return the next opcode in the program stream.
     #[inline]
-    pub(crate) fn next_op(&mut self) -> Option<Op> {
+    pub(crate) fn next_op(&mut self) -> Op {
         self.top_mut().next_op()
     }
 

crates/kernel/src/vm/vm_execute.rs

@@ -132,15 +132,19 @@ macro_rules! binary_bool_op {
 macro_rules! binary_var_op {
     ( $vm:ident, $state:ident, $op:tt ) => {
         let rhs = $state.pop();
-        let lhs = $state.pop();
+        let lhs = $state.peek_top();
         let result = lhs.$op(&rhs);
         match result {
-            Ok(result) => $state.push(&result),
-            Err(err_code) => return $vm.push_error($state, err_code),
+            Ok(result) => $state.update(0, &result),
+            Err(err_code) => {
+                $state.pop();
+                return $vm.push_error($state, err_code);
+            }
         }
     };
 }
 
+#[inline]
 pub(crate) fn one_to_zero_index(v: &Var) -> Result<usize, Error> {
     let Variant::Int(index) = v.variant() else {
         return Err(E_TYPE);
@@ -185,9 +189,7 @@ impl VM {
             // Otherwise, start poppin' opcodes.
             // We panic here if we run out of opcodes, as that means there's a bug in either the
             // compiler or in opcode execution.
-            let op = state.next_op().expect(
-                "Unexpected program termination; opcode stream should end with RETURN or DONE",
-            );
+            let op = state.next_op();
 
             state.tick_count += 1;
 
@@ -349,30 +351,27 @@ impl VM {
                     state.update(0, &new_list);
                 }
                 Op::IndexSet => {
-                    // collection[index] = value
-                    let value = state.pop(); /* rhs value */
-
-                    // Index into range, must be int.
-                    let index = state.pop();
-
-                    let lhs = state.pop(); /* lhs except last index, should be list or str */
-
+                    let (rhs, index, lhs) = (state.pop(), state.pop(), state.peek_top());
                     let i = match one_to_zero_index(&index) {
                         Ok(i) => i,
-                        Err(e) => return self.push_error(state, e),
+                        Err(e) => {
+                            state.pop();
+                            return self.push_error(state, e);
+                        }
                     };
-                    match lhs.index_set(i, &value) {
+                    match lhs.index_set(i, &rhs) {
                         Ok(v) => {
-                            state.push(&v);
+                            state.update(0, &v);
                         }
                         Err(e) => {
+                            state.pop();
                             return self.push_error(state, e);
                         }
                     }
                 }
                 Op::MakeSingletonList => {
-                    let v = state.pop();
-                    state.push(&v_list(&[v]))
+                    let v = state.peek_top();
+                    state.update(0, &v_list(&[v]));
                 }
                 Op::PutTemp => {
                     state.top_mut().temp = state.peek_top();
@@ -419,7 +418,7 @@ impl VM {
                     // Explicit division by zero check to raise E_DIV.
                     // Note that LambdaMOO consider 1/0.0 to be E_DIV, but Rust permits it, creating
                     // `inf`. I'll follow Rust's lead here, unless it leads to problems.
-                    let divargs = state.peek(2);
+                    let divargs = state.peek_range(2);
                     if let Variant::Int(0) = divargs[1].variant() {
                         return self.push_error(state, E_DIV);
                     };
@@ -451,14 +450,17 @@ impl VM {
                     }
                 }
                 Op::Not => {
-                    let v = !state.pop().is_true();
-                    state.push(&v_bool(v));
+                    let v = !state.peek_top().is_true();
+                    state.update(0, &v_bool(v));
                 }
                 Op::UnaryMinus => {
-                    let v = state.pop();
+                    let v = state.peek_top();
                     match v.negative() {
-                        Err(e) => return self.push_error(state, e),
-                        Ok(v) => state.push(&v),
+                        Err(e) => {
+                            state.pop();
+                            return self.push_error(state, e);
+                        }
+                        Ok(v) => state.update(0, &v),
                     }
                 }
                 Op::Push(ident) => {
@@ -472,8 +474,7 @@ impl VM {
                     state.set_env(ident, &v);
                 }
                 Op::PushRef => {
-                    let peek = state.peek(2);
-                    let (index, list) = (peek[1].clone(), peek[0].clone());
+                    let (index, list) = state.peek2();
                     let index = match one_to_zero_index(&index) {
                         Ok(i) => i,
                         Err(e) => return self.push_error(state, e),
@@ -484,38 +485,51 @@ impl VM {
                     }
                 }
                 Op::Ref => {
-                    let index = state.pop();
-                    let l = state.pop();
+                    let (index, l) = (state.pop(), state.peek_top());
                     let index = match one_to_zero_index(&index) {
                         Ok(i) => i,
-                        Err(e) => return self.push_error(state, e),
+                        Err(e) => {
+                            state.pop();
+                            return self.push_error(state, e);
+                        }
                     };
+
                     match l.index(index) {
-                        Err(e) => return self.push_error(state, e),
-                        Ok(v) => state.push(&v),
+                        Err(e) => {
+                            state.pop();
+                            return self.push_error(state, e);
+                        }
+                        Ok(v) => state.update(0, &v),
                     }
                 }
                 Op::RangeRef => {
-                    let (to, from, base) = (state.pop(), state.pop(), state.pop());
+                    let (to, from, base) = (state.pop(), state.pop(), state.peek_top());
                     match (to.variant(), from.variant()) {
                         (Variant::Int(to), Variant::Int(from)) => match base.range(*from, *to) {
-                            Err(e) => return self.push_error(state, e),
-                            Ok(v) => state.push(&v),
+                            Err(e) => {
+                                state.pop();
+                                return self.push_error(state, e);
+                            }
+                            Ok(v) => state.update(0, &v),
                         },
                         (_, _) => return self.push_error(state, E_TYPE),
                     };
                 }
                 Op::RangeSet => {
                     let (value, to, from, base) =
-                        (state.pop(), state.pop(), state.pop(), state.pop());
+                        (state.pop(), state.pop(), state.pop(), state.peek_top());
                     match (to.variant(), from.variant()) {
                         (Variant::Int(to), Variant::Int(from)) => {
                             match base.rangeset(value, *from, *to) {
-                                Err(e) => return self.push_error(state, e),
-                                Ok(v) => state.push(&v),
+                                Err(e) => {
+                                    state.pop();
+                                    return self.push_error(state, e);
+                                }
+                                Ok(v) => state.update(0, &v),
                             }
                         }
                         _ => {
+                            state.pop();
                             return self.push_error(state, E_TYPE);
                         }
                     }
@@ -530,32 +544,48 @@ impl VM {
                     state.push(&v.clone());
                 }
                 Op::Length(offset) => {
-                    let vsr = &state.top().valstack;
-                    let v = &vsr[offset.0];
+                    let v = state.peek_abs(offset.0);
                     match v.len() {
-                        Ok(v) => state.push(&v),
+                        Ok(l) => state.push(&l),
                         Err(e) => return self.push_error(state, e),
                     }
                 }
                 Op::GetProp => {
-                    let (propname, obj) = (state.pop(), state.pop());
+                    let (propname, obj) = (state.pop(), state.peek_top());
 
-                    return self
+                    match self
                         .resolve_property(state, world_state, propname, obj)
-                        .await;
+                        .await
+                    {
+                        Ok(v) => state.update(0, &v),
+                        Err(e) => {
+                            state.pop();
+                            return self.push_error(state, e);
+                        }
+                    }
                 }
                 Op::PushGetProp => {
-                    let peeked = state.peek(2);
-                    let (propname, obj) = (peeked[1].clone(), peeked[0].clone());
-                    return self
+                    let (propname, obj) = state.peek2();
+                    match self
                         .resolve_property(state, world_state, propname, obj)
-                        .await;
+                        .await
+                    {
+                        Ok(v) => state.push(&v),
+                        Err(e) => return self.push_error(state, e),
+                    }
                 }
                 Op::PutProp => {
-                    let (rhs, propname, obj) = (state.pop(), state.pop(), state.pop());
-                    return self
+                    let (rhs, propname, obj) = (state.pop(), state.pop(), state.peek_top());
+                    match self
                         .set_property(state, world_state, propname, obj, rhs)
-                        .await;
+                        .await
+                    {
+                        Ok(v) => state.update(0, &v),
+                        Err(e) => {
+                            state.pop();
+                            return self.push_error(state, e);
+                        }
+                    }
                 }
                 Op::Fork { id, fv_offset } => {
                     // Delay time should be on stack

crates/kernel/src/vm/vm_util.rs

@@ -15,11 +15,12 @@
 use tracing::debug;
 
 use moor_values::model::world_state::WorldState;
+use moor_values::var::error::Error;
 use moor_values::var::error::Error::{E_INVIND, E_TYPE};
 use moor_values::var::variant::Variant;
 use moor_values::var::Var;
 
-use crate::vm::{ExecutionResult, VMExecState, VM};
+use crate::vm::{VMExecState, VM};
 
 impl VM {
     /// VM-level property resolution.
@@ -29,13 +30,13 @@ impl VM {
         world_state: &mut dyn WorldState,
         propname: Var,
         obj: Var,
-    ) -> ExecutionResult {
+    ) -> Result<Var, Error> {
         let Variant::Str(propname) = propname.variant() else {
-            return self.push_error(state, E_TYPE);
+            return Err(E_TYPE);
         };
 
         let Variant::Obj(obj) = obj.variant() else {
-            return self.push_error(state, E_INVIND);
+            return Err(E_INVIND);
         };
 
         let result = world_state
@@ -45,11 +46,10 @@ impl VM {
             Ok(v) => v,
             Err(e) => {
                 debug!(obj = ?obj, propname = propname.as_str(), "Error resolving property");
-                return self.push_error(state, e.to_error_code());
+                return Err(e.to_error_code());
             }
         };
-        state.push(&v);
-        ExecutionResult::More
+        Ok(v)
     }
 
     /// VM-level property assignment
@@ -60,11 +60,11 @@ impl VM {
         propname: Var,
         obj: Var,
         value: Var,
-    ) -> ExecutionResult {
+    ) -> Result<Var, Error> {
         let (propname, obj) = match (propname.variant(), obj.variant()) {
             (Variant::Str(propname), Variant::Obj(obj)) => (propname, obj),
             (_, _) => {
-                return self.push_error(state, E_TYPE);
+                return Err(E_TYPE);
             }
         };
 
@@ -73,13 +73,8 @@ impl VM {
             .await;
 
         match update_result {
-            Ok(()) => {
-                state.push(&value);
-            }
-            Err(e) => {
-                return self.push_error(state, e.to_error_code());
-            }
+            Ok(()) => Ok(value),
+            Err(e) => Err(e.to_error_code()),
         }
-        ExecutionResult::More
     }
 }