chore(tvix/eval): mark async functions which are called by the VM
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>
This commit is contained in:
parent
e7a534e0c6
commit
1e80b9ea8b
3 changed files with 9 additions and 0 deletions
|
@ -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;
|
||||
|
|
|
@ -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.
|
||||
|
|
|
@ -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> {
|
||||
|
|
Loading…
Reference in a new issue