refactor(tvix/eval): Don't double-box Path values r/7461

PathBuf internally contains a heap pointer (an OsString), so we were in
effect double-boxing here. Removing the extra layer by making
Tvix::Value represented by a Box<Path> rather than a Box<PathBuf> saves
us an indirection, while still avoiding the extra memory overhead of the
capacity which was the reason we were boxing PathBuf in the first place.

Change-Id: I8c185b9d4646161d1921917f83e87421496a3e24
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10725
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI

6 files changed, 23 insertions, 20 deletions

diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 6ccae2f9de..31cd78fade 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -53,7 +53,7 @@ pub async fn coerce_value_to_path(
 ) -> Result<Result<PathBuf, CatchableErrorKind>, ErrorKind> {
     let value = generators::request_force(co, v).await;
     if let Value::Path(p) = value {
-        return Ok(Ok(*p));
+        return Ok(Ok(p.into()));
     }
 
     match generators::request_string_coerce(
@@ -400,8 +400,8 @@ mod pure_builtins {
             })
             .unwrap_or(b".");
         if is_path {
-            Ok(Value::Path(Box::new(PathBuf::from(
-                OsString::assert_from_raw_vec(result.to_owned()),
+            Ok(Value::from(PathBuf::from(OsString::assert_from_raw_vec(
+                result.to_owned(),
             ))))
         } else {
             Ok(Value::String(NixString::new_inherit_context_from(
@@ -1713,7 +1713,7 @@ mod placeholder_builtins {
         let res = [
             ("line", 42.into()),
             ("col", 42.into()),
-            ("file", Value::Path(Box::new("/deep/thought".into()))),
+            ("file", Value::from(PathBuf::from("/deep/thought"))),
         ];
         Ok(Value::attrs(NixAttrs::from_iter(res.into_iter())))
     }
diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index b4ef7973c7..60f5666ea9 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -390,7 +390,7 @@ impl Compiler<'_> {
 
             let home_relative_path = &raw_path[2..(raw_path.len())];
             self.emit_constant(
-                Value::UnresolvedPath(Box::new(home_relative_path.into())),
+                Value::UnresolvedPath(PathBuf::from(home_relative_path).into_boxed_path()),
                 node,
             );
             self.push_op(OpCode::OpResolveHomePath, node);
@@ -408,7 +408,10 @@ impl Compiler<'_> {
             let path = &raw_path[1..(raw_path.len() - 1)];
             // Make a thunk to resolve the path (without using `findFile`, at least for now?)
             return self.thunk(slot, node, move |c, _| {
-                c.emit_constant(Value::UnresolvedPath(Box::new(path.into())), node);
+                c.emit_constant(
+                    Value::UnresolvedPath(PathBuf::from(path).into_boxed_path()),
+                    node,
+                );
                 c.push_op(OpCode::OpFindFile, node);
             });
         } else {
@@ -419,7 +422,7 @@ impl Compiler<'_> {
 
         // TODO: Use https://github.com/rust-lang/rfcs/issues/2208
         // once it is available
-        let value = Value::Path(Box::new(crate::value::canon_path(path)));
+        let value = Value::Path(crate::value::canon_path(path).into_boxed_path());
         self.emit_constant(value, node);
     }
 
diff --git a/tvix/eval/src/value/arbitrary.rs b/tvix/eval/src/value/arbitrary.rs
index bf53f4fcb2..d894aa3fe9 100644
--- a/tvix/eval/src/value/arbitrary.rs
+++ b/tvix/eval/src/value/arbitrary.rs
@@ -2,7 +2,7 @@
 
 use imbl::proptest::{ord_map, vector};
 use proptest::{prelude::*, strategy::BoxedStrategy};
-use std::ffi::OsString;
+use std::{ffi::OsString, path::PathBuf};
 
 use super::{attrs::AttrsRep, NixAttrs, NixList, NixString, Value};
 
@@ -92,7 +92,7 @@ fn leaf_value() -> impl Strategy<Value = Value> {
         any::<i64>().prop_map(Integer),
         any::<f64>().prop_map(Float),
         any::<NixString>().prop_map(String),
-        any::<OsString>().prop_map(|s| Path(Box::new(s.into()))),
+        any::<OsString>().prop_map(|s| Path(PathBuf::from(s).into_boxed_path())),
     ]
 }
 
diff --git a/tvix/eval/src/value/json.rs b/tvix/eval/src/value/json.rs
index 4e915a36e5..c2f8b2c2b1 100644
--- a/tvix/eval/src/value/json.rs
+++ b/tvix/eval/src/value/json.rs
@@ -27,7 +27,7 @@ impl Value {
             Value::String(s) => Json::String(s.to_str()?.to_owned()),
 
             Value::Path(p) => {
-                let imported = generators::request_path_import(co, *p).await;
+                let imported = generators::request_path_import(co, p.into_path_buf()).await;
                 Json::String(imported.to_string_lossy().to_string())
             }
 
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index e181b3a2da..df663dd34c 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -3,7 +3,7 @@
 use std::cmp::Ordering;
 use std::fmt::Display;
 use std::num::{NonZeroI32, NonZeroUsize};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::rc::Rc;
 
 use bstr::{BString, ByteVec};
@@ -50,7 +50,7 @@ pub enum Value {
     String(NixString),
 
     #[serde(skip)]
-    Path(Box<PathBuf>),
+    Path(Box<Path>),
     Attrs(Box<NixAttrs>),
     List(NixList),
 
@@ -76,7 +76,7 @@ pub enum Value {
     #[serde(skip)]
     DeferredUpvalue(StackIdx),
     #[serde(skip)]
-    UnresolvedPath(Box<PathBuf>),
+    UnresolvedPath(Box<Path>),
     #[serde(skip)]
     Json(serde_json::Value),
 
@@ -349,7 +349,7 @@ impl Value {
                         import_paths: true, ..
                     },
                 ) => {
-                    let imported = generators::request_path_import(co, *p).await;
+                    let imported = generators::request_path_import(co, p.to_path_buf()).await;
                     // When we import a path from the evaluator, we must attach
                     // its original path as its context.
                     context = context.append(NixContextElement::Plain(
@@ -363,7 +363,7 @@ impl Value {
                         import_paths: false,
                         ..
                     },
-                ) => Ok(p.into_os_string().into_encoded_bytes().into()),
+                ) => Ok((*p).as_os_str().as_encoded_bytes().into()),
 
                 // Attribute sets can be converted to strings if they either have an
                 // `__toString` attribute which holds a function that receives the
@@ -705,7 +705,7 @@ impl Value {
         Value::String(s),
         s.clone()
     );
-    gen_cast!(to_path, Box<PathBuf>, "path", Value::Path(p), p.clone());
+    gen_cast!(to_path, Box<Path>, "path", Value::Path(p), p.clone());
     gen_cast!(to_attrs, Box<NixAttrs>, "set", Value::Attrs(a), a.clone());
     gen_cast!(to_list, NixList, "list", Value::List(l), l.clone());
     gen_cast!(
@@ -1009,7 +1009,7 @@ impl From<f64> for Value {
 
 impl From<PathBuf> for Value {
     fn from(path: PathBuf) -> Self {
-        Self::Path(Box::new(path))
+        Self::Path(path.into_boxed_path())
     }
 }
 
diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index d51d44b6a1..6e36edd956 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -862,7 +862,7 @@ where
                     Value::UnresolvedPath(path) => {
                         let resolved = self
                             .nix_search_path
-                            .resolve(&self.io_handle, *path)
+                            .resolve(&self.io_handle, path)
                             .with_span(&frame, self)?;
                         self.stack.push(resolved.into());
                     }
@@ -882,7 +882,7 @@ where
                                 );
                             }
                             Some(mut buf) => {
-                                buf.push(*path);
+                                buf.push(path);
                                 self.stack.push(buf.into());
                             }
                         };
@@ -1222,7 +1222,7 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> {
     // What we try to do is solely determined by the type of the first value!
     let result = match (a, b) {
         (Value::Path(p), v) => {
-            let mut path = p.into_os_string();
+            let mut path = p.as_os_str().to_owned();
             match generators::request_string_coerce(
                 &co,
                 v,