chore(tvix/eval): mark async functions which are called by the VM r/5991

Given Rust's current lack of support for tail calls, we cannot avoid
using `async` for builtins.  This is the only way to avoid
overflowing the cpu stack when we have arbitrarily deep
builtin/interpreted/builtin/interpreted/... "sandwiches"

There are only five `async fn` functions which are not builtins
(some come in multiple "flavors"):

- add_values
- resolve_with
- force, final_deep_force
- nix_eq, nix_cmp_eq
- coerce_to_string

These can be written iteratively rather than recursively (and in
fact nix_eq used to be written that way!).  I volunteer to rewrite
them.  If written iteratively they would no longer need to be
`async`.

There are two motivations for limiting our reliance on `async` to
only the situation (builtins) where we have no other choice:

1. Performance.

   We don't really have any good measurement of the performance hit
   that the Box<dyn Future>s impose on us.  Right now all of our
   large (nixpkgs-eval) tests are swamped by the cost of other
   things (e.g. fork()ing `nix-store`) so we can't really measure
   it.  Builtins tend to be expensive operations anyways
   (regexp-matching, sorting, etc) that are likely to already cost
   more than the `async` overhead.

2. Preserving the ability to switch to `musttail` calls.

   Clang/LLVM recently got `musttail` (mandatory-elimination tail
   calls).  Rust has refused to add this mainly because WASM doesn't
   support, but WASM `tail_call` has been implemented and was
   recently moved to phase 4 (standardization).  It is very likely
   that Rust will get tail calls sometime in the next year; if it
   does, we won't need async anymore.  In the meantime, I'd like to
   avoid adding any further reliance on `async` in places where it
   wouldn't be straightforward to replace it with a tail call.

https://reviews.llvm.org/D99517

https://github.com/WebAssembly/proposals/pull/157

https: //github.com/rust-lang/rfcs/issues/2691#issuecomment-1462152908
Change-Id: Id15945d5a92bf52c16d93456e3437f91d93bdc57
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8290
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>

3 files changed, 9 insertions, 0 deletions

diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index c9c7f3ca47..f57cfda5ee 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -184,6 +184,7 @@ pub enum PointerEquality {
 }
 
 impl Value {
+    // TODO(amjoseph): de-asyncify this (when called directly by the VM)
     /// Deeply forces a value, traversing e.g. lists and attribute sets and forcing
     /// their contents, too.
     ///
@@ -242,6 +243,7 @@ impl Value {
         Ok(value)
     }
 
+    // TODO(amjoseph): de-asyncify this (when called directly by the VM)
     /// Coerce a `Value` to a string. See `CoercionKind` for a rundown of what
     /// input types are accepted under what circumstances.
     pub async fn coerce_to_string(self, co: GenCo, kind: CoercionKind) -> Result<Value, ErrorKind> {
@@ -331,6 +333,7 @@ impl Value {
         }
     }
 
+    // TODO(amjoseph): de-asyncify this (when called directly by the VM)
     /// Compare two Nix values for equality, forcing nested parts of the structure
     /// as needed.
     ///
@@ -539,6 +542,7 @@ impl Value {
     gen_is!(is_number, Value::Integer(_) | Value::Float(_));
     gen_is!(is_bool, Value::Bool(_));
 
+    // TODO(amjoseph): de-asyncify this (when called directly by the VM)
     /// Compare `self` against other using (fallible) Nix ordering semantics.
     ///
     /// Note that as this returns an `Option<Ordering>` it can not directly be
@@ -606,6 +610,7 @@ impl Value {
         }
     }
 
+    // TODO(amjoseph): de-asyncify this (when called directly by the VM)
     pub async fn force(self, co: GenCo) -> Result<Value, ErrorKind> {
         if let Value::Thunk(thunk) = self {
             return thunk.force(co).await;
diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs
index e990a5cad5..f2959ba15c 100644
--- a/tvix/eval/src/value/thunk.rs
+++ b/tvix/eval/src/value/thunk.rs
@@ -113,6 +113,7 @@ impl Thunk {
         )))))
     }
 
+    // TODO(amjoseph): de-asyncify this
     pub async fn force(self, co: GenCo) -> Result<Value, ErrorKind> {
         // If the current thunk is already fully evaluated, return its evaluated
         // value. The VM will continue running the code that landed us here.
diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index febd4ebd97..610246abe0 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -998,6 +998,7 @@ impl<'o> VM<'o> {
     }
 }
 
+// TODO(amjoseph): de-asyncify this
 /// Resolve a dynamically bound identifier (through `with`) by looking
 /// for matching values in the with-stacks carried at runtime.
 async fn resolve_with(
@@ -1050,6 +1051,7 @@ async fn resolve_with(
     Err(ErrorKind::UnknownDynamicVariable(ident))
 }
 
+// TODO(amjoseph): de-asyncify this
 async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> {
     let result = match (a, b) {
         (Value::Path(p), v) => {
@@ -1079,6 +1081,7 @@ pub struct RuntimeResult {
     pub warnings: Vec<EvalWarning>,
 }
 
+// TODO(amjoseph): de-asyncify this
 /// Generator that retrieves the final value from the stack, and deep-forces it
 /// before returning.
 async fn final_deep_force(co: GenCo) -> Result<Value, ErrorKind> {