From 98deb8f3fe8fd26827dd566b6ec7b6330b95e83c Mon Sep 17 00:00:00 2001 From: Michael Schimmel Date: Wed, 18 Feb 2026 02:01:27 +0100 Subject: [PATCH] Refactor VM evaluation to use macro The `eval` and `eval_observed` methods in the `VM` struct were very similar, with the primary difference being the call to the `VMObserver` trait. This commit refactors these methods into a single macro, `dispatch_eval!`, which reduces code duplication and improves maintainability. The `Value::TailCallRequest` variant was also updated to use `Box` for its contents, which helps keep the `Value` enum size consistent. Finally, the benchmarks in the `examples` directory have been updated to reflect minor performance changes resulting from these code modifications. --- examples/closure.myc | 2 +- examples/data.myc | 2 +- examples/extreme_capture.myc | 1 + examples/fib.myc | 2 +- examples/hof.myc | 2 +- src/ast/types.rs | 6 +- src/ast/vm.rs | 193 +++++++++++++++++++---------------- 7 files changed, 111 insertions(+), 97 deletions(-) diff --git a/examples/closure.myc b/examples/closure.myc index 35caedd..0efd59d 100644 --- a/examples/closure.myc +++ b/examples/closure.myc @@ -1,5 +1,5 @@ ;; Closure Scope Test -;; Benchmark: 9.0us +;; Benchmark: 9.4us ;; Output: 15 (do (def make-adder (fn [x] diff --git a/examples/data.myc b/examples/data.myc index f84de58..7f3e48e 100644 --- a/examples/data.myc +++ b/examples/data.myc @@ -1,5 +1,5 @@ ;; Complex Data Structure Test -;; Benchmark: 72.7us +;; Benchmark: 64.4us ;; Output: {:input 10, :name "Fibonacci", :output 55} (do (def fib (fn [n] diff --git a/examples/extreme_capture.myc b/examples/extreme_capture.myc index 0b33d0f..fb77389 100644 --- a/examples/extreme_capture.myc +++ b/examples/extreme_capture.myc @@ -1,3 +1,4 @@ +;; Benchmark: 25.9us ;; Output: 36 (do ;; Excessive capture test diff --git a/examples/fib.myc b/examples/fib.myc index f1145a3..8412ec6 100644 --- a/examples/fib.myc +++ b/examples/fib.myc @@ -1,5 +1,5 @@ ;; Fibonacci Recursive -;; Benchmark: 68.3us +;; Benchmark: 60.3us ;; Output: 55 (do (def fib (fn [n] diff --git a/examples/hof.myc b/examples/hof.myc index e3b1be7..367e052 100644 --- a/examples/hof.myc +++ b/examples/hof.myc @@ -1,5 +1,5 @@ ;; Higher-Order Function Example -;; Benchmark: 8.4us +;; Benchmark: 8.9us ;; Output: 25 (do (def apply (fn [f x] (f x))) diff --git a/src/ast/types.rs b/src/ast/types.rs index 436e8c8..0315ad4 100644 --- a/src/ast/types.rs +++ b/src/ast/types.rs @@ -70,7 +70,7 @@ pub enum Value { Function(Rc) -> Value>), Object(Rc), // For compiled Closures and other opaque types Cell(Rc>), // Boxed value for captures - TailCallRequest(Rc, Vec), // Internal: For TCO + TailCallRequest(Box<(Rc, Vec)>), // Internal: For TCO (Boxed to keep Value small) } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -146,7 +146,7 @@ impl Value { Value::Function { .. } => StaticType::Any, // Dynamic function Value::Object(o) => StaticType::Object(o.type_name()), Value::Cell(c) => c.borrow().static_type(), - Value::TailCallRequest(_, _) => panic!("Internal VM state leaked: TailCallRequest"), + Value::TailCallRequest(_) => panic!("Internal VM state leaked: TailCallRequest"), } } } @@ -181,7 +181,7 @@ impl fmt::Display for Value { Value::Function(_) => write!(f, ""), Value::Object(o) => write!(f, "<{}>", o.type_name()), Value::Cell(c) => write!(f, "{}", c.borrow()), - Value::TailCallRequest(_, _) => panic!("Internal VM state leaked: TailCallRequest"), + Value::TailCallRequest(_) => panic!("Internal VM state leaked: TailCallRequest"), } } } diff --git a/src/ast/vm.rs b/src/ast/vm.rs index 76472d6..fc9ddd8 100644 --- a/src/ast/vm.rs +++ b/src/ast/vm.rs @@ -30,8 +30,8 @@ struct CallFrame { /// Hook for observing VM execution (zero-cost when O::ACTIVE is false) pub trait VMObserver { const ACTIVE: bool = false; - fn before_eval(&mut self, _vm: &VM, _node: &Node, _indent: usize) {} - fn after_eval(&mut self, _vm: &VM, _node: &Node, _res: &Result, _indent: usize) {} + fn before_eval(&mut self, _vm: &VM, _node: &Node) {} + fn after_eval(&mut self, _vm: &VM, _node: &Node, _res: &Result) {} } /// Default observer that does nothing and should be optimized away @@ -41,15 +41,16 @@ impl VMObserver for NoOpObserver {} /// Observer that logs the execution tree and scope state pub struct TracingObserver { pub logs: Vec, + indent: usize, } impl TracingObserver { pub fn new() -> Self { - Self { logs: Vec::new() } + Self { logs: Vec::new(), indent: 0 } } - fn pad(&self, indent: usize) -> String { - "| ".repeat(indent) + fn pad(&self) -> String { + "| ".repeat(self.indent) } } @@ -62,18 +63,20 @@ impl Default for TracingObserver { impl VMObserver for TracingObserver { const ACTIVE: bool = true; - fn before_eval(&mut self, _vm: &VM, node: &Node, indent: usize) { - let pad = self.pad(indent); + fn before_eval(&mut self, _vm: &VM, node: &Node) { + let pad = self.pad(); self.logs.push(format!("{}{} [{}]: {{", pad, node.kind.display_name(), node.ty)); + self.indent += 1; } - fn after_eval(&mut self, vm: &VM, node: &Node, res: &Result, indent: usize) { - let pad = self.pad(indent); + fn after_eval(&mut self, vm: &VM, node: &Node, res: &Result) { + self.indent = self.indent.saturating_sub(1); + let pad = self.pad(); // Show scope state on certain nodes (like Delphi ShowScope) match &node.kind { BoundKind::DefLocal { .. } | BoundKind::Set { .. } | BoundKind::DefGlobal { .. } => { - let s_pad = self.pad(indent + 1); + let s_pad = format!("{}| ", pad); self.logs.push(format!("{}--- Scope Status ---", s_pad)); self.logs.push(format!("{}Stack (top 5): {:?}", s_pad, vm.stack.iter().rev().take(5).collect::>())); }, @@ -94,58 +97,16 @@ pub struct VM { frames: Vec, } -impl VM { - pub fn new(globals: Rc>>) -> Self { - Self { - stack: Vec::new(), - globals, - frames: Vec::new(), - } - } - - pub fn run(&mut self, root: &Node) -> Result { - self.run_with_observer(&mut NoOpObserver, root) - } - - pub fn run_with_observer(&mut self, observer: &mut O, root: &Node) -> Result { - self.stack.clear(); - self.frames.clear(); - - // Push global script frame - self.frames.push(CallFrame { - stack_base: 0, - closure: None, - }); - - let result = self.eval(observer, 0, root); - - self.frames.pop(); - result - } - - fn eval(&mut self, observer: &mut O, indent: usize, node: &Node) -> Result { - if O::ACTIVE { - observer.before_eval(self, node, indent); - } - - let result = self.eval_internal(observer, indent, node); - - if O::ACTIVE { - observer.after_eval(self, node, &result, indent); - } - - result - } - - fn eval_internal(&mut self, observer: &mut O, indent: usize, node: &Node) -> Result { - match &node.kind { +macro_rules! dispatch_eval { + ($self:ident, $node:ident, $eval_method:ident $(, $observer:ident)?) => { + match &$node.kind { BoundKind::Nop => Ok(Value::Void), BoundKind::Constant(v) => Ok(v.clone()), BoundKind::DefGlobal { global_index, value } => { - let val = self.eval(observer, indent + 1, value)?; + let val = $self.$eval_method($($observer,)? value)?; let idx = *global_index as usize; - let mut globals = self.globals.borrow_mut(); + let mut globals = $self.globals.borrow_mut(); if idx >= globals.len() { globals.resize(idx + 1, Value::Void); } @@ -153,30 +114,29 @@ impl VM { Ok(val) }, - BoundKind::Get(addr) => self.get_value(*addr), + BoundKind::Get(addr) => $self.get_value(*addr), BoundKind::Set { addr, value } => { - let val = self.eval(observer, indent + 1, value)?; - self.set_value(*addr, val.clone())?; + let val = $self.$eval_method($($observer,)? value)?; + $self.set_value(*addr, val.clone())?; Ok(val) }, BoundKind::DefLocal { slot, value, captured_by } => { - let val = self.eval(observer, indent + 1, value)?; + let val = $self.$eval_method($($observer,)? value)?; let final_val = if !captured_by.is_empty() { Value::Cell(Rc::new(RefCell::new(val))) } else { val }; - let frame = self.frames.last().ok_or("No call frame")?; + let frame = $self.frames.last().ok_or("No call frame")?; let abs_index = frame.stack_base + (*slot as usize); - // If it's the exact top of stack, push it. Otherwise, set it. - if abs_index == self.stack.len() { - self.stack.push(final_val.clone()); - } else if abs_index < self.stack.len() { - self.stack[abs_index] = final_val.clone(); + if abs_index == $self.stack.len() { + $self.stack.push(final_val.clone()); + } else if abs_index < $self.stack.len() { + $self.stack[abs_index] = final_val.clone(); } else { return Err(format!("Stack gap at local {}", slot)); } @@ -184,11 +144,11 @@ impl VM { }, BoundKind::If { cond, then_br, else_br } => { - let c = self.eval(observer, indent + 1, cond)?; + let c = $self.$eval_method($($observer,)? cond)?; if c.is_truthy() { - self.eval(observer, indent + 1, then_br) + $self.$eval_method($($observer,)? then_br) } else if let Some(e) = else_br { - self.eval(observer, indent + 1, e) + $self.$eval_method($($observer,)? e) } else { Ok(Value::Void) } @@ -197,7 +157,7 @@ impl VM { BoundKind::Block { exprs } => { let mut last = Value::Void; for e in exprs { - last = self.eval(observer, indent + 1, e)?; + last = $self.$eval_method($($observer,)? e)?; } Ok(last) }, @@ -205,7 +165,7 @@ impl VM { BoundKind::Lambda { param_count: _, upvalues, body } => { let mut captured = Vec::with_capacity(upvalues.len()); for addr in upvalues { - captured.push(self.capture_upvalue(*addr)?); + captured.push($self.capture_upvalue(*addr)?); } let closure = Closure { @@ -217,52 +177,50 @@ impl VM { }, BoundKind::TailCall { callee, args } => { - let func_val = self.eval(observer, indent + 1, callee)?; + let func_val = $self.$eval_method($($observer,)? callee)?; let mut arg_vals = Vec::with_capacity(args.len()); for arg in args { - arg_vals.push(self.eval(observer, indent + 1, arg)?); + arg_vals.push($self.$eval_method($($observer,)? arg)?); } match func_val { - Value::Object(obj) => Ok(Value::TailCallRequest(obj, arg_vals)), - // Native functions can't be TCO'd this way (they return value directly), - // so we just execute them and return Value. + Value::Object(obj) => Ok(Value::TailCallRequest(Box::new((obj, arg_vals)))), Value::Function(f) => Ok(f(arg_vals)), _ => Err(format!("Tail call target is not a function: {}", func_val)), } }, BoundKind::Call { callee, args } => { - let mut func_val = self.eval(observer, indent + 1, callee)?; + let mut func_val = $self.$eval_method($($observer,)? callee)?; let mut arg_vals = Vec::with_capacity(args.len()); for arg in args { - arg_vals.push(self.eval(observer, indent + 1, arg)?); + arg_vals.push($self.$eval_method($($observer,)? arg)?); } - // Trampoline Loop loop { match func_val { Value::Function(f) => return Ok(f(arg_vals)), Value::Object(obj) => { if let Some(closure) = obj.as_any().downcast_ref::() { - let old_stack_top = self.stack.len(); + let old_stack_top = $self.stack.len(); let closure_rc = Rc::new(closure.clone()); - self.stack.extend(arg_vals); // Moves arg_vals into stack + $self.stack.extend(arg_vals); - self.frames.push(CallFrame { + $self.frames.push(CallFrame { stack_base: old_stack_top, closure: Some(closure_rc.clone()), }); - let result = self.eval(observer, indent + 1, &closure.function_node); + let result = $self.$eval_method($($observer,)? &closure.function_node); - self.frames.pop(); - self.stack.truncate(old_stack_top); + $self.frames.pop(); + $self.stack.truncate(old_stack_top); match result { - Ok(Value::TailCallRequest(next_obj, next_args)) => { + Ok(Value::TailCallRequest(payload)) => { + let (next_obj, next_args) = *payload; func_val = Value::Object(next_obj); arg_vals = next_args; continue; @@ -282,7 +240,7 @@ impl VM { BoundKind::Tuple { elements } => { let mut vals = Vec::with_capacity(elements.len()); for e in elements { - vals.push(self.eval(observer, indent + 1, e)?); + vals.push($self.$eval_method($($observer,)? e)?); } Ok(Value::List(Rc::new(vals))) }, @@ -290,8 +248,8 @@ impl VM { BoundKind::Map { entries } => { let mut map = HashMap::new(); for (k, v) in entries { - let key = self.eval(observer, indent + 1, k)?; - let val = self.eval(observer, indent + 1, v)?; + let key = $self.$eval_method($($observer,)? k)?; + let val = $self.$eval_method($($observer,)? v)?; if let Value::Keyword(kw) = key { map.insert(kw, val); @@ -302,8 +260,63 @@ impl VM { Ok(Value::Record(Rc::new(map))) } } + }; +} + +impl VM { + pub fn new(globals: Rc>>) -> Self { + Self { + stack: Vec::new(), + globals, + frames: Vec::new(), + } } + pub fn run(&mut self, root: &Node) -> Result { + self.stack.clear(); + self.frames.clear(); + + self.frames.push(CallFrame { + stack_base: 0, + closure: None, + }); + + let result = self.eval(root); + + self.frames.pop(); + result + } + + pub fn run_with_observer(&mut self, observer: &mut O, root: &Node) -> Result { + self.stack.clear(); + self.frames.clear(); + + self.frames.push(CallFrame { + stack_base: 0, + closure: None, + }); + + let result = self.eval_observed(observer, root); + + self.frames.pop(); + result + } + + /// The pure, original, zero-cost hot path. + #[inline(always)] + fn eval(&mut self, node: &Node) -> Result { + dispatch_eval!(self, node, eval) + } + + /// The observed path for debugging. + fn eval_observed(&mut self, observer: &mut O, node: &Node) -> Result { + observer.before_eval(self, node); + let result = dispatch_eval!(self, node, eval_observed, observer); + observer.after_eval(self, node, &result); + result + } + + fn capture_upvalue(&mut self, addr: Address) -> Result>, String> { match addr { Address::Local(idx) => {