A bit of cleanup in VM external interfaces

Renames for clarity.
Move around some structs to make boundaries clearer.
Some typos

crates/kernel/benches/vm_benches.rs

@@ -27,8 +27,9 @@ use moor_kernel::builtins::BuiltinRegistry;
 use moor_kernel::config::FeaturesConfig;
 use moor_kernel::tasks::sessions::{NoopClientSession, Session};
 use moor_kernel::tasks::task_scheduler_client::TaskSchedulerClient;
-use moor_kernel::tasks::vm_host::{VMHostResponse, VmHost};
+use moor_kernel::tasks::vm_host::VmHost;
 use moor_kernel::tasks::VerbCall;
+use moor_kernel::vm::VMHostResponse;
 use moor_values::model::CommitResult;
 use moor_values::model::VerbArgsSpec;
 use moor_values::model::{BinaryType, VerbFlag};

crates/kernel/src/builtins/bf_server.rs

@@ -400,7 +400,7 @@ fn bf_suspend(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
         Some(Duration::from_secs_f64(seconds))
     };
 
-    Ok(VmInstr(ExecutionResult::Suspend(seconds)))
+    Ok(VmInstr(ExecutionResult::TaskSuspend(seconds)))
 }
 bf_declare!(suspend, bf_suspend);
 
@@ -428,7 +428,7 @@ fn bf_read(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
         }
     }
 
-    Ok(VmInstr(ExecutionResult::NeedInput))
+    Ok(VmInstr(ExecutionResult::TaskNeedInput))
 }
 bf_declare!(read, bf_read);
 
