feat(tvix/eval): Leak strings (with flag to disable) r/8443

Default to always leaking strings, and copying strings by copying
pointers rather than cloning the underlying allocation. This (somewhat
bafflingly) doesn't seem to affect any benchmarks, but paves the way for
some tricks around string allocation that do.

Unfortunately, we can't do this (yet?) for contextful strings, for
reasons I don't currently understand but which I will address later,
when I address contextful strings more holistically.

I've left a flag in here to disable this, both to test the cloning logic
for strings for when/if we decide to bring this back, and to allow
people who care more about memory usage than perf to disable leaking.

Change-Id: Iec44bcbfe9b3d20389d2450b9a551792a79b9b26
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12045
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>

3 files changed, 21 insertions, 4 deletions

diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix
index 4826317e9fb1..926145c31fca 100644
--- a/tvix/Cargo.nix
+++ b/tvix/Cargo.nix
@@ -16122,7 +16122,7 @@ rec {
           "proptest" = [ "dep:proptest" ];
           "test-strategy" = [ "dep:test-strategy" ];
         };
-        resolvedDefaultFeatures = [ "arbitrary" "default" "impure" "nix_tests" "proptest" "test-strategy" ];
+        resolvedDefaultFeatures = [ "arbitrary" "default" "impure" "nix_tests" "no_leak" "proptest" "test-strategy" ];
       };
       "tvix-eval-builtin-macros" = rec {
         crateName = "tvix-eval-builtin-macros";
diff --git a/tvix/eval/Cargo.toml b/tvix/eval/Cargo.toml
index 83600eb6e2bd..ed2ed0440804 100644
--- a/tvix/eval/Cargo.toml
+++ b/tvix/eval/Cargo.toml
@@ -58,6 +58,11 @@ impure = []
 # Enables Arbitrary impls for internal types (required to run tests)
 arbitrary = ["proptest", "test-strategy", "imbl/proptest"]
 
+# Don't leak strings (enable this if you care about peak memory usage of eval)
+#
+# This is intended as a stop-gap until we have a garbage collector
+no_leak = []
+
 [[bench]]
 name = "eval"
 harness = false
diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs
index 163e140a19c4..3e991890ea5a 100644
--- a/tvix/eval/src/value/string.rs
+++ b/tvix/eval/src/value/string.rs
@@ -409,6 +409,10 @@ unsafe impl Sync for NixString {}
 
 impl Drop for NixString {
     fn drop(&mut self) {
+        if !cfg!(feature = "no_leak") {
+            return;
+        }
+
         // SAFETY: There's no way to construct a NixString that doesn't leave the allocation correct
         // according to the rules of dealloc
         unsafe {
@@ -419,9 +423,17 @@ impl Drop for NixString {
 
 impl Clone for NixString {
     fn clone(&self) -> Self {
-        // SAFETY: There's no way to construct a NixString that doesn't leave the allocation correct
-        // according to the rules of clone
-        unsafe { Self(NixStringInner::clone(self.0)) }
+        if cfg!(feature = "no_leak") || self.context().is_some() {
+            // SAFETY: There's no way to construct a NixString that doesn't leave the allocation correct
+            // according to the rules of clone
+            unsafe { Self(NixStringInner::clone(self.0)) }
+        } else {
+            // SAFETY:
+            //
+            // - NixStrings are never mutated
+            // - NixStrings are never freed
+            Self(self.0)
+        }
     }
 }