about summary refs log tree commit diff
diff options
context:
space:
mode:
authorVincent Ambo <mail@tazj.in>2022-08-11T16·02+0300
committertazjin <tazjin@tvl.su>2022-08-26T15·06+0000
commit999b9c7a138a5dd5277085570042fc67a893e5af (patch)
treee7eb2baadeb0a172e2b6c48c1644f6604c58d769
parentf331874aeb6ed528d667f8737d1d0b1c6f6f61cf (diff)
refactor(tvix/value): replace static representation with SmolStr r/4498
The only uses of the static variant were for `"name"` and `"value"`,
which are both small enough to fit into a SmolStr. The size of
NixString accomodates `String` anyways, so we may as well inline them.

Additionally smol_str is already in the dependency graph because rnix
uses it, and using it for representations of identifiers is sensible.

Change-Id: I9969312256d1657d69128e54c47dc7294a18ce58
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6165
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
-rw-r--r--tvix/eval/Cargo.lock1
-rw-r--r--tvix/eval/Cargo.toml1
-rw-r--r--tvix/eval/src/compiler.rs4
-rw-r--r--tvix/eval/src/value/attrs.rs6
-rw-r--r--tvix/eval/src/value/string.rs15
5 files changed, 15 insertions, 12 deletions
diff --git a/tvix/eval/Cargo.lock b/tvix/eval/Cargo.lock
index a31340b820..ba3f9388df 100644
--- a/tvix/eval/Cargo.lock
+++ b/tvix/eval/Cargo.lock
@@ -586,6 +586,7 @@ version = "0.1.0"
 dependencies = [
  "criterion",
  "rnix",
+ "smol_str",
  "test-generator",
 ]
 
diff --git a/tvix/eval/Cargo.toml b/tvix/eval/Cargo.toml
index 4c1cf4f302..48156dc1e7 100644
--- a/tvix/eval/Cargo.toml
+++ b/tvix/eval/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2021"
 
 [dependencies]
 rnix = "0.10.2"
+smol_str = "0.1"
 
 [dev-dependencies]
 criterion = "0.3.6"
diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs
index b9e850a36b..622acf9822 100644
--- a/tvix/eval/src/compiler.rs
+++ b/tvix/eval/src/compiler.rs
@@ -105,7 +105,7 @@ impl Compiler {
             let ident = rnix::types::Ident::cast(node).unwrap();
             let idx = self
                 .chunk
-                .add_constant(Value::String(ident.as_str().to_string().into()));
+                .add_constant(Value::String(ident.as_str().into()));
             self.chunk.add_op(OpCode::OpConstant(idx));
             return Ok(());
         }
@@ -275,7 +275,7 @@ impl Compiler {
                         // TODO(tazjin): intern!
                         let idx = self
                             .chunk
-                            .add_constant(Value::String(ident.as_str().to_string().into()));
+                            .add_constant(Value::String(ident.as_str().into()));
                         self.chunk.add_op(OpCode::OpConstant(idx));
                     }
 
diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs
index ecb819fad7..0ab4538a4d 100644
--- a/tvix/eval/src/value/attrs.rs
+++ b/tvix/eval/src/value/attrs.rs
@@ -38,8 +38,8 @@ impl AttrsRep {
 
             AttrsRep::KV { name, value } => {
                 *self = AttrsRep::Map(BTreeMap::from([
-                    ("name".into(), std::mem::replace(name, Value::Blackhole)),
-                    ("value".into(), std::mem::replace(value, Value::Blackhole)),
+                    (NixString::NAME, std::mem::replace(name, Value::Blackhole)),
+                    (NixString::VALUE, std::mem::replace(value, Value::Blackhole)),
                 ]));
                 self.map_mut()
             }
@@ -62,7 +62,7 @@ impl AttrsRep {
                 None
             }
 
-            AttrsRep::Map(map) => map.get(&key.to_string().into()),
+            AttrsRep::Map(map) => map.get(&key.into()),
         }
     }
 }
diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs
index 3025668218..626cf398f2 100644
--- a/tvix/eval/src/value/string.rs
+++ b/tvix/eval/src/value/string.rs
@@ -1,3 +1,4 @@
+use smol_str::SmolStr;
 use std::hash::Hash;
 use std::{borrow::Cow, fmt::Display};
 
@@ -6,7 +7,7 @@ use std::{borrow::Cow, fmt::Display};
 
 #[derive(Clone, Debug)]
 enum StringRepr {
-    Static(&'static str),
+    Smol(SmolStr),
     Heap(String),
 }
 
@@ -33,9 +34,9 @@ impl Ord for NixString {
     }
 }
 
-impl From<&'static str> for NixString {
-    fn from(s: &'static str) -> Self {
-        NixString(StringRepr::Static(s))
+impl From<&str> for NixString {
+    fn from(s: &str) -> Self {
+        NixString(StringRepr::Smol(SmolStr::new(s)))
     }
 }
 
@@ -52,12 +53,12 @@ impl Hash for NixString {
 }
 
 impl NixString {
-    pub const NAME: Self = NixString(StringRepr::Static("name"));
-    pub const VALUE: Self = NixString(StringRepr::Static("value"));
+    pub const NAME: Self = NixString(StringRepr::Smol(SmolStr::new_inline("name")));
+    pub const VALUE: Self = NixString(StringRepr::Smol(SmolStr::new_inline("value")));
 
     pub fn as_str(&self) -> &str {
         match &self.0 {
-            StringRepr::Static(s) => s,
+            StringRepr::Smol(s) => s.as_str(),
             StringRepr::Heap(s) => s,
         }
     }