Skip to content

Commit

Permalink
Use List instead of Vec<Var> throughout
Browse files Browse the repository at this point in the history 
Should reduce copies on verb dispatch.
  • Loading branch information
@rdaum
rdaum committed Dec 23, 2024
1 parent 3254a06 commit 32f5abe
Show file tree
Hide file tree
Showing 21 changed files with 125 additions and 107 deletions.
24 changes: 19 additions & 5 deletions crates/common/src/var/list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use num_traits::ToPrimitive;
use std::cmp::max;
use std::fmt::{Debug, Formatter};
use std::hash::Hash;
use std::ops::Index;

#[derive(Clone)]
pub struct List(Box<im::Vector<Var>>);
Expand All @@ -42,7 +43,7 @@ impl List {
}

pub fn iter(&self) -> impl Iterator<Item = Var> + '_ {
(0..self.len()).map(move |i| self.index(i).unwrap())
self.0.iter().cloned()
}

/// Remove the first found instance of `item` from the list.
Expand Down Expand Up @@ -225,6 +226,19 @@ impl Sequence for List {
}
}

impl Into<Var> for List {

Check warning on line 229 in crates/common/src/var/list.rs

View workflow job for this annotation

GitHub Actions / clippy

an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true 

warning: an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true
   --> crates/common/src/var/list.rs:229:1
    |
229 | impl Into<Var> for List {
    | ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into
    = note: `#[warn(clippy::from_over_into)]` on by default
help: replace the `Into` implementation with `From<var::list::List>`
    |
229 ~ impl From<List> for Var {
230 ~     fn from(val: List) -> Self {
231 ~         Var::from_variant(Variant::List(val))
    |
fn into(self) -> Var {
Var::from_variant(Variant::List(self))
}
}

impl Index<usize> for List {
type Output = Var;

fn index(&self, index: usize) -> &Self::Output {
&self.0[index]
}
}
impl PartialEq for List {
fn eq(&self, other: &Self) -> bool {
if self.len() != other.len() {
Expand All @@ -233,8 +247,8 @@ impl PartialEq for List {

// elements comparison
for i in 0..self.len() {
let a = self.index(i).unwrap();
let b = other.index(i).unwrap();
let a = Sequence::index(self, i).unwrap();
let b = Sequence::index(other, i).unwrap();
if a != b {
return false;
}
Expand All @@ -260,8 +274,8 @@ impl Ord for List {

// elements comparison
for i in 0..self.len() {
let a = self.index(i).unwrap();
let b = other.index(i).unwrap();
let a = Sequence::index(self, i).unwrap();
let b = Sequence::index(other, i).unwrap();
match a.cmp(&b) {
std::cmp::Ordering::Equal => continue,
x => return x,
Expand Down
6 changes: 3 additions & 3 deletions crates/daemon/src/rpc_server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ use moor_values::model::{Named, ObjectRef, PropFlag, ValSet, VerbFlag};
use moor_values::tasks::SchedulerError::CommandExecutionError;
use moor_values::tasks::{CommandError, NarrativeEvent, SchedulerError, TaskId};
use moor_values::util::parse_into_words;
use moor_values::Variant;
use moor_values::SYSTEM_OBJECT;
use moor_values::{v_obj, v_str, Symbol};
use moor_values::{List, Variant};
use moor_values::{Obj, Var};
use rpc_common::DaemonToClientReply::{LoginResult, NewConnection};
use rpc_common::{
Expand Down Expand Up @@ -869,7 +869,7 @@ impl RpcServer {
player,
&ObjectRef::Id(handler_object.clone()),
connected_verb,
vec![v_obj(player.clone())],
List::mk_list(&[v_obj(player.clone())]),
"".to_string(),
&SYSTEM_OBJECT,
session,
Expand Down Expand Up @@ -1038,7 +1038,7 @@ impl RpcServer {
connection,
object,
verb,
args,
List::mk_list(&args),
"".to_string(),
&SYSTEM_OBJECT,
session,
Expand Down
4 changes: 2 additions & 2 deletions crates/kernel/src/builtins/bf_list_sets.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ fn bf_listset(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
}
let index = bf_args.args[2].clone();
let value = bf_args.args[1].clone();
let list = &mut bf_args.args[0];
let list = bf_args.args[0].clone();
if list.type_code() != VarType::TYPE_LIST {
return Err(BfErr::Code(E_TYPE));
}
Expand All @@ -124,7 +124,7 @@ fn bf_setadd(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
return Err(BfErr::Code(E_ARGS));
}
let value = bf_args.args[1].clone();
let list = &mut bf_args.args[0];
let list = bf_args.args[0].clone();
let Variant::List(list) = list.variant() else {
return Err(BfErr::Code(E_TYPE));
};
Expand Down
2 changes: 1 addition & 1 deletion crates/kernel/src/builtins/bf_maps.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
use crate::bf_declare;
use crate::builtins::{BfCallState, BfErr, BfRet, BuiltinFunction};
use moor_compiler::offset_for_builtin;
use moor_values::Associative;
use moor_values::Error::{E_ARGS, E_RANGE, E_TYPE};
use moor_values::{v_bool, v_list, Var, Variant};
use moor_values::{Associative, Sequence};
/// Returns a copy of map with the value corresponding to key removed. If key is not a valid key, then E_RANGE is raised.
fn bf_mapdelete(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
if bf_args.args.len() != 2 {
Expand Down
17 changes: 10 additions & 7 deletions crates/kernel/src/builtins/bf_num.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use rand::Rng;

use moor_compiler::offset_for_builtin;
use moor_values::Error::{E_ARGS, E_INVARG, E_TYPE};
use moor_values::Variant;
use moor_values::{v_float, v_int, v_str};
use moor_values::{Sequence, Variant};

use crate::bf_declare;
use crate::builtins::BfRet::Ret;
Expand Down Expand Up @@ -74,12 +74,15 @@ fn bf_random(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
}

let mut rng = rand::thread_rng();
match &bf_args.args.first().map(|var| var.variant()) {
Some(Variant::Int(i)) if *i > 0 => Ok(Ret(v_int(rng.gen_range(1..=*i)))),
Some(Variant::Int(_)) => Err(BfErr::Code(E_INVARG)),
None => Ok(Ret(v_int(rng.gen_range(1..=2147483647)))),
_ => Err(BfErr::Code(E_TYPE)),
}
bf_args
.args
.is_empty()
.then_some(Ok(Ret(v_int(rng.gen_range(1..=2147483647)))))
.unwrap_or_else(|| match &bf_args.args[0].variant() {
Variant::Int(i) if *i > 0 => Ok(Ret(v_int(rng.gen_range(1..=*i)))),
Variant::Int(_) => Err(BfErr::Code(E_INVARG)),
_ => Err(BfErr::Code(E_TYPE)),
})
}
bf_declare!(random, bf_random);

Expand Down
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 @@ -21,10 +21,10 @@ use moor_values::model::WorldStateError;
use moor_values::model::{ObjFlag, ValSet};
use moor_values::util::BitEnum;
use moor_values::Error::{E_ARGS, E_INVARG, E_NACC, E_PERM, E_TYPE};
use moor_values::Variant;
use moor_values::{v_bool, v_int, v_none, v_obj, v_str};
use moor_values::{v_list, Sequence, Symbol};
use moor_values::{v_list_iter, NOTHING};
use moor_values::{List, Variant};

use crate::bf_declare;
use crate::builtins::BfRet::{Ret, VmInstr};
Expand Down Expand Up @@ -167,7 +167,7 @@ fn bf_create(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
location: v_obj(new_obj.clone()),
this: v_obj(new_obj),
player: bf_args.exec_state.top().player.clone(),
args: vec![],
args: List::mk_list(&[]),
argstr: "".to_string(),
caller: bf_args.exec_state.top().this.clone(),
},
Expand Down Expand Up @@ -278,7 +278,7 @@ fn bf_recycle(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
location: v_obj(obj.clone()),
this: v_obj(obj),
player: bf_args.exec_state.top().player.clone(),
args: Vec::new(),
args: List::mk_list(&[]),
argstr: "".to_string(),
caller: bf_args.exec_state.top().this.clone(),
},
Expand Down Expand Up @@ -347,7 +347,7 @@ fn bf_recycle(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
location: v_obj(head_obj.clone()),
this: v_obj(head_obj.clone()),
player: bf_args.exec_state.top().player.clone(),
args: vec![v_obj(obj)],
args: List::mk_list(&[v_obj(obj)]),
argstr: "".to_string(),
caller: bf_args.exec_state.top().this.clone(),
},
Expand Down Expand Up @@ -449,7 +449,7 @@ fn bf_move(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
location: v_obj(whereto.clone()),
this: v_obj(whereto.clone()),
player: bf_args.exec_state.top().player.clone(),
args: vec![v_obj(what)],
args: List::mk_list(&[v_obj(what)]),
argstr: "".to_string(),
caller: bf_args.exec_state.top().this.clone(),
},
Expand Down Expand Up @@ -527,7 +527,7 @@ fn bf_move(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
location: v_obj(original_location.clone()),
this: v_obj(original_location),
player: bf_args.exec_state.top().player.clone(),
args: vec![v_obj(what)],
args: List::mk_list(&[v_obj(what)]),
argstr: "".to_string(),
caller: bf_args.exec_state.top().this.clone(),
},
Expand Down Expand Up @@ -574,7 +574,7 @@ fn bf_move(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
location: v_obj(whereto.clone()),
this: v_obj(whereto),
player: bf_args.exec_state.top().player.clone(),
args: vec![v_obj(what)],
args: List::mk_list(&[v_obj(what)]),
argstr: "".to_string(),
caller: bf_args.exec_state.top().this.clone(),
},
Expand Down
9 changes: 6 additions & 3 deletions crates/kernel/src/builtins/bf_server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ use moor_compiler::{offset_for_builtin, ArgCount, ArgType, Builtin, BUILTINS};
use moor_values::model::{ObjFlag, WorldStateError};
use moor_values::tasks::NarrativeEvent;
use moor_values::Error::{E_ARGS, E_INVARG, E_INVIND, E_PERM, E_TYPE};
use moor_values::Symbol;
use moor_values::Variant;
use moor_values::{v_bool, v_int, v_list, v_none, v_obj, v_str, v_string, Var};
use moor_values::{v_list_iter, Error};
use moor_values::{Sequence, Symbol};

use crate::bf_declare;
use crate::builtins::BfRet::{Ret, VmInstr};
Expand Down Expand Up @@ -657,7 +657,10 @@ fn bf_call_function(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
};

// Arguments are everything left, if any.
let args = &bf_args.args[1..];
let (_, args) = bf_args.args.pop_front().map_err(|_| BfErr::Code(E_ARGS))?;
let Variant::List(arguments) = args.variant() else {
return Err(BfErr::Code(E_TYPE));
};

// Find the function id for the given function name.
let func_name = Symbol::mk_case_insensitive(func_name.as_string().as_str());
Expand All @@ -668,7 +671,7 @@ fn bf_call_function(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
// Then ask the scheduler to run the function as a continuation of what we're doing now.
Ok(VmInstr(ExecutionResult::ContinueBuiltin {
builtin,
arguments: args[..].to_vec(),
arguments: arguments.clone(),
}))
}
bf_declare!(call_function, bf_call_function);
Expand Down
2 changes: 1 addition & 1 deletion crates/kernel/src/builtins/bf_strings.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ use rand::Rng;

use moor_compiler::offset_for_builtin;
use moor_values::Error::{E_ARGS, E_INVARG, E_TYPE};
use moor_values::Variant;
use moor_values::{v_int, v_str, v_string};
use moor_values::{Sequence, Variant};

use crate::bf_declare;
use crate::builtins::BfRet::Ret;
Expand Down
2 changes: 1 addition & 1 deletion crates/kernel/src/builtins/bf_values.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ bf_declare!(typeof, bf_typeof);

fn bf_tostr(bf_args: &mut BfCallState<'_>) -> Result<BfRet, BfErr> {
let mut result = String::new();
for arg in &bf_args.args {
for arg in bf_args.args.iter() {
match arg.variant() {
Variant::None => result.push_str("None"),
Variant::Int(i) => result.push_str(&i.to_string()),
Expand Down
4 changes: 2 additions & 2 deletions crates/kernel/src/builtins/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ use moor_compiler::{BuiltinId, BUILTINS};
use moor_values::model::Perms;
use moor_values::model::WorldState;
use moor_values::model::WorldStateError;
use moor_values::Error;
use moor_values::Obj;
use moor_values::Symbol;
use moor_values::Var;
use moor_values::{Error, List};

use crate::builtins::bf_list_sets::register_bf_list_sets;
use crate::builtins::bf_maps::register_bf_maps;
Expand Down Expand Up @@ -92,7 +92,7 @@ pub struct BfCallState<'a> {
/// The name of the invoked function.
pub(crate) name: Symbol,
/// Arguments passed to the function.
pub(crate) args: Vec<Var>,
pub(crate) args: List,
/// The current execution state of this task in this VM, including the stack
/// so that BFs can inspect and manipulate it.
pub(crate) exec_state: &'a mut VMExecState,
Expand Down
10 changes: 5 additions & 5 deletions crates/kernel/src/tasks/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::time::SystemTime;
use bincode::{Decode, Encode};

use moor_compiler::Program;
use moor_values::Obj;
use moor_values::{List, Obj};
use moor_values::{Symbol, Var};

pub use crate::tasks::tasks_db::{NoopTasksDb, TasksDb, TasksDbError};
Expand Down Expand Up @@ -84,7 +84,7 @@ pub struct VerbCall {
pub location: Var,
pub this: Var,
pub player: Obj,
pub args: Vec<Var>,
pub args: List,
pub argstr: String,
pub caller: Var,
}
Expand Down Expand Up @@ -133,8 +133,8 @@ pub mod vm_test_utils {

use moor_compiler::Program;
use moor_values::model::WorldState;
use moor_values::SYSTEM_OBJECT;
use moor_values::{v_obj, Symbol};
use moor_values::{List, SYSTEM_OBJECT};
use moor_values::{Obj, Var};

use crate::builtins::BuiltinRegistry;
Expand Down Expand Up @@ -210,7 +210,7 @@ pub mod vm_test_utils {
session: Arc<dyn Session>,
builtins: Arc<BuiltinRegistry>,
verb_name: &str,
args: Vec<Var>,
args: List,
) -> ExecResult {
execute(world_state, session, builtins, |world_state, vm_host| {
let verb_name = Symbol::mk_case_insensitive(verb_name);
Expand Down Expand Up @@ -338,7 +338,7 @@ pub enum TaskStart {
player: Obj,
vloc: Var,
verb: Symbol,
args: Vec<Var>,
args: List,
argstr: String,
},
/// The scheduler is telling the task to run a task that was forked from another task.
Expand Down
5 changes: 3 additions & 2 deletions crates/kernel/src/tasks/scheduler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use moor_values::tasks::{
AbortLimitReason, CommandError, SchedulerError, TaskId, VerbProgramError,
};
use moor_values::Error::{E_INVARG, E_INVIND, E_PERM};
use moor_values::{v_err, v_int, v_none, v_obj, v_string, Symbol, Var};
use moor_values::{v_err, v_int, v_none, v_obj, v_string, List, Symbol, Var};
use moor_values::{AsByteBuffer, SYSTEM_OBJECT};
use moor_values::{Obj, Variant};

Expand Down Expand Up @@ -483,7 +483,8 @@ impl Scheduler {
session,
reply,
} => {
let args = command.into_iter().map(v_string).collect::<Vec<Var>>();
let args = command.into_iter().map(v_string);
let args = List::from_iter(args);
let task_start = Arc::new(TaskStart::StartVerb {
player: player.clone(),
vloc: v_obj(handler_object),
Expand Down
6 changes: 3 additions & 3 deletions crates/kernel/src/tasks/scheduler_client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use uuid::Uuid;

use moor_compiler::{compile, Program};
use moor_values::model::{ObjectRef, PropDef, PropPerms, VerbDef, VerbDefs};
use moor_values::{Obj, Symbol, Var};
use moor_values::{List, Obj, Symbol, Var};

use crate::config::FeaturesConfig;
use crate::tasks::sessions::Session;
Expand Down Expand Up @@ -78,7 +78,7 @@ impl SchedulerClient {
player: &Obj,
vloc: &ObjectRef,
verb: Symbol,
args: Vec<Var>,
args: List,
argstr: String,
perms: &Obj,
session: Arc<dyn Session>,
Expand Down Expand Up @@ -370,7 +370,7 @@ pub enum SchedulerClientMsg {
player: Obj,
vloc: ObjectRef,
verb: Symbol,
args: Vec<Var>,
args: List,
argstr: String,
perms: Obj,
session: Arc<dyn Session>,
Expand Down
Loading

0 comments on commit 32f5abe

Please sign in to comment.