From 9dfb93b0436b2cb254fbbecfc3e0a655d120e51a Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Mon, 21 Aug 2023 15:40:00 +0200 Subject: [PATCH 1/2] Use a single virtual machine for complex executions --- crates/rune/src/cli/run.rs | 2 +- crates/rune/src/runtime/function.rs | 31 +++-- crates/rune/src/runtime/generator.rs | 10 +- crates/rune/src/runtime/stack.rs | 20 ++- crates/rune/src/runtime/stream.rs | 10 +- crates/rune/src/runtime/vm.rs | 92 +++++++++--- crates/rune/src/runtime/vm_call.rs | 76 ++++++++-- crates/rune/src/runtime/vm_error.rs | 4 + crates/rune/src/runtime/vm_execution.rs | 131 ++++++++---------- crates/rune/src/tests.rs | 2 +- ...rnal_fn_ptr.rs => vm_function_pointers.rs} | 48 ++++++- 11 files changed, 288 insertions(+), 138 deletions(-) rename crates/rune/src/tests/{vm_test_external_fn_ptr.rs => vm_function_pointers.rs} (52%) diff --git a/crates/rune/src/cli/run.rs b/crates/rune/src/cli/run.rs index 6cfc06fc8..68db5559c 100644 --- a/crates/rune/src/cli/run.rs +++ b/crates/rune/src/cli/run.rs @@ -317,7 +317,7 @@ async fn do_trace( mut limit: usize, ) -> Result where - T: AsMut + AsRef, + T: AsRef + AsMut, { let mut current_frame_len = execution.vm().call_frames().len(); diff --git a/crates/rune/src/runtime/function.rs b/crates/rune/src/runtime/function.rs index 7de6e164f..d98443a2c 100644 --- a/crates/rune/src/runtime/function.rs +++ b/crates/rune/src/runtime/function.rs @@ -823,26 +823,33 @@ impl FnOffset { /// /// This will cause a halt in case the vm being called into isn't the same /// as the context and unit of the function. + #[tracing::instrument(skip_all, fields(args, extra = extra.count(), ?self.offset, ?self.call, ?self.args, ?self.hash))] fn call_with_vm(&self, vm: &mut Vm, args: usize, extra: E) -> VmResult> where E: Args, { + tracing::trace!("calling"); + vm_try!(check_args(args, self.args)); - // Fast past, just allocate a call frame and keep running. - if let Call::Immediate = self.call { - if vm.is_same(&self.context, &self.unit) { - vm_try!(vm.push_call_frame(self.offset, args)); - vm_try!(extra.into_stack(vm.stack_mut())); - return VmResult::Ok(None); - } + let same_unit = matches!(self.call, Call::Immediate if vm.is_same_unit(&self.unit)); + let same_context = + matches!(self.call, Call::Immediate if vm.is_same_context(&self.context)); + + vm_try!(vm.push_call_frame(self.offset, args, !same_context)); + vm_try!(extra.into_stack(vm.stack_mut())); + + // Fast path, just allocate a call frame and keep running. + if same_context && same_unit { + tracing::trace!("same context and unit"); + return VmResult::Ok(None); } - let mut new_stack = vm_try!(vm.stack_mut().drain(args)).collect::(); - vm_try!(extra.into_stack(&mut new_stack)); - let mut vm = Vm::with_stack(self.context.clone(), self.unit.clone(), new_stack); - vm.set_ip(self.offset); - VmResult::Ok(Some(VmCall::new(self.call, vm))) + VmResult::Ok(Some(VmCall::new( + self.call, + (!same_context).then(|| self.context.clone()), + (!same_unit).then(|| self.unit.clone()), + ))) } } diff --git a/crates/rune/src/runtime/generator.rs b/crates/rune/src/runtime/generator.rs index 334a4dfb5..7820f0f68 100644 --- a/crates/rune/src/runtime/generator.rs +++ b/crates/rune/src/runtime/generator.rs @@ -10,14 +10,14 @@ use crate::runtime::{ /// A generator with a stored virtual machine. pub struct Generator where - T: AsMut, + T: AsRef + AsMut, { execution: Option>, } impl Generator where - T: AsMut, + T: AsRef + AsMut, { /// Construct a generator from a virtual machine. pub(crate) fn new(vm: T) -> Self { @@ -108,7 +108,7 @@ impl iter::Iterator for GeneratorIterator { impl fmt::Debug for Generator where - T: AsMut, + T: AsRef + AsMut, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Generator") @@ -119,11 +119,11 @@ where impl Named for Generator where - T: AsMut, + T: AsRef + AsMut, { const BASE_NAME: RawStr = RawStr::from_str("Generator"); } -impl InstallWith for Generator where T: AsMut {} +impl InstallWith for Generator where T: AsRef + AsMut {} from_value!(Generator, into_generator); diff --git a/crates/rune/src/runtime/stack.rs b/crates/rune/src/runtime/stack.rs index e56434942..d486e9c0d 100644 --- a/crates/rune/src/runtime/stack.rs +++ b/crates/rune/src/runtime/stack.rs @@ -1,7 +1,7 @@ use core::array; use core::fmt; use core::iter; -use core::mem; +use core::mem::replace; use core::slice; use crate::no_std::borrow::Cow; @@ -111,6 +111,14 @@ impl Stack { self.stack.len() } + pub(crate) fn stack_size(&self) -> Result { + let Some(size) = self.stack.len().checked_sub(self.stack_bottom) else { + return Err(StackError); + }; + + Ok(size) + } + /// Perform a raw access over the stack. /// /// This ignores [stack_bottom] and will just check that the given slice @@ -345,9 +353,12 @@ impl Stack { /// This is used internally when returning from a call frame. /// /// Returns the old stack top. + #[tracing::instrument(skip_all)] pub(crate) fn swap_stack_bottom(&mut self, count: usize) -> Result { + tracing::trace!(stack = ?self.stack.len(), self.stack_bottom, count); + match self.stack.len().checked_sub(count) { - Some(new_top) => Ok(mem::replace(&mut self.stack_bottom, new_top)), + Some(new_top) => Ok(replace(&mut self.stack_bottom, new_top)), None => Err(StackError), } } @@ -356,10 +367,7 @@ impl Stack { // at the point of return. #[tracing::instrument(skip_all)] pub(crate) fn check_stack_top(&self) -> Result<(), StackError> { - tracing::trace!( - stack_len = self.stack.len(), - stack_bottom = self.stack_bottom, - ); + tracing::trace!(stack = self.stack.len(), self.stack_bottom,); if self.stack.len() == self.stack_bottom { return Ok(()); diff --git a/crates/rune/src/runtime/stream.rs b/crates/rune/src/runtime/stream.rs index 015532288..4c5f33c93 100644 --- a/crates/rune/src/runtime/stream.rs +++ b/crates/rune/src/runtime/stream.rs @@ -9,14 +9,14 @@ use crate::runtime::{ /// A stream with a stored virtual machine. pub struct Stream where - T: AsMut, + T: AsRef + AsMut, { execution: Option>, } impl Stream where - T: AsMut, + T: AsRef + AsMut, { /// Construct a stream from a virtual machine. pub(crate) fn new(vm: T) -> Self { @@ -83,16 +83,16 @@ impl Stream<&mut Vm> { impl Named for Stream where - T: AsMut, + T: AsRef + AsMut, { const BASE_NAME: RawStr = RawStr::from_str("Stream"); } -impl InstallWith for Stream where T: AsMut {} +impl InstallWith for Stream where T: AsRef + AsMut {} impl fmt::Debug for Stream where - T: AsMut, + T: AsRef + AsMut, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Stream") diff --git a/crates/rune/src/runtime/vm.rs b/crates/rune/src/runtime/vm.rs index 27f940388..32ed314d9 100644 --- a/crates/rune/src/runtime/vm.rs +++ b/crates/rune/src/runtime/vm.rs @@ -1,6 +1,6 @@ use core::cmp::Ordering; use core::fmt; -use core::mem; +use core::mem::{replace, swap}; use core::ops; use core::slice; @@ -132,6 +132,16 @@ impl Vm { Arc::ptr_eq(&self.context, context) && Arc::ptr_eq(&self.unit, unit) } + /// Test if the virtual machine is the same context. + pub fn is_same_context(&self, context: &Arc) -> bool { + Arc::ptr_eq(&self.context, context) + } + + /// Test if the virtual machine is the same context. + pub fn is_same_unit(&self, unit: &Arc) -> bool { + Arc::ptr_eq(&self.unit, unit) + } + /// Set the current instruction pointer. #[inline] pub fn set_ip(&mut self, ip: usize) { @@ -156,12 +166,24 @@ impl Vm { &mut self.stack } + /// Access the context related to the virtual machine mutably. + #[inline] + pub fn context_mut(&mut self) -> &mut Arc { + &mut self.context + } + /// Access the context related to the virtual machine. #[inline] pub fn context(&self) -> &Arc { &self.context } + /// Access the underlying unit of the virtual machine mutablys. + #[inline] + pub fn unit_mut(&mut self) -> &mut Arc { + &mut self.unit + } + /// Access the underlying unit of the virtual machine. #[inline] pub fn unit(&self) -> &Arc { @@ -593,32 +615,58 @@ impl Vm { /// /// This will cause the `args` number of elements on the stack to be /// associated and accessible to the new call frame. - pub(crate) fn push_call_frame(&mut self, ip: usize, args: usize) -> Result<(), VmErrorKind> { - let stack_top = self.stack.swap_stack_bottom(args)?; + #[tracing::instrument(skip(self), fields(call_frames = self.call_frames.len(), stack_bottom = self.stack.stack_bottom(), stack = self.stack.len(), self.ip))] + pub(crate) fn push_call_frame( + &mut self, + ip: usize, + args: usize, + isolated: bool, + ) -> Result<(), VmErrorKind> { + tracing::trace!("pushing call frame"); - self.call_frames.push(CallFrame { - ip: self.ip, - stack_bottom: stack_top, - }); + let stack_bottom = self.stack.swap_stack_bottom(args)?; + let ip = replace(&mut self.ip, ip); - self.ip = ip; + let frame = CallFrame { + ip, + stack_bottom, + isolated, + }; + + self.call_frames.push(frame); Ok(()) } + /// Pop a call frame from an internal call, which needs the current stack + /// pointer to be returned and does not check for context isolation through + /// [`CallFrame::isolated`]. + #[tracing::instrument(skip(self), fields(call_frames = self.call_frames.len(), stack_bottom = self.stack.stack_bottom(), stack = self.stack.len(), self.ip))] + pub(crate) fn pop_call_frame_from_call(&mut self) -> Result, VmErrorKind> { + tracing::trace!("popping call frame from call"); + + let Some(frame) = self.call_frames.pop() else { + return Ok(None); + }; + + tracing::trace!(?frame); + self.stack.pop_stack_top(frame.stack_bottom)?; + Ok(Some(replace(&mut self.ip, frame.ip))) + } + /// Pop a call frame and return it. - #[tracing::instrument(skip_all)] - fn pop_call_frame(&mut self) -> Result { - let frame = match self.call_frames.pop() { - Some(frame) => frame, - None => { - self.stack.check_stack_top()?; - return Ok(true); - } + #[tracing::instrument(skip(self), fields(call_frames = self.call_frames.len(), stack_bottom = self.stack.stack_bottom(), stack = self.stack.len(), self.ip))] + pub(crate) fn pop_call_frame(&mut self) -> Result { + tracing::trace!("popping call frame"); + + let Some(frame) = self.call_frames.pop() else { + self.stack.check_stack_top()?; + return Ok(true); }; + tracing::trace!(?frame); self.stack.pop_stack_top(frame.stack_bottom)?; self.ip = frame.ip; - Ok(false) + Ok(frame.isolated) } /// Implementation of getting a string index on an object-like type. @@ -1161,7 +1209,7 @@ impl Vm { false } Call::Immediate => { - self.push_call_frame(offset, args)?; + self.push_call_frame(offset, args, false)?; true } Call::Stream => { @@ -1540,7 +1588,7 @@ impl Vm { fn op_replace(&mut self, offset: usize) -> VmResult<()> { let mut value = vm_try!(self.stack.pop()); let stack_value = vm_try!(self.stack.at_offset_mut(offset)); - mem::swap(stack_value, &mut value); + swap(stack_value, &mut value); VmResult::Ok(()) } @@ -2007,6 +2055,7 @@ impl Vm { } #[inline] + #[tracing::instrument(skip(self))] fn op_return_internal( &mut self, return_value: Value, @@ -2083,6 +2132,7 @@ impl Vm { } #[cfg_attr(feature = "bench", inline(never))] + #[tracing::instrument(skip(self))] fn op_return_unit(&mut self) -> Result { let exit = self.pop_call_frame()?; self.stack.push(()); @@ -2888,6 +2938,7 @@ impl Vm { } #[cfg_attr(feature = "bench", inline(never))] + #[tracing::instrument(skip(self))] fn op_call_fn(&mut self, args: usize) -> VmResult> { let function = vm_try!(self.stack.pop()); @@ -3272,6 +3323,9 @@ pub struct CallFrame { /// I.e. a function should not be able to manipulate the size of any other /// stack than its own. pub stack_bottom: usize, + /// Indicates that the call frame is isolated and should force an exit into + /// the vm execution context. + pub isolated: bool, } /// Clear stack on drop. diff --git a/crates/rune/src/runtime/vm_call.rs b/crates/rune/src/runtime/vm_call.rs index 6c49fd198..eef712b02 100644 --- a/crates/rune/src/runtime/vm_call.rs +++ b/crates/rune/src/runtime/vm_call.rs @@ -1,38 +1,90 @@ -use crate::runtime::{Call, Future, Generator, Stream, Value, Vm, VmExecution, VmResult}; +use crate::no_std::sync::Arc; + +use crate::runtime::vm_execution::VmExecutionState; +use crate::runtime::{ + Call, Future, Generator, RuntimeContext, Stack, Stream, Unit, Value, Vm, VmErrorKind, + VmExecution, VmResult, +}; /// An instruction to push a virtual machine to the execution. #[derive(Debug)] +#[must_use = "The construction of a vm call leaves the virtual machine stack in an intermediate state, VmCall::into_execution must be called to fix it"] pub(crate) struct VmCall { call: Call, - vm: Vm, + /// Is set if the context differs for the call for the current virtual machine. + context: Option>, + /// Is set if the unit differs for the call for the current virtual machine. + unit: Option>, } impl VmCall { - /// Construct a new nested vm call. - pub(crate) fn new(call: Call, vm: Vm) -> Self { - Self { call, vm } + pub(crate) fn new( + call: Call, + context: Option>, + unit: Option>, + ) -> Self { + Self { + call, + context, + unit, + } } /// Encode the push itno an execution. + #[tracing::instrument(skip_all)] pub(crate) fn into_execution(self, execution: &mut VmExecution) -> VmResult<()> where - T: AsMut, + T: AsRef + AsMut, { let value = match self.call { Call::Async => { - let mut execution = self.vm.into_execution(); + let vm = vm_try!(self.build_vm(execution)); + let mut execution = vm.into_execution(); Value::from(Future::new(async move { execution.async_complete().await })) } Call::Immediate => { - execution.push_vm(self.vm); + execution.push_state(VmExecutionState { + context: self.context, + unit: self.unit, + }); + return VmResult::Ok(()); } - Call::Stream => Value::from(Stream::new(self.vm)), - Call::Generator => Value::from(Generator::new(self.vm)), + Call::Stream => { + let vm = vm_try!(self.build_vm(execution)); + Value::from(Stream::new(vm)) + } + Call::Generator => { + let vm = vm_try!(self.build_vm(execution)); + Value::from(Generator::new(vm)) + } }; - let vm = execution.vm_mut(); - vm.stack_mut().push(value); + execution.vm_mut().stack_mut().push(value); VmResult::Ok(()) } + + #[tracing::instrument(skip_all)] + fn build_vm(self, execution: &mut VmExecution) -> VmResult + where + T: AsRef + AsMut, + { + let vm = execution.vm_mut(); + let args = vm_try!(vm.stack_mut().stack_size()); + + tracing::trace!(args); + + let new_stack = vm_try!(vm.stack_mut().drain(args)).collect::(); + + let Some(ip) = vm_try!(vm.pop_call_frame_from_call()) else { + return VmResult::err(VmErrorKind::MissingCallFrame); + }; + + let context = self.context.unwrap_or_else(|| vm.context().clone()); + let unit = self.unit.unwrap_or_else(|| vm.unit().clone()); + + let mut vm = Vm::with_stack(context, unit, new_stack); + vm.set_ip(ip); + VmResult::Ok(vm) + } } diff --git a/crates/rune/src/runtime/vm_error.rs b/crates/rune/src/runtime/vm_error.rs index c51ae53a9..da32c6ee9 100644 --- a/crates/rune/src/runtime/vm_error.rs +++ b/crates/rune/src/runtime/vm_error.rs @@ -588,6 +588,7 @@ pub(crate) enum VmErrorKind { lhs: f64, rhs: f64, }, + MissingCallFrame, } impl fmt::Display for VmErrorKind { @@ -785,6 +786,9 @@ impl fmt::Display for VmErrorKind { "Cannot perform a comparison of the floats {lhs} and {rhs}", ) } + VmErrorKind::MissingCallFrame => { + write!(f, "Missing call frame for internal vm call") + } } } } diff --git a/crates/rune/src/runtime/vm_execution.rs b/crates/rune/src/runtime/vm_execution.rs index 716c45781..72da6dee4 100644 --- a/crates/rune/src/runtime/vm_execution.rs +++ b/crates/rune/src/runtime/vm_execution.rs @@ -1,12 +1,14 @@ use core::fmt; use core::future::Future; -use core::mem::take; +use core::mem::{replace, take}; use crate::no_std::prelude::*; +use crate::no_std::sync::Arc; use crate::runtime::budget; use crate::runtime::{ - Generator, GeneratorState, Stream, Value, Vm, VmErrorKind, VmHalt, VmHaltInfo, VmResult, + Generator, GeneratorState, RuntimeContext, Stream, Unit, Value, Vm, VmErrorKind, VmHalt, + VmHaltInfo, VmResult, }; use crate::shared::AssertSend; @@ -33,51 +35,37 @@ impl fmt::Display for ExecutionState { } } +pub(crate) struct VmExecutionState { + pub(crate) context: Option>, + pub(crate) unit: Option>, +} + /// The execution environment for a virtual machine. /// /// When an execution is dropped, the stack of the stack of the head machine /// will be cleared. pub struct VmExecution where - T: AsMut, + T: AsRef + AsMut, { /// The current head vm which holds the execution. head: T, /// The state of an execution. state: ExecutionState, - /// The current stack of virtual machines and the execution state that must - /// be restored once one is popped. - vms: Vec<(Vm, ExecutionState)>, -} - -macro_rules! vm { - ($slf:expr) => { - match $slf.vms.last() { - Some((vm, _)) => vm, - None => $slf.head.as_ref(), - } - }; -} - -macro_rules! vm_mut { - ($slf:expr) => { - match $slf.vms.last_mut() { - Some((vm, _)) => vm, - None => $slf.head.as_mut(), - } - }; + /// Indicates the current stack of suspended contexts. + states: Vec, } impl VmExecution where - T: AsMut, + T: AsRef + AsMut, { /// Construct an execution from a virtual machine. pub(crate) fn new(head: T) -> Self { Self { head, - vms: vec![], state: ExecutionState::Initial, + states: vec![], } } @@ -156,16 +144,13 @@ where } /// Get a reference to the current virtual machine. - pub fn vm(&self) -> &Vm - where - T: AsRef, - { - vm!(self) + pub fn vm(&self) -> &Vm { + self.head.as_ref() } /// Get a mutable reference the current virtual machine. pub fn vm_mut(&mut self) -> &mut Vm { - vm_mut!(self) + self.head.as_mut() } /// Complete the current execution without support for async instructions. @@ -203,7 +188,7 @@ where }); } - vm_mut!(self).stack_mut().push(value); + self.head.as_mut().stack_mut().push(value); self.inner_async_resume().await } @@ -213,7 +198,7 @@ where /// it while returning a unit from the current `yield`. pub async fn async_resume(&mut self) -> VmResult { if matches!(self.state, ExecutionState::Resumed) { - vm_mut!(self).stack_mut().push(Value::EmptyTuple); + self.head.as_mut().stack_mut().push(Value::EmptyTuple); } else { self.state = ExecutionState::Resumed; } @@ -223,10 +208,9 @@ where async fn inner_async_resume(&mut self) -> VmResult { loop { - let len = self.vms.len(); - let vm = vm_mut!(self); + let vm = self.head.as_mut(); - match vm_try!(Self::run(vm)) { + match vm_try!(vm.run().with_vm(vm)) { VmHalt::Exited => (), VmHalt::Awaited(awaited) => { vm_try!(awaited.into_vm(vm).await); @@ -247,12 +231,12 @@ where } } - if len == 0 { + if self.states.is_empty() { let value = vm_try!(self.end()); return VmResult::Ok(GeneratorState::Complete(value)); } - vm_try!(self.pop_vm()); + vm_try!(self.pop_state()); } } @@ -267,7 +251,7 @@ where }); } - vm_mut!(self).stack_mut().push(value); + self.head.as_mut().stack_mut().push(value); self.inner_resume() } @@ -280,7 +264,7 @@ where #[tracing::instrument(skip_all)] pub fn resume(&mut self) -> VmResult { if matches!(self.state, ExecutionState::Resumed) { - vm_mut!(self).stack_mut().push(Value::EmptyTuple); + self.head.as_mut().stack_mut().push(Value::EmptyTuple); } else { self.state = ExecutionState::Resumed; } @@ -290,10 +274,10 @@ where fn inner_resume(&mut self) -> VmResult { loop { - let len = self.vms.len(); - let vm = vm_mut!(self); + let len = self.states.len(); + let vm = self.head.as_mut(); - match vm_try!(Self::run(vm)) { + match vm_try!(vm.run().with_vm(vm)) { VmHalt::Exited => (), VmHalt::VmCall(vm_call) => { vm_try!(vm_call.into_execution(self)); @@ -315,7 +299,7 @@ where return VmResult::Ok(GeneratorState::Complete(value)); } - vm_try!(self.pop_vm()); + vm_try!(self.pop_state()); } } @@ -324,10 +308,10 @@ where /// /// If any async instructions are encountered, this will error. pub fn step(&mut self) -> VmResult> { - let len = self.vms.len(); - let vm = vm_mut!(self); + let len = self.states.len(); + let vm = self.head.as_mut(); - match vm_try!(budget::with(1, || Self::run(vm)).call()) { + match vm_try!(budget::with(1, || vm.run().with_vm(vm)).call()) { VmHalt::Exited => (), VmHalt::VmCall(vm_call) => { vm_try!(vm_call.into_execution(self)); @@ -346,17 +330,16 @@ where return VmResult::Ok(Some(value)); } - vm_try!(self.pop_vm()); + vm_try!(self.pop_state()); VmResult::Ok(None) } /// Step the single execution for one step with support for async /// instructions. pub async fn async_step(&mut self) -> VmResult> { - let len = self.vms.len(); - let vm = vm_mut!(self); + let vm = self.head.as_mut(); - match vm_try!(budget::with(1, || Self::run(vm)).call()) { + match vm_try!(budget::with(1, || vm.run().with_vm(vm)).call()) { VmHalt::Exited => (), VmHalt::Awaited(awaited) => { vm_try!(awaited.into_vm(vm).await); @@ -374,12 +357,12 @@ where } } - if len == 0 { + if self.states.is_empty() { let value = vm_try!(self.end()); return VmResult::Ok(Some(value)); } - vm_try!(self.pop_vm()); + vm_try!(self.pop_state()); VmResult::Ok(None) } @@ -387,34 +370,38 @@ where pub(crate) fn end(&mut self) -> VmResult { let vm = self.head.as_mut(); let value = vm_try!(vm.stack_mut().pop()); - debug_assert!(self.vms.is_empty(), "execution vms should be empty"); + debug_assert!(self.states.is_empty(), "execution vms should be empty"); VmResult::Ok(value) } /// Push a virtual machine state onto the execution. - pub(crate) fn push_vm(&mut self, vm: Vm) { - self.vms.push((vm, self.state)); - self.state = ExecutionState::Initial; + #[tracing::instrument(skip_all)] + pub(crate) fn push_state(&mut self, state: VmExecutionState) { + tracing::trace!("pushing suspended state"); + let vm = self.head.as_mut(); + let context = state.context.map(|c| replace(vm.context_mut(), c)); + let unit = state.unit.map(|u| replace(vm.unit_mut(), u)); + self.states.push(VmExecutionState { context, unit }); } /// Pop a virtual machine state from the execution and transfer the top of /// the stack from the popped machine. - fn pop_vm(&mut self) -> VmResult<()> { - let (mut from, state) = vm_try!(self.vms.pop().ok_or(VmErrorKind::NoRunningVm)); + #[tracing::instrument(skip_all)] + fn pop_state(&mut self) -> VmResult<()> { + tracing::trace!("popping suspended state"); - let stack = from.stack_mut(); - let value = vm_try!(stack.pop()); - debug_assert!(stack.is_empty(), "vm stack not clean"); + let state = vm_try!(self.states.pop().ok_or(VmErrorKind::NoRunningVm)); + let vm = self.head.as_mut(); - let onto = vm_mut!(self); - onto.stack_mut().push(value); - self.state = state; - VmResult::Ok(()) - } + if let Some(context) = state.context { + *vm.context_mut() = context; + } - #[inline] - fn run(vm: &mut Vm) -> VmResult { - vm.run().with_vm(vm) + if let Some(unit) = state.unit { + *vm.unit_mut() = unit; + } + + VmResult::Ok(()) } } @@ -426,7 +413,7 @@ impl VmExecution<&mut Vm> { VmExecution { head, - vms: self.vms, + states: self.states, state: self.state, } } diff --git a/crates/rune/src/tests.rs b/crates/rune/src/tests.rs index 859aab83b..8cfd5694c 100644 --- a/crates/rune/src/tests.rs +++ b/crates/rune/src/tests.rs @@ -414,6 +414,7 @@ mod vm_closures; mod vm_const_exprs; mod vm_early_termination; mod vm_function; +mod vm_function_pointers; mod vm_general; mod vm_generators; mod vm_is; @@ -425,7 +426,6 @@ mod vm_option; mod vm_pat; mod vm_result; mod vm_streams; -mod vm_test_external_fn_ptr; mod vm_test_from_value_derive; mod vm_test_imports; mod vm_test_instance_fns; diff --git a/crates/rune/src/tests/vm_test_external_fn_ptr.rs b/crates/rune/src/tests/vm_function_pointers.rs similarity index 52% rename from crates/rune/src/tests/vm_test_external_fn_ptr.rs rename to crates/rune/src/tests/vm_function_pointers.rs index 81fb0b44d..6d0d3b4d5 100644 --- a/crates/rune/src/tests/vm_test_external_fn_ptr.rs +++ b/crates/rune/src/tests/vm_function_pointers.rs @@ -1,13 +1,11 @@ prelude!(); +/// Here we're constructing a unit-specific function pointer and ensuring that +/// the vm execution can handle it correctly. #[test] -fn test_external_function() -> Result<()> { +fn vm_execution_unit_fn() -> Result<()> { let context = Context::with_default_modules()?; - // NB: here we test passing the function from one virtual machine instance - // into another, making sure that the function holds everything it needs to - // be called. - let function: Function = run( &context, r#" @@ -31,6 +29,46 @@ fn test_external_function() -> Result<()> { Ok(()) } +/// Here we're constructing multiple different kinds of function pointers and +/// ensuring that the vm execution can handle them correctly. +#[test] +fn vm_execution_with_complex_external() -> Result<()> { + let mut m = Module::new(); + m.function(["external"], || 42i64)?; + + let mut c1 = Context::with_default_modules()?; + c1.install(m)?; + + let c2 = Context::with_default_modules()?; + + let function: Function = run( + &c1, + r#" + fn unit() { 84 } + fn function() { (external, unit) } + pub fn main() { function } + "#, + ["main"], + (), + )?; + + let (o1, o2): (i64, i64) = run( + &c2, + r#" + pub fn main(f) { + let (f1, f2) = f(); + (f1(), f2()) + } + "#, + ["main"], + (function,), + )?; + + assert_eq!(o1, 42); + assert_eq!(o2, 84); + Ok(()) +} + #[test] fn test_external_generator() -> Result<()> { let context = Context::with_default_modules()?; From cd564998a383214dde2a7bcb0f303d626ff34dcc Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Mon, 21 Aug 2023 16:19:47 +0200 Subject: [PATCH 2/2] Add benchmark --- benches/benches/bench_main.rs | 2 ++ .../benches/benchmarks/external_functions.rs | 36 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 benches/benches/benchmarks/external_functions.rs diff --git a/benches/benches/bench_main.rs b/benches/benches/bench_main.rs index 4530b00c3..8f0a4f427 100644 --- a/benches/benches/bench_main.rs +++ b/benches/benches/bench_main.rs @@ -49,6 +49,7 @@ mod benchmarks { pub mod aoc_2020_1a; pub mod aoc_2020_1b; pub mod brainfuck; + pub mod external_functions; pub mod fib; } @@ -59,4 +60,5 @@ criterion::criterion_main! { benchmarks::aoc_2020_19b::benches, benchmarks::brainfuck::benches, benchmarks::fib::benches, + benchmarks::external_functions::benches, } diff --git a/benches/benches/benchmarks/external_functions.rs b/benches/benches/benchmarks/external_functions.rs new file mode 100644 index 000000000..6b5f86971 --- /dev/null +++ b/benches/benches/benchmarks/external_functions.rs @@ -0,0 +1,36 @@ +//! Benchmark external function calls. + +use criterion::Criterion; + +criterion::criterion_group!(benches, external_functions); + +fn external_functions(b: &mut Criterion) { + let mut vm1 = rune_vm! { + fn a() { + 79 + } + + fn b(f) { + f() + } + + pub fn main() { + (a, b) + } + }; + + let mut vm2 = rune_vm! { + pub fn main(argument) { + let (a, b) = argument; + assert_eq!(b(a), 79); + } + }; + + let entry = rune::Hash::type_hash(["main"]); + + b.bench_function("external_functions", |b| { + let output = vm1.call(entry, ()).expect("failed to fetch function"); + + b.iter(|| vm2.call(entry, (output.clone(),)).expect("failed call")); + }); +}