diff options
author | Vincent Ambo <mail@tazj.in> | 2023-02-03T09·25+0300 |
---|---|---|
committer | tazjin <tazjin@tvl.su> | 2023-02-03T18·47+0000 |
commit | f16b0f24e2f7c360b1daf7ae7b8e3341907faf76 (patch) | |
tree | 831f17857e1e1b1eb64a5d91552a0899184cafaf | |
parent | 32698766ef05c1c5f65a2fdbb8d08c558d793dec (diff) |
refactor(tvix/eval): wrap `Builtin` type in a Box r/5832
This reduces the size of `Builtin` from 88 (!) bytes to 8, and as the largest variant of `Value`, the size of that type from 96 to 64. The next largest type is NixList, clocking in at 64 bytes. This has noticeable performance impact. In an implementation without disk I/O, evaluating nixpkgs.stdenv looks like this: Benchmark 1: tvix -E '(import <nixpkgs> {}).stdenv.drvPath' Time (mean ± σ): 1.151 s ± 0.003 s [User: 1.041 s, System: 0.109 s] Range (min … max): 1.147 s … 1.155 s 10 runs After this change, it looks like this: Benchmark 1: tvix -E '(import <nixpkgs> {}).stdenv.drvPath' Time (mean ± σ): 1.046 s ± 0.004 s [User: 0.954 s, System: 0.092 s] Range (min … max): 1.041 s … 1.053 s 10 runs Change-Id: I5ab7cc02a9a450c0227daf1f1f72966358311ebb Reviewed-on: https://cl.tvl.fyi/c/depot/+/8027 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
Diffstat (limited to '')
-rw-r--r-- | tvix/eval/src/value/builtin.rs | 50 |
1 files changed, 30 insertions, 20 deletions
diff --git a/tvix/eval/src/value/builtin.rs b/tvix/eval/src/value/builtin.rs index bb142651818a..c7fc33903d96 100644 --- a/tvix/eval/src/value/builtin.rs +++ b/tvix/eval/src/value/builtin.rs @@ -33,6 +33,19 @@ pub struct BuiltinArgument { pub name: &'static str, } +#[derive(Clone)] +pub struct BuiltinRepr { + name: &'static str, + /// Array of arguments to the builtin. + arguments: &'static [BuiltinArgument], + /// Optional documentation for the builtin. + documentation: Option<&'static str>, + func: Rc<dyn BuiltinFn>, + + /// Partially applied function arguments. + partials: Vec<Value>, +} + /// Represents a single built-in function which directly executes Rust /// code that operates on a Nix value. /// @@ -46,16 +59,12 @@ pub struct BuiltinArgument { /// "capture" the partially applied arguments, and are treated /// specially when printing their representation etc. #[derive(Clone)] -pub struct Builtin { - name: &'static str, - /// Array of arguments to the builtin. - arguments: &'static [BuiltinArgument], - /// Optional documentation for the builtin. - documentation: Option<&'static str>, - func: Rc<dyn BuiltinFn>, +pub struct Builtin(Box<BuiltinRepr>); - /// Partially applied function arguments. - partials: Vec<Value>, +impl From<BuiltinRepr> for Builtin { + fn from(value: BuiltinRepr) -> Self { + Builtin(Box::new(value)) + } } impl Builtin { @@ -65,36 +74,37 @@ impl Builtin { documentation: Option<&'static str>, func: F, ) -> Self { - Builtin { + BuiltinRepr { name, arguments, documentation, func: Rc::new(func), partials: vec![], } + .into() } pub fn name(&self) -> &'static str { - self.name + self.0.name } pub fn documentation(&self) -> Option<&'static str> { - self.documentation + self.0.documentation } /// Apply an additional argument to the builtin, which will either /// lead to execution of the function or to returning a partial /// builtin. pub fn apply(mut self, vm: &mut VM, arg: Value) -> Result<Value, ErrorKind> { - self.partials.push(arg); + self.0.partials.push(arg); - if self.partials.len() == self.arguments.len() { - for (idx, BuiltinArgument { strict, .. }) in self.arguments.iter().enumerate() { + if self.0.partials.len() == self.0.arguments.len() { + for (idx, BuiltinArgument { strict, .. }) in self.0.arguments.iter().enumerate() { if *strict { - self.partials[idx].force(vm)?; + self.0.partials[idx].force(vm)?; } } - return (self.func)(self.partials, vm); + return (self.0.func)(self.0.partials, vm); } // Function is not yet ready to be called. @@ -104,13 +114,13 @@ impl Builtin { impl Debug for Builtin { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "builtin[{}]", self.name) + write!(f, "builtin[{}]", self.0.name) } } impl Display for Builtin { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if !self.partials.is_empty() { + if !self.0.partials.is_empty() { f.write_str("<<primop-app>>") } else { f.write_str("<<primop>>") @@ -121,6 +131,6 @@ impl Display for Builtin { /// Builtins are uniquely identified by their name impl PartialEq for Builtin { fn eq(&self, other: &Self) -> bool { - self.name == other.name + self.0.name == other.0.name } } |