Skip to content

Commit

Permalink
Rework VM dispatching
Browse files Browse the repository at this point in the history 
Reworks so that the MOO execution loop doesn't get access to the whole
exec state, activation, etc. and simply gets the stack frame and returns
 the next operation to complete. Avoids some borrowing shenanigans that
 were getting vexing.
  • Loading branch information
@rdaum
rdaum committed Dec 21, 2024
1 parent 3056724 commit 540b20f
Show file tree
Hide file tree
Showing 6 changed files with 175 additions and 149 deletions.
14 changes: 7 additions & 7 deletions crates/kernel/src/builtins/bf_objects.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::bf_declare;
use crate::builtins::BfRet::{Ret, VmInstr};
use crate::builtins::{world_state_bf_err, BfCallState, BfErr, BfRet, BuiltinFunction};
use crate::tasks::VerbCall;
use crate::vm::ExecutionResult::ContinueVerb;
use crate::vm::ExecutionResult::DispatchVerb;

lazy_static! {
static ref INITIALIZE_SYM: Symbol = Symbol::mk("initialize");
Expand Down Expand Up @@ -158,7 +158,7 @@ fn bf_create(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
let bf_frame = bf_args.bf_frame_mut();
bf_frame.bf_trampoline = Some(BF_CREATE_OBJECT_TRAMPOLINE_DONE);
bf_frame.bf_trampoline_arg = Some(v_obj(new_obj.clone()));
Ok(VmInstr(ContinueVerb {
Ok(VmInstr(DispatchVerb {
permissions: bf_args.task_perms_who(),
resolved_verb,
binary,
Expand Down Expand Up @@ -269,7 +269,7 @@ fn bf_recycle(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
bf_frame.bf_trampoline = Some(BF_RECYCLE_TRAMPOLINE_CALL_EXITFUNC);
bf_frame.bf_trampoline_arg = Some(contents);

return Ok(VmInstr(ContinueVerb {
return Ok(VmInstr(DispatchVerb {
permissions: bf_args.task_perms_who(),
resolved_verb,
binary,
Expand Down Expand Up @@ -338,7 +338,7 @@ fn bf_recycle(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
bf_frame.bf_trampoline = Some(BF_RECYCLE_TRAMPOLINE_CALL_EXITFUNC);

// Call :exitfunc on the head object.
return Ok(VmInstr(ContinueVerb {
return Ok(VmInstr(DispatchVerb {
permissions: bf_args.task_perms_who(),
resolved_verb,
binary,
Expand Down Expand Up @@ -440,7 +440,7 @@ fn bf_move(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
let bf_frame = bf_args.bf_frame_mut();
bf_frame.bf_trampoline = Some(BF_MOVE_TRAMPOLINE_MOVE_CALL_EXITFUNC);
bf_frame.bf_trampoline_arg = None;
return Ok(VmInstr(ContinueVerb {
return Ok(VmInstr(DispatchVerb {
permissions: bf_args.task_perms_who(),
resolved_verb,
binary,
Expand Down Expand Up @@ -518,7 +518,7 @@ fn bf_move(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
bf_frame.bf_trampoline = Some(BF_MOVE_TRAMPOLINE_CALL_ENTERFUNC);
bf_frame.bf_trampoline_arg = None;

let continuation = ContinueVerb {
let continuation = DispatchVerb {
permissions: bf_args.task_perms_who(),
resolved_verb,
binary,
Expand Down Expand Up @@ -565,7 +565,7 @@ fn bf_move(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
bf_frame.bf_trampoline = Some(BF_MOVE_TRAMPOLINE_DONE);
bf_frame.bf_trampoline_arg = None;

return Ok(VmInstr(ContinueVerb {
return Ok(VmInstr(DispatchVerb {
permissions: bf_args.task_perms_who(),
resolved_verb,
binary,
Expand Down
91 changes: 68 additions & 23 deletions crates/kernel/src/tasks/vm_host.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,22 +248,47 @@ impl VmHost {
// Grant the loop its next tick slice.
self.vm_exec_state.tick_slice = self.max_ticks - self.vm_exec_state.tick_count;

let pre_exec_tick_count = self.vm_exec_state.tick_count;

// Actually invoke the VM, asking it to loop until it's ready to yield back to us.
let mut result = self.run_interpreter(&exec_params, world_state, session.clone());

let post_exec_tick_count = self.vm_exec_state.tick_count;
trace!(
task_id,
executed_ticks = post_exec_tick_count - pre_exec_tick_count,
?result,
"Executed ticks",
);
while self.is_running() {
match result {
ExecutionResult::More => return ContinueOk,
ExecutionResult::ContinueVerb {
ExecutionResult::PushError(e) => {
result = self.vm_exec_state.push_error(e);
continue;
}
ExecutionResult::RaiseError(e) => {
result = self.vm_exec_state.raise_error(e);
continue;
}
ExecutionResult::Return(value) => {
result = self
.vm_exec_state
.unwind_stack(FinallyReason::Return(value));
continue;
}
ExecutionResult::Unwind(fr) => {
result = self.vm_exec_state.unwind_stack(fr);
continue;
}
ExecutionResult::Pass(pass_args) => {
result = self
.vm_exec_state
.prepare_pass_verb(world_state, &pass_args);
continue;
}
ExecutionResult::PrepareVerbDispatch {
this,
verb_name,
args,
} => {
result = self
.vm_exec_state
.verb_dispatch(&exec_params, world_state, this, verb_name, args)
.unwrap_or_else(|e| ExecutionResult::PushError(e));

Check warning on line 288 in crates/kernel/src/tasks/vm_host.rs

View workflow job for this annotation

GitHub Actions / clippy

redundant closure 

warning: redundant closure
   --> crates/kernel/src/tasks/vm_host.rs:288:41
    |
288 |                         .unwrap_or_else(|e| ExecutionResult::PushError(e));
    |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `ExecutionResult::PushError`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
    = note: `#[warn(clippy::redundant_closure)]` on by default
continue;
}
ExecutionResult::DispatchVerb {
permissions,
resolved_verb,
binary,
Expand Down Expand Up @@ -310,7 +335,19 @@ impl VmHost {
);
continue;
}
ExecutionResult::DispatchFork(fork_request) => {
ExecutionResult::DispatchFork(delay, task_id, fv_offset) => {
let a = self.vm_exec_state.top().clone();
let parent_task_id = self.vm_exec_state.task_id;
let new_activation = a.clone();
let fork_request = Fork {
player: a.player.clone(),
progr: a.permissions.clone(),
parent_task_id,
delay,
activation: new_activation,
fork_vector_offset: fv_offset,
task_id,
};
return DispatchFork(fork_request);
}
ExecutionResult::Suspend(delay) => {
Expand Down Expand Up @@ -371,23 +408,31 @@ impl VmHost {
}

// Pick the right kind of execution flow depending on the activation -- builtin or MOO?
// (To avoid borrow issues on the exec state, we won't actually pull the frame out, just look
// at its type and execute the right function which will have to unpack the frame itself.
// this is a bit not-ideal but it's the best I can do right now.)
let result = match &self.vm_exec_state.top().frame {
Frame::Moo(_) => {
return moo_frame_execute(
vm_exec_params,
&mut self.vm_exec_state,
let mut tick_count = self.vm_exec_state.tick_count;
let tick_slice = self.vm_exec_state.tick_slice;
let activation = self.vm_exec_state.top_mut();

let (result, new_tick_count) = match &mut activation.frame {
Frame::Moo(fr) => {
let result = moo_frame_execute(
tick_slice,
&mut tick_count,
activation.permissions.clone(),
fr,
world_state,
session.clone(),
);
(result, tick_count)
}
Frame::Bf(_) => {
self.vm_exec_state
.reenter_builtin_function(vm_exec_params, world_state, session)
let result = self.vm_exec_state.reenter_builtin_function(
vm_exec_params,
world_state,
session,
);
(result, tick_count)
}
};
self.vm_exec_state.tick_count = new_tick_count;

result
}
Expand Down
26 changes: 22 additions & 4 deletions crates/kernel/src/vm/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use moor_compiler::{BuiltinId, Name};
use moor_compiler::{Offset, Program};
use moor_values::matching::command_parse::ParsedCommand;
use moor_values::model::VerbDef;
use moor_values::{Obj, Var};
use moor_values::{Error, List, Obj, Symbol, Var};
pub use vm_call::VerbExecutionRequest;
pub use vm_unwind::FinallyReason;

Expand Down Expand Up @@ -83,12 +83,30 @@ pub struct VmExecParams {
pub enum ExecutionResult {
/// Execution of this call stack is complete.
Complete(Var),
/// An error occurred during execution, that we might need to push to the stack and
/// potentially resume or unwind, depending on the context.
PushError(Error),
/// An error occurred during execution, that should definitely be treated as a proper "raise"
/// and unwind event unless there's a catch handler in place
RaiseError(Error),
/// An explicit stack unwind (for a reason other than a return.
Unwind(FinallyReason),
/// Explicit return, unwind stack
Return(Var),
/// Create the frames necessary to perform a `pass` up the inheritance chain.
Pass(List),
/// All is well. The task should let the VM continue executing.
More,
/// An exception was raised during execution.
Exception(FinallyReason),
/// Request dispatch to another verb
ContinueVerb {
/// Begin preparing to call a verb, by looking up the verb and preparing the dispatch.
PrepareVerbDispatch {
this: Var,
verb_name: Symbol,
args: List,
},
/// Perform the verb dispatch, building the stack frame and executing it.
DispatchVerb {
/// The applicable permissions context.
permissions: Obj,
/// The requested verb.
Expand All @@ -101,7 +119,7 @@ pub enum ExecutionResult {
command: Option<ParsedCommand>,
},
/// Request dispatch of a new task as a fork
DispatchFork(Fork),
DispatchFork(Option<Duration>, Option<Name>, Offset),
/// Request dispatch of a builtin function with the given arguments.
ContinueBuiltin {
builtin: BuiltinId,
Expand Down
Loading

0 comments on commit 540b20f

Please sign in to comment.