From c06fb01b3b7a752e0c04ba21a02cdc3f431055e1 Mon Sep 17 00:00:00 2001
From: Peter Kolloch <info@eigenvalue.net>
Date: Wed, 21 Feb 2024 18:24:50 +0700
Subject: feat(tvix/nix-compat): input_derivations with StorePaths

...in `Derivation`.

This is more type-safe and should consume less memory.

This also removes some allocations in the potentially hot path of output hash calculation.

https: //b.tvl.fyi/issues/264
Change-Id: I6ad7d3cb868dc9f750894d449a6065608ef06e8c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10957
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Peter Kolloch <info@eigenvalue.net>
Reviewed-by: Peter Kolloch <info@eigenvalue.net>
---
 tvix/glue/src/builtins/derivation.rs | 19 ++++++++++++++++++-
 tvix/glue/src/known_paths.rs         | 10 ++--------
 tvix/glue/src/tvix_store_io.rs       |  8 +-------
 3 files changed, 21 insertions(+), 16 deletions(-)

(limited to 'tvix/glue')

diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs
index 1ee8d531c7..1208ca94de 100644
--- a/tvix/glue/src/builtins/derivation.rs
+++ b/tvix/glue/src/builtins/derivation.rs
@@ -4,6 +4,7 @@ use crate::tvix_store_io::TvixStoreIO;
 use bstr::BString;
 use nix_compat::derivation::{Derivation, Output};
 use nix_compat::nixhash;
+use nix_compat::store_path::StorePath;
 use std::collections::{btree_map, BTreeSet};
 use std::rc::Rc;
 use tvix_eval::builtin_macros::builtins;
@@ -26,7 +27,23 @@ fn populate_inputs(drv: &mut Derivation, full_context: NixContext) {
                 drv.input_sources.insert(source.clone());
             }
 
-            NixContextElement::Single { name, derivation } => {
+            NixContextElement::Single {
+                name,
+                derivation: derivation_str,
+            } => {
+                // TODO: b/264
+                // We assume derivations to be passed validated, so ignoring rest
+                // and expecting parsing is ok.
+                let (derivation, _rest) =
+                    StorePath::from_absolute_path_full(derivation_str).expect("valid store path");
+
+                #[cfg(debug_assertions)]
+                assert!(
+                    _rest.iter().next().is_none(),
+                    "Extra path not empty for {}",
+                    derivation_str
+                );
+
                 match drv.input_derivations.entry(derivation.clone()) {
                     btree_map::Entry::Vacant(entry) => {
                         entry.insert(BTreeSet::from([name.clone()]));
diff --git a/tvix/glue/src/known_paths.rs b/tvix/glue/src/known_paths.rs
index 03b04d4fc8..bac7e34a7e 100644
--- a/tvix/glue/src/known_paths.rs
+++ b/tvix/glue/src/known_paths.rs
@@ -59,14 +59,8 @@ impl KnownPaths {
         // check input derivations to have been inserted.
         #[cfg(debug_assertions)]
         {
-            // TODO: b/264
-            // We assume derivations to be passed validated, so ignoring rest
-            // and expecting parsing is ok.
-            for input_drv_path_str in drv.input_derivations.keys() {
-                let (input_drv_path, _rest) =
-                    StorePath::from_absolute_path_full(input_drv_path_str)
-                        .expect("parse input drv path");
-                debug_assert!(self.derivations.contains_key(&input_drv_path));
+            for input_drv_path in drv.input_derivations.keys() {
+                debug_assert!(self.derivations.contains_key(input_drv_path));
             }
         }
 
diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs
index 375501b65a..296a369e29 100644
--- a/tvix/glue/src/tvix_store_io.rs
+++ b/tvix/glue/src/tvix_store_io.rs
@@ -140,17 +140,11 @@ impl TvixStoreIO {
                 let input_nodes: BTreeSet<Node> =
                     futures::stream::iter(drv.input_derivations.iter())
                         .map(|(input_drv_path, output_names)| {
-                            // since Derivation is validated, we know this can be parsed.
-                            let input_drv_path =
-                                StorePathRef::from_absolute_path(input_drv_path.as_bytes())
-                                    .expect("invalid drv path")
-                                    .to_owned();
-
                             // look up the derivation object
                             let input_drv = {
                                 let known_paths = self.known_paths.borrow();
                                 known_paths
-                                    .get_drv_by_drvpath(&input_drv_path)
+                                    .get_drv_by_drvpath(input_drv_path)
                                     .unwrap_or_else(|| panic!("{} not found", input_drv_path))
                                     .to_owned()
                             };
-- 
cgit 1.4.1