From f16b0f24e2f7c360b1daf7ae7b8e3341907faf76 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Fri, 3 Feb 2023 12:25:26 +0300
Subject: refactor(tvix/eval): wrap `Builtin` type in a Box
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 tvix/eval/src/value/builtin.rs | 50 +++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/tvix/eval/src/value/builtin.rs b/tvix/eval/src/value/builtin.rs
index bb14265181..c7fc33903d 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
     }
 }
-- 
cgit 1.4.1