@@ -669,7 +669,7 @@ fn bf_call_function(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
         .ok_or(BfErr::Code(E_ARGS))?;
 
     // Then ask the scheduler to run the function as a continuation of what we're doing now.
-    Ok(VmInstr(ExecutionResult::ContinueBuiltin {
+    Ok(VmInstr(ExecutionResult::DispatchBuiltin {
         builtin,
         arguments: arguments.clone(),
     }))
@@ -946,7 +946,7 @@ fn bf_eval(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
             bf_frame.bf_trampoline = Some(BF_SERVER_EVAL_TRAMPOLINE_RESUME);
             // Now we have to construct things to set up for eval. Which means tramping through with a
             // setup-for-eval result here.
-            Ok(VmInstr(ExecutionResult::PerformEval {
+            Ok(VmInstr(ExecutionResult::DispatchEval {
                 permissions: bf_args.task_perms_who(),
                 player: bf_args.exec_state.top().player.clone(),
                 program,

crates/kernel/src/tasks/mod.rs

@@ -140,8 +140,9 @@ pub mod vm_test_utils {
     use crate::builtins::BuiltinRegistry;
     use crate::config::FeaturesConfig;
     use crate::tasks::sessions::Session;
-    use crate::tasks::vm_host::{VMHostResponse, VmHost};
+    use crate::tasks::vm_host::VmHost;
     use crate::tasks::VerbCall;
+    use crate::vm::VMHostResponse;
     use moor_values::tasks::Exception;
 
     pub type ExecResult = Result<Var, Exception>;

crates/kernel/src/tasks/task.rs

@@ -50,8 +50,9 @@ use crate::builtins::BuiltinRegistry;
 use crate::config::{Config, FeaturesConfig};
 use crate::tasks::sessions::Session;
 use crate::tasks::task_scheduler_client::{TaskControlMsg, TaskSchedulerClient};
-use crate::tasks::vm_host::{VMHostResponse, VmHost};
+use crate::tasks::vm_host::VmHost;
 use crate::tasks::{ServerOptions, TaskStart, VerbCall};
+use crate::vm::VMHostResponse;
 use moor_values::matching::command_parse::{parse_command, ParseCommandError, ParsedCommand};
 use moor_values::matching::match_env::MatchEnvironmentParseMatcher;
 use moor_values::matching::ws_match_env::WsMatchEnv;

crates/kernel/src/tasks/vm_host.rs

@@ -28,7 +28,7 @@ use moor_compiler::Program;
 use moor_compiler::{compile, CompileOptions};
 use moor_values::model::{BinaryType, ObjFlag};
 use moor_values::model::{VerbDef, WorldState};
-use moor_values::tasks::{AbortLimitReason, Exception, TaskId};
+use moor_values::tasks::{AbortLimitReason, TaskId};
 use moor_values::Error::E_MAXREC;
 use moor_values::Obj;
 use moor_values::Var;
@@ -39,39 +39,16 @@ use crate::builtins::BuiltinRegistry;
 use crate::config::FeaturesConfig;
 use crate::tasks::sessions::Session;
 use crate::tasks::task_scheduler_client::TaskSchedulerClient;
-use crate::tasks::vm_host::VMHostResponse::{AbortLimit, ContinueOk, DispatchFork, Suspend};
 use crate::tasks::VerbCall;
 use crate::vm::activation::Frame;
 use crate::vm::moo_execute::moo_frame_execute;
-use crate::vm::vm_call::VerbProgram;
-use crate::vm::VmExecParams;
-use crate::vm::{ExecutionResult, Fork, VerbExecutionRequest};
+use crate::vm::vm_call::{VerbProgram, VmExecParams};
+use crate::vm::VMHostResponse::{AbortLimit, ContinueOk, DispatchFork, Suspend};
+use crate::vm::{ExecutionResult, Fork, VMHostResponse, VerbExecutionRequest};
 use crate::vm::{FinallyReason, VMExecState};
 use crate::PhantomUnsync;
 use moor_values::matching::command_parse::ParsedCommand;
 
-/// Return common from exec_interpreter back to the Task scheduler loop
-pub enum VMHostResponse {
-    /// Tell the task to just keep on letting us do what we're doing.
-    ContinueOk,
-    /// Tell the task to ask the scheduler to dispatch a fork request, and then resume execution.
-    DispatchFork(Fork),
-    /// Tell the task to suspend us.
-    Suspend(Option<Duration>),
-    /// Tell the task Johnny 5 needs input from the client (`read` invocation).
-    SuspendNeedInput,
-    /// Task timed out or exceeded ticks.
-    AbortLimit(AbortLimitReason),
-    /// Tell the task that execution has completed, and the task is successful.
-    CompleteSuccess(Var),
-    /// The VM aborted. (FinallyReason::Abort in MOO VM)
-    CompleteAbort,
-    /// The VM threw an exception. (FinallyReason::Uncaught in MOO VM)
-    CompleteException(Exception),
-    /// A rollback-retry was requested.
-    RollbackRetry,
-}
-
 /// A 'host' for running some kind of interpreter / virtual machine inside a running moor task.
 pub struct VmHost {
     /// Where we store current execution state for this host. Includes all all activations and the
@@ -271,7 +248,7 @@ impl VmHost {
                     result = self.vm_exec_state.unwind_stack(fr);
                     continue;
                 }
-                ExecutionResult::Pass(pass_args) => {
+                ExecutionResult::DispatchVerbPass(pass_args) => {
                     result = self
                         .vm_exec_state
                         .prepare_pass_verb(world_state, &pass_args);
@@ -310,7 +287,7 @@ impl VmHost {
                     self.vm_exec_state.exec_call_request(call_request);
                     return ContinueOk;
                 }
-                ExecutionResult::PerformEval {
+                ExecutionResult::DispatchEval {
                     permissions,
                     player,
                     program,
@@ -319,7 +296,7 @@ impl VmHost {
                         .exec_eval_request(&permissions, &player, program);
                     return ContinueOk;
                 }
-                ExecutionResult::ContinueBuiltin {
+                ExecutionResult::DispatchBuiltin {
                     builtin: bf_offset,
                     arguments: args,
                 } => {
@@ -335,7 +312,7 @@ impl VmHost {
                     );
                     continue;
                 }
-                ExecutionResult::DispatchFork(delay, task_id, fv_offset) => {
+                ExecutionResult::TaskStartFork(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();
@@ -350,10 +327,10 @@ impl VmHost {
                     };
                     return DispatchFork(fork_request);
                 }
-                ExecutionResult::Suspend(delay) => {
+                ExecutionResult::TaskSuspend(delay) => {
                     return Suspend(delay);
                 }
-                ExecutionResult::NeedInput => {
+                ExecutionResult::TaskNeedInput => {
                     return VMHostResponse::SuspendNeedInput;
                 }
                 ExecutionResult::Complete(a) => {
@@ -376,7 +353,7 @@ impl VmHost {
                         }
                     };
                 }
-                ExecutionResult::RollbackRestart => {
+                ExecutionResult::TaskRollbackRestart => {
                     trace!(task_id, "Task rollback-restart");
                     return VMHostResponse::RollbackRetry;
                 }

crates/kernel/src/vm/exec_state.rs

@@ -35,10 +35,7 @@ pub struct Caller {
     pub line_number: usize,
 }
 
-/// Represents the state of VM execution.
-/// The actual "VM" remains stateless and could be potentially re-used for multiple tasks,
-/// and swapped out at each level of the activation stack for different runtimes.
-/// e.g. a MOO VM, a WASM VM, a JS VM, etc. but all having access to the same shared state.
+/// Represents the state of VM execution for a given task.
 #[derive(Encode, Decode)]
 pub struct VMExecState {
     /// The task ID of the task that for current stack of activations.
@@ -47,7 +44,7 @@ pub struct VMExecState {
     /// (For language runtimes that keep their own stack, this is simply the "entry" point
     ///  for the function invocation.)
     pub(crate) stack: Vec<Activation>,
-    /// The tick slice for the current execution.
+    /// The tick slice for the current/next execution.
     pub(crate) tick_slice: usize,
     /// The total number of ticks that the task is allowed to run.
     pub(crate) max_ticks: usize,

crates/kernel/src/vm/mod.rs

@@ -17,7 +17,6 @@
 //! Aims to be semantically identical, so as to be able to run existing LambdaMOO compatible cores
 //! without blocking issues.
 
-use std::sync::Arc;
 use std::time::Duration;
 
 use bincode::{Decode, Encode};
@@ -27,14 +26,12 @@ 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::tasks::{AbortLimitReason, Exception};
 use moor_values::{Error, List, Obj, Symbol, Var};
 pub use vm_call::VerbExecutionRequest;
 pub use vm_unwind::FinallyReason;
 
 // Exports to the rest of the kernel
-use crate::builtins::BuiltinRegistry;
-use crate::config::FeaturesConfig;
-use crate::tasks::task_scheduler_client::TaskSchedulerClient;
 use crate::tasks::VerbCall;
 use crate::vm::activation::Activation;
 
@@ -48,57 +45,27 @@ mod moo_frame;
 #[cfg(test)]
 mod vm_test;
 
-/// The set of parameters for a VM-requested fork.
-#[derive(Debug, Clone, Encode, Decode)]
-pub struct Fork {
-    /// The player. This is in the activation as well, but it's nicer to have it up here and
-    /// explicit
-    pub(crate) player: Obj,
-    /// The permissions context for the forked task.
-    pub(crate) progr: Obj,
-    /// The task ID of the task that forked us
-    pub(crate) parent_task_id: usize,
-    /// The time to delay before starting the forked task, if any.
-    pub(crate) delay: Option<Duration>,
-    /// A copy of the activation record from the task that forked us.
-    pub(crate) activation: Activation,
-    /// The unique fork vector offset into the fork vector for the executing binary held in the
-    /// activation record.  This is copied into the main vector and execution proceeds from there,
-    /// instead.
-    pub(crate) fork_vector_offset: Offset,
-    /// The (optional) variable label where the task ID of the new task should be stored, in both
-    /// the parent activation and the new task's activation.
-    pub task_id: Option<Name>,
-}
-
-/// Represents the set of parameters passed to the VM for execution.
-pub struct VmExecParams {
-    pub task_scheduler_client: TaskSchedulerClient,
-    pub builtin_registry: Arc<BuiltinRegistry>,
-    pub max_stack_depth: usize,
-    pub config: FeaturesConfig,
-}
-
+/// Possible outcomes from VM execution inner loop, which are used to determine what to do next.
 #[derive(Debug, Clone)]
 pub enum ExecutionResult {
-    /// Execution of this call stack is complete.
+    /// All is well. The task should let the VM continue executing.
+    More,
+    /// Execution of this stack frame is complete with a return value.
     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.
+    /// 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),
+    /// Create the frames necessary to perform a `pass` up the inheritance chain.
+    DispatchVerbPass(List),
     /// Begin preparing to call a verb, by looking up the verb and preparing the dispatch.
     PrepareVerbDispatch {
         this: Var,
@@ -118,29 +85,78 @@ pub enum ExecutionResult {
         /// The parsed user command that led to this verb dispatch, if any.
         command: Option<ParsedCommand>,
     },
-    /// Request dispatch of a new task as a fork
-    DispatchFork(Option<Duration>, Option<Name>, Offset),
-    /// Request dispatch of a builtin function with the given arguments.
-    ContinueBuiltin { builtin: BuiltinId, arguments: List },
-    /// Request that this task be suspended for a duration of time.
-    /// This leads to the task performing a commit, being suspended for a delay, and then being
-    /// resumed under a new transaction.
-    /// If the duration is None, then the task is suspended indefinitely, until it is killed or
-    /// resumed using `resume()` or `kill_task()`.
-    Suspend(Option<Duration>),
-    /// Request input from the client.
-    NeedInput,
     /// Request `eval` execution, which is a kind of special activation creation where we've already
     /// been given the program to execute instead of having to look it up.
-    PerformEval {
+    DispatchEval {
         /// The permissions context for the eval.
         permissions: Obj,
         /// The player who is performing the eval.
         player: Obj,
         /// The program to execute.
         program: Program,
     },
+    /// Request dispatch of a builtin function with the given arguments.
+    DispatchBuiltin { builtin: BuiltinId, arguments: List },
+    /// Request start of a new task as a fork, at a given offset into the fork vector of the
+    /// current program. If the duration is None, the task should be started immediately, otherwise
+    /// it should be scheduled to start after the given delay.
+    /// If a Name is provided, the task ID of the new task should be stored in the variable with
+    /// that in the parent activation.
+    TaskStartFork(Option<Duration>, Option<Name>, Offset),
+    /// Request that this task be suspended for a duration of time.
+    /// This leads to the task performing a commit, being suspended for a delay, and then being
+    /// resumed under a new transaction.
+    /// If the duration is None, then the task is suspended indefinitely, until it is killed or
+    /// resumed using `resume()` or `kill_task()`.
+    TaskSuspend(Option<Duration>),
+    /// Request input from the client.
+    TaskNeedInput,
     /// Rollback the current transaction and restart the task in a new transaction.
     /// This can happen when a conflict occurs during execution, independent of a commit.
-    RollbackRestart,
+    TaskRollbackRestart,
+}
+
+/// The set of parameters for a VM-requested fork.
+#[derive(Debug, Clone, Encode, Decode)]
+pub struct Fork {
+    /// The player. This is in the activation as well, but it's nicer to have it up here and
+    /// explicit
+    pub(crate) player: Obj,
+    /// The permissions context for the forked task.
+    pub(crate) progr: Obj,
+    /// The task ID of the task that forked us
+    pub(crate) parent_task_id: usize,
+    /// The time to delay before starting the forked task, if any.
+    pub(crate) delay: Option<Duration>,
+    /// A copy of the activation record from the task that forked us.
+    pub(crate) activation: Activation,
+    /// The unique fork vector offset into the fork vector for the executing binary held in the
+    /// activation record.  This is copied into the main vector and execution proceeds from there,
+    /// instead.
+    pub(crate) fork_vector_offset: Offset,
+    /// The (optional) variable label where the task ID of the new task should be stored, in both
+    /// the parent activation and the new task's activation.
+    pub task_id: Option<Name>,
+}
+
+/// Return common from exec_interpreter back to the Task scheduler loop
+pub enum VMHostResponse {
+    /// Tell the task to just keep on letting us do what we're doing.
+    ContinueOk,
+    /// Tell the task to ask the scheduler to dispatch a fork request, and then resume execution.
+    DispatchFork(Fork),
+    /// Tell the task to suspend us.
+    Suspend(Option<Duration>),
+    /// Tell the task Johnny 5 needs input from the client (`read` invocation).
+    SuspendNeedInput,
+    /// Task timed out or exceeded ticks.
+    AbortLimit(AbortLimitReason),
+    /// Tell the task that execution has completed, and the task is successful.
+    CompleteSuccess(Var),
+    /// The VM aborted. (FinallyReason::Abort in MOO VM)
+    CompleteAbort,
+    /// The VM threw an exception. (FinallyReason::Uncaught in MOO VM)
+    CompleteException(Exception),
+    /// A rollback-retry was requested.
+    RollbackRetry,
 }