From 5f0f4ea3746d6107839454bb5f4967d8757f5bb8 Mon Sep 17 00:00:00 2001
From: Aspen Smith <root@gws.fyi>
Date: Thu, 1 Feb 2024 12:28:29 -0500
Subject: refactor(tvix/eval): Box Value::String

NixString is *quite* large - like 80 bytes - because of the extra
capacity value for BString and because of the context. We want to keep
Value small since we're passing it around a lot, so let's box the
NixString inside Value::String to save on some memory, and make cloning
ostensibly a little cheaper

Change-Id: I343c8b4e7f61dc3dcbbaba4382efb3b3e5bbabb2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10729
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
---
 tvix/eval/src/builtins/impure.rs | 15 ++++++---------
 tvix/eval/src/builtins/mod.rs    | 25 ++++++++++++-------------
 2 files changed, 18 insertions(+), 22 deletions(-)

(limited to 'tvix/eval/src/builtins')

diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs
index 28b8697644dd..5852f148fe6e 100644
--- a/tvix/eval/src/builtins/impure.rs
+++ b/tvix/eval/src/builtins/impure.rs
@@ -50,15 +50,12 @@ mod impure_builtins {
                         NixString::from(
                             String::from_utf8(name.to_vec()).expect("parsing file name as string"),
                         ),
-                        Value::String(
-                            match ftype {
-                                FileType::Directory => "directory",
-                                FileType::Regular => "regular",
-                                FileType::Symlink => "symlink",
-                                FileType::Unknown => "unknown",
-                            }
-                            .into(),
-                        ),
+                        Value::from(match ftype {
+                            FileType::Directory => "directory",
+                            FileType::Regular => "regular",
+                            FileType::Symlink => "symlink",
+                            FileType::Unknown => "unknown",
+                        }),
                     )
                 });
 
diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 31cd78fadeab..0374b4226839 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -167,7 +167,7 @@ mod pure_builtins {
         let mut output = Vec::with_capacity(xs.len());
 
         for (key, _val) in xs.iter() {
-            output.push(Value::String(key.clone()));
+            output.push(Value::from(key.clone()));
         }
 
         Ok(Value::List(NixList::construct(output.len(), output)))
@@ -404,7 +404,7 @@ mod pure_builtins {
                 result.to_owned(),
             ))))
         } else {
-            Ok(Value::String(NixString::new_inherit_context_from(
+            Ok(Value::from(NixString::new_inherit_context_from(
                 &str,
                 result.into(),
             )))
@@ -1095,8 +1095,7 @@ mod pure_builtins {
                         // can be observed in make-initrd.nix when it comes
                         // to compressors which are matched over their full command
                         // and then a compressor name will be extracted from that.
-                        grp.map(|g| Value::String(g.as_str().into()))
-                            .unwrap_or(Value::Null)
+                        grp.map(|g| Value::from(g.as_str())).unwrap_or(Value::Null)
                     })
                     .collect::<imbl::Vector<Value>>()
                     .into(),
@@ -1308,7 +1307,7 @@ mod pure_builtins {
             }
         }
 
-        Ok(Value::String(NixString::new_context_from(context, res)))
+        Ok(Value::from(NixString::new_context_from(context, res)))
     }
 
     #[builtin("seq")]
@@ -1396,9 +1395,9 @@ mod pure_builtins {
 
         let parts = s
             .map(|s| {
-                Value::String(match s {
-                    VersionPart::Number(n) => n.into(),
-                    VersionPart::Word(w) => w.into(),
+                Value::from(match s {
+                    VersionPart::Number(n) => n,
+                    VersionPart::Word(w) => w,
                 })
             })
             .collect::<Vec<Value>>();
@@ -1466,7 +1465,7 @@ mod pure_builtins {
         // non-negative when the starting index is GTE the
         // string's length.
         if beg >= x.len() {
-            return Ok(Value::String(NixString::new_inherit_context_from(
+            return Ok(Value::from(NixString::new_inherit_context_from(
                 &x,
                 BString::default(),
             )));
@@ -1478,7 +1477,7 @@ mod pure_builtins {
             cmp::min(beg + (len as usize), x.len())
         };
 
-        Ok(Value::String(NixString::new_inherit_context_from(
+        Ok(Value::from(NixString::new_inherit_context_from(
             &x,
             (&x[beg..end]).into(),
         )))
@@ -1599,7 +1598,7 @@ mod pure_builtins {
             return Ok(x);
         }
 
-        Ok(Value::String(x.type_of().into()))
+        Ok(Value::from(x.type_of()))
     }
 }
 
@@ -1634,7 +1633,7 @@ pub fn pure_builtins() -> Vec<(&'static str, Value)> {
     let mut result = pure_builtins::builtins();
 
     // Pure-value builtins
-    result.push(("nixVersion", Value::String("2.3-compat-tvix-0.1".into())));
+    result.push(("nixVersion", Value::from("2.3-compat-tvix-0.1")));
     result.push(("langVersion", Value::Integer(6)));
     result.push(("null", Value::Null));
     result.push(("true", Value::Bool(true)));
@@ -1685,7 +1684,7 @@ mod placeholder_builtins {
             .await?
             .to_contextful_str()?;
         v.clear_context();
-        Ok(Value::String(v))
+        Ok(Value::from(v))
     }
 
     #[builtin("addErrorContext")]
-- 
cgit 1.4.1