diff options
author | Adam Joseph <adam@westernsemico.com> | 2023-03-13T12·58-0700 |
---|---|---|
committer | tazjin <tazjin@tvl.su> | 2023-03-13T21·33+0000 |
commit | 1e80b9ea8b9a28588a90a0835988bb3b3067e7d6 (patch) | |
tree | 9885978090d605a7676541c5e24fab9a310def4f /tvix/eval/src/value/thunk.rs | |
parent | e7a534e0c66758530b02ffce608e9f5bef9db3c5 (diff) |
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>
Diffstat (limited to 'tvix/eval/src/value/thunk.rs')
-rw-r--r-- | tvix/eval/src/value/thunk.rs | 1 |
1 files changed, 1 insertions, 0 deletions
diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs index e990a5cad5cc..f2959ba15c74 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. |