From e3c92ac3b4b07a7397b565738ec4237b9bf621f6 Mon Sep 17 00:00:00 2001
From: Aspen Smith <root@gws.fyi>
Date: Sat, 10 Feb 2024 11:29:58 -0500
Subject: fix(tvix/eval): Replace inner NixString repr with Box<Bstr>

Storing a full BString here incurs the extra overhead of the capacity
for the inner byte-vector, which we basically never use as Nix strings
are immutable (and we don't do any mutation / sharing analysis).
Switching to a Box<BStr> cuts us from 72 bytes to 64 bytes per
string (and there are a lot of strings!)

Change-Id: I11f34c14a08fa02759f260b1c78b2a2b981714e4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10794
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
---
 tvix/glue/src/builtins/derivation.rs | 25 +++++++++++--------------
 tvix/glue/src/tvix_store_io.rs       |  4 ++--
 2 files changed, 13 insertions(+), 16 deletions(-)

(limited to 'tvix/glue/src')

diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs
index b1487f4505e0..299d064c40d6 100644
--- a/tvix/glue/src/builtins/derivation.rs
+++ b/tvix/glue/src/builtins/derivation.rs
@@ -125,7 +125,7 @@ pub(crate) mod derivation_builtins {
     use std::collections::BTreeMap;
 
     use super::*;
-    use bstr::{ByteSlice, ByteVec};
+    use bstr::ByteSlice;
     use nix_compat::store_path::hash_placeholder;
     use tvix_eval::generators::Gen;
     use tvix_eval::{NixContext, NixContextElement, NixString};
@@ -251,7 +251,7 @@ pub(crate) mod derivation_builtins {
                             Err(cek) => return Ok(Value::Catchable(cek)),
                             Ok(s) => {
                                 input_context.mimic(&s);
-                                drv.arguments.push((**s).clone().into_string()?)
+                                drv.arguments.push(s.to_str()?.to_owned())
                             }
                         }
                     }
@@ -280,14 +280,14 @@ pub(crate) mod derivation_builtins {
                         // Populate drv.outputs
                         if drv
                             .outputs
-                            .insert((**output_name).clone().into_string()?, Default::default())
+                            .insert(output_name.to_str()?.to_owned(), Default::default())
                             .is_some()
                         {
                             Err(DerivationError::DuplicateOutput(
-                                (**output_name).clone().into_string_lossy(),
+                                output_name.to_str_lossy().into_owned(),
                             ))?
                         }
-                        output_names.push((**output_name).clone().into_string()?);
+                        output_names.push(output_name.to_str()?.to_owned());
                     }
 
                     // Add drv.environment[outputs] unconditionally.
@@ -304,9 +304,9 @@ pub(crate) mod derivation_builtins {
                             input_context.mimic(&val_str);
 
                             if arg_name == "builder" {
-                                drv.builder = (**val_str).clone().into_string()?;
+                                drv.builder = val_str.to_str()?.to_owned();
                             } else {
-                                drv.system = (**val_str).clone().into_string()?;
+                                drv.system = val_str.to_str()?.to_owned();
                             }
 
                             // Either populate drv.environment or structured_attrs.
@@ -314,7 +314,7 @@ pub(crate) mod derivation_builtins {
                                 // No need to check for dups, we only iterate over every attribute name once
                                 structured_attrs.insert(
                                     arg_name.to_owned(),
-                                    (**val_str).clone().into_string()?.into(),
+                                    val_str.to_str()?.to_owned().into(),
                                 );
                             } else {
                                 insert_env(&mut drv, arg_name, val_str.as_bytes().into())?;
@@ -373,7 +373,7 @@ pub(crate) mod derivation_builtins {
                 if let Some(attr) = attrs.select(key) {
                     match strong_importing_coerce_to_string(co, attr.clone()).await {
                         Err(cek) => return Ok(Err(cek)),
-                        Ok(str) => return Ok(Ok(Some((**str).clone().into_string()?))),
+                        Ok(str) => return Ok(Ok(Some(str.to_str()?.to_owned()))),
                     }
                 }
 
@@ -520,7 +520,7 @@ pub(crate) mod derivation_builtins {
             nix_compat::store_path::build_text_path(name.to_str()?, &content, content.iter_plain())
                 .map_err(|_e| {
                     nix_compat::derivation::DerivationError::InvalidOutputName(
-                        (**name).clone().into_string_lossy(),
+                        name.to_str_lossy().into_owned(),
                     )
                 })
                 .map_err(DerivationError::InvalidDerivation)?
@@ -530,9 +530,6 @@ pub(crate) mod derivation_builtins {
 
         // TODO: actually persist the file in the store at that path ...
 
-        Ok(Value::from(NixString::new_context_from(
-            context,
-            path.into(),
-        )))
+        Ok(Value::from(NixString::new_context_from(context, path)))
     }
 }
diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs
index 4146ededb40c..ed06eba89686 100644
--- a/tvix/glue/src/tvix_store_io.rs
+++ b/tvix/glue/src/tvix_store_io.rs
@@ -297,7 +297,7 @@ impl EvalIO for TvixStoreIO {
 mod tests {
     use std::{path::Path, rc::Rc, sync::Arc};
 
-    use bstr::ByteVec;
+    use bstr::ByteSlice;
     use tempfile::TempDir;
     use tvix_build::buildservice::DummyBuildService;
     use tvix_castore::{
@@ -356,7 +356,7 @@ mod tests {
 
         let value = result.value.expect("must be some");
         match value {
-            tvix_eval::Value::String(s) => Some((***s).clone().into_string_lossy()),
+            tvix_eval::Value::String(s) => Some(s.to_str_lossy().into_owned()),
             _ => panic!("unexpected value type: {:?}", value),
         }
     }
-- 
cgit 1.4.1