From 6dc9ca5723ee46ade3ad3618ae780ec88ae884e2 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Wed, 10 Aug 2022 17:53:22 +0300 Subject: [PATCH] feat(tvix/value): introduce string representation with &'static str For cases where the strings are statically known (such as the oft-occuring name/value), this can be a useful optimisation. It's also much more convenient in tests. Change-Id: Ie462b684805bd4986ea5e85ca4bff663bc2d3c3c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6111 Tested-by: BuildkiteCI Reviewed-by: eta --- tvix/eval/src/compiler.rs | 8 ++++---- tvix/eval/src/value/attrs.rs | 22 ++++++++-------------- tvix/eval/src/value/string.rs | 34 +++++++++++++++++++++++++++++----- tvix/eval/src/vm.rs | 6 +++--- 4 files changed, 44 insertions(+), 26 deletions(-) diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs index 5b6f748dc..668ec842e 100644 --- a/tvix/eval/src/compiler.rs +++ b/tvix/eval/src/compiler.rs @@ -111,7 +111,7 @@ impl Compiler { rnix::StrPart::Ast(node) => self.compile(node)?, rnix::StrPart::Literal(lit) => { - let idx = self.chunk.add_constant(Value::String(NixString(lit))); + let idx = self.chunk.add_constant(Value::String(lit.into())); self.chunk.add_op(OpCode::OpConstant(idx)); } } @@ -206,9 +206,9 @@ impl Compiler { let ident = rnix::types::Ident::cast(fragment).unwrap(); // TODO(tazjin): intern! - let idx = self - .chunk - .add_constant(Value::String(NixString(ident.as_str().to_string()))); + let idx = self.chunk.add_constant(Value::String(NixString::Heap( + ident.as_str().to_string(), + ))); self.chunk.add_op(OpCode::OpConstant(idx)); } diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index 75ade6a89..7e3b5f231 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -71,14 +71,8 @@ impl NixAttrs { NixAttrs::KV { name, value } => { *self = NixAttrs::Map(BTreeMap::from([ - ( - NixString("name".into()), - std::mem::replace(name, Value::Blackhole), - ), - ( - NixString("value".into()), - std::mem::replace(value, Value::Blackhole), - ), + ("name".into(), std::mem::replace(name, Value::Blackhole)), + ("value".into(), std::mem::replace(value, Value::Blackhole)), ])); self.map_mut() } @@ -155,14 +149,14 @@ impl NixAttrs { fn attempt_optimise_kv(slice: &mut [Value]) -> Option { let (name_idx, value_idx) = { match (&slice[2], &slice[0]) { - (Value::String(NixString(s1)), Value::String(NixString(s2))) - if (s1 == "name" && s2 == "value") => + (Value::String(s1), Value::String(s2)) + if (*s1 == NixString::NAME && *s2 == NixString::VALUE) => { (3, 1) } - (Value::String(NixString(s1)), Value::String(NixString(s2))) - if (s1 == "value" && s2 == "name") => + (Value::String(s1), Value::String(s2)) + if (*s1 == NixString::VALUE && *s2 == NixString::NAME) => { (1, 3) } @@ -189,7 +183,7 @@ fn set_attr(attrs: &mut NixAttrs, key: NixString, value: Value) -> EvalResult<() match entry { std::collections::btree_map::Entry::Occupied(entry) => { return Err(Error::DuplicateAttrsKey { - key: entry.key().0.clone(), + key: entry.key().as_str().to_string(), }) } @@ -254,7 +248,7 @@ fn set_nested_attr( _ => { return Err(Error::DuplicateAttrsKey { - key: entry.key().0.clone(), + key: entry.key().as_str().to_string(), }) } }, diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs index d4776caea..47661e03a 100644 --- a/tvix/eval/src/value/string.rs +++ b/tvix/eval/src/value/string.rs @@ -4,16 +4,40 @@ use std::fmt::Display; /// backing implementations. #[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] -pub struct NixString(pub String); +pub enum NixString { + Static(&'static str), + Heap(String), +} impl Display for NixString { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(self.0.as_str()) + match self { + NixString::Static(s) => f.write_str(s), + NixString::Heap(s) => f.write_str(s), + } } } -impl From<&str> for NixString { - fn from(s: &str) -> Self { - NixString(s.to_string()) +impl From<&'static str> for NixString { + fn from(s: &'static str) -> Self { + NixString::Static(s) + } +} + +impl From for NixString { + fn from(s: String) -> Self { + NixString::Heap(s) + } +} + +impl NixString { + pub const NAME: Self = NixString::Static("name"); + pub const VALUE: Self = NixString::Static("value"); + + pub fn as_str(&self) -> &str { + match self { + NixString::Static(s) => s, + NixString::Heap(s) => s, + } } } diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index a58d77cc2..a529c5a79 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -7,7 +7,7 @@ use crate::{ chunk::Chunk, errors::{Error, EvalResult}, opcode::OpCode, - value::{NixAttrs, NixList, NixString, Value}, + value::{NixAttrs, NixList, Value}, }; pub struct VM { @@ -159,10 +159,10 @@ impl VM { let mut out = String::new(); for _ in 0..count { - out.push_str(&self.pop().as_string()?.0); + out.push_str(&self.pop().as_string()?.as_str()); } - self.push(Value::String(NixString(out))); + self.push(Value::String(out.into())); Ok(()) }