maurice/snix
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(tvix/eval): avoid cloning in NixAttrs::update if possible

Browse source 
Refactors the update function to take the attribute sets by value
instead.

To facilitate this, we use an equivalent of the currently unstable
`Rc::clone_or_unwrap` in the VM when encountering attribute sets, so
that in cases where the only references to the attrs being updated are
the ones on the stack those clones are avoided completely.

This does make update() a little bit more tricky internally, as some
optimised branches can directly return the moved value, and others
need to destructure with ownership. For this reason there are now two
different match statements handling the different ownership cases.

Change-Id: Ia77d3ba5c86afb75b9f1f51758bda61729ba5aab
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6279
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This commit is contained in:
Vincent Ambo 2022-08-26 19:22:18 +03:00 • committed by tazjin
parent 3270817b90
commit 3a2fcc8bc2
2 changed files with 35 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

12
tvix/eval/src/vm.rs
View file

@ -203,10 +203,10 @@ impl VM {
OpCode::OpAttrPath(count) => self.run_attr_path(count)?,
OpCode::OpAttrsUpdate => {
let rhs = self.pop().to_attrs()?;
let lhs = self.pop().to_attrs()?;
let rhs = unwrap_or_clone_rc(self.pop().to_attrs()?);
let lhs = unwrap_or_clone_rc(self.pop().to_attrs()?);
self.push(Value::Attrs(Rc::new(lhs.update(&rhs))))
self.push(Value::Attrs(Rc::new(lhs.update(rhs))))
}
OpCode::OpAttrsSelect => {
@ -414,6 +414,12 @@ impl VM {
}
}
// TODO: use Rc::unwrap_or_clone once it is stabilised.
// https://doc.rust-lang.org/std/rc/struct.Rc.html#method.unwrap_or_clone
fn unwrap_or_clone_rc<T: Clone>(rc: Rc<T>) -> T {
Rc::try_unwrap(rc).unwrap_or_else(|rc| (*rc).clone())
}
pub fn run_lambda(lambda: Lambda) -> EvalResult<Value> {
let mut vm = VM {
frames: vec![],