From 107e5980e221368f6dd95a8586eb32093f93bcb1 Mon Sep 17 00:00:00 2001 From: Ryan Daum Date: Sat, 6 Jan 2024 23:52:49 -0500 Subject: [PATCH] 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 | 15 ++-- crates/kernel/src/vm/exec_state.rs | 17 +++- crates/kernel/src/vm/vm_execute.rs | 132 ++++++++++++++++++----------- crates/kernel/src/vm/vm_util.rs | 27 +++--- 4 files changed, 114 insertions(+), 77 deletions(-) diff --git a/crates/kernel/src/vm/activation.rs b/crates/kernel/src/vm/activation.rs index f0ee04f9..de0f6f6d 100644 --- a/crates/kernel/src/vm/activation.rs +++ b/crates/kernel/src/vm/activation.rs @@ -319,13 +319,11 @@ impl Activation { } #[inline] - pub fn next_op(&mut self) -> Option { - 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 { + pub fn peek_range(&self, width: usize) -> Vec { 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(); diff --git a/crates/kernel/src/vm/exec_state.rs b/crates/kernel/src/vm/exec_state.rs index 6b478e12..43214fc5 100644 --- a/crates/kernel/src/vm/exec_state.rs +++ b/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 { - self.top().peek(amt) + pub(crate) fn peek_range(&self, amt: usize) -> Vec { + 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 { + pub(crate) fn next_op(&mut self) -> Op { self.top_mut().next_op() } diff --git a/crates/kernel/src/vm/vm_execute.rs b/crates/kernel/src/vm/vm_execute.rs index ca665adf..f149b5b5 100644 --- a/crates/kernel/src/vm/vm_execute.rs +++ b/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 { 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 diff --git a/crates/kernel/src/vm/vm_util.rs b/crates/kernel/src/vm/vm_util.rs index bfef381e..d1bbb1e0 100644 --- a/crates/kernel/src/vm/vm_util.rs +++ b/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 { 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 { 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 } }