From 2246a31e726762ea741a299f598c7878fa66dd83 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Fri, 2 Sep 2022 21:49:11 +0300 Subject: refactor(tvix/eval): return call frame result from VM::call Previously, "calling" (setting up the VM run loop for executing a call frame) and "running" (running this loop to completion) were separate operations. This was basically an attempt to avoid nesting `VM::run` invocations. However, doing things this way introduced some tricky bugs for exiting out of the call frames of thunks vs. builtins & closures. For now, we unify the two operations and always return the value to the caller directly. For now this makes calls a little less effective, but it gives us a chance to nail down some other strange behaviours and then re-optimise this afterwards. To make sure we tackle this again further down I've added it to the list of known possible optimisations. Change-Id: I96828ab6a628136e0bac1bf03555faa4e6b74ece Reviewed-on: https://cl.tvl.fyi/c/depot/+/6415 Reviewed-by: sterni Tested-by: BuildkiteCI --- tvix/eval/docs/known-optimisation-potential.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'tvix/eval/docs') diff --git a/tvix/eval/docs/known-optimisation-potential.md b/tvix/eval/docs/known-optimisation-potential.md index e7271f4b35..da9cf265ea 100644 --- a/tvix/eval/docs/known-optimisation-potential.md +++ b/tvix/eval/docs/known-optimisation-potential.md @@ -65,3 +65,19 @@ optimisations, but note the most important ones here. The same thing goes for resolving `with builtins;`, which should definitely resolve statically. + +* Avoid nested `VM::run` calls [hard] + + Currently when encountering Nix-native callables (thunks, closures) + the VM's run loop will nest and return the value of the nested call + frame one level up. This makes the Rust call stack almost mirror the + Nix call stack, which is usually undesirable. + + It is possible to detect situations where this is avoidable and + instead set up the VM in such a way that it continues and produces + the desired result in the same run loop, but this is kind of tricky + to get right - especially while other parts are still in flux. + + For details consult the commit with Gerrit change ID + `I96828ab6a628136e0bac1bf03555faa4e6b74ece`, in which the initial + attempt at doing this was reverted. -- cgit 1.4.1