diff options
author | Adam Joseph <adam@westernsemico.com> | 2022-10-15T23·10-0700 |
---|---|---|
committer | tazjin <tazjin@tvl.su> | 2022-10-19T10·38+0000 |
commit | d978b556e6d3189760e36fbc0fefe683618cf6ad (patch) | |
tree | 9eea21dfac3510b805ef202dd764143c1e03092e /tvix/eval/src/builtins/impure.rs | |
parent | c91d86ee5ccada2c3868170f263be2b3d396d759 (diff) |
feat(tvix/eval): deduplicate overlap between Closure and Thunk r/5159
This commit deduplicates the Thunk-like functionality from Closure and unifies it with Thunk. Specifically, we now have one and only one way of breaking reference cycles in the Value-graph: Thunk. No other variant contains a RefCell. This should make it easier to reason about the behavior of the VM. InnerClosure and UpvaluesCarrier are no longer necessary. This refactoring allowed an improvement in code generation: `Rc<RefCell<>>`s are now created only for closures which do not have self-references or deferred upvalues, instead of for all closures. OpClosure has been split into two separate opcodes: - OpClosure creates non-recursive closures with no deferred upvalues. The VM will not create an `Rc<RefCell<>>` when executing this instruction. - OpThunkClosure is used for closures with self-references or deferred upvalues. The VM will create a Thunk when executing this opcode, but the Thunk will start out already in the `ThunkRepr::Evaluated` state, rather than in the `ThunkRepr::Suspeneded` state. To avoid confusion, OpThunk has been renamed OpThunkSuspended. Thanks to @sterni for suggesting that all this could be done without adding an additional variant to ThunkRepr. This does however mean that there will be mutating accesses to `ThunkRepr::Evaluated`, which was not previously the case. The field `is_finalised:bool` has been added to `Closure` to ensure that these mutating accesses are performed only on finalised Closures. Both the check and the field are present only if `#[cfg(debug_assertions)]`. Change-Id: I04131501029772f30e28da8281d864427685097f Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7019 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
Diffstat (limited to 'tvix/eval/src/builtins/impure.rs')
-rw-r--r-- | tvix/eval/src/builtins/impure.rs | 5 |
1 files changed, 4 insertions, 1 deletions
diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs index 3d666528e271..5edd81999a7c 100644 --- a/tvix/eval/src/builtins/impure.rs +++ b/tvix/eval/src/builtins/impure.rs @@ -151,7 +151,10 @@ pub fn builtins_import( // Compilation succeeded, we can construct a thunk from whatever it spat // out and return that. - Ok(Value::Thunk(Thunk::new(result.lambda, vm.current_span()))) + Ok(Value::Thunk(Thunk::new_suspended( + result.lambda, + vm.current_span(), + ))) }, ) } |