mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(tvix/eval): simplify NixString representation(s)

Browse source 
Instead of the two different representations (which we don't really
use much), use a `Box<str>` (which potentially shaves another 8 bytes
off `Value`).

NixString values themselves are immutable anyways (which was a
guarantee we already had with `SmolStr`), so this doesn't change
anything else.

Change-Id: I1d8454c056c21ecb0aebc473cfb3ae06cd70dbb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8151
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This commit is contained in:
Vincent Ambo 2023-02-27 13:32:03 +03:00 committed by tazjin
parent 4bbfeaf1cb
commit 1941082cbb
3 changed files with 37 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
tvix/eval/src/value/attrs.rs
View file

@ -8,16 +8,23 @@
use std::iter::FromIterator;
use imbl::{ordmap, OrdMap};
use lazy_static::lazy_static;
use serde::de::{Deserializer, Error, Visitor};
use serde::ser::SerializeMap;
use serde::{Deserialize, Serialize};
use crate::errors::ErrorKind;
use super::string::NixString;
use super::thunk::ThunkSet;
use super::TotalDisplay;
use super::Value;
use crate::errors::ErrorKind;
lazy_static! {
static ref NAME_S: NixString = "name".into();
static ref NAME_REF: &'static NixString = &NAME_S;
static ref VALUE_S: NixString = "value".into();
static ref VALUE_REF: &'static NixString = &VALUE_S;
}
#[cfg(test)]
mod tests;
@ -57,8 +64,8 @@ impl AttrsRep {
AttrsRep::KV { name, value } => {
*self = AttrsRep::Im(ordmap! {
NixString::NAME => name.clone(),
NixString::VALUE => value.clone()
NAME_S.clone() => name.clone(),
VALUE_S.clone() => value.clone()
});
self.map_mut()
@ -230,13 +237,13 @@ impl NixAttrs {
// Slightly more advanced, but still optimised updates
match (self.0, other.0) {
(AttrsRep::Im(mut m), AttrsRep::KV { name, value }) => {
m.insert(NixString::NAME, name);
m.insert(NixString::VALUE, value);
m.insert(NAME_S.clone(), name);
m.insert(VALUE_S.clone(), value);
NixAttrs(AttrsRep::Im(m))
}
(AttrsRep::KV { name, value }, AttrsRep::Im(mut m)) => {
match m.entry(NixString::NAME) {
match m.entry(NAME_S.clone()) {
imbl::ordmap::Entry::Vacant(e) => {
e.insert(name);
}
@ -244,7 +251,7 @@ impl NixAttrs {
imbl::ordmap::Entry::Occupied(_) => { /* name from `m` has precedence */ }
};
match m.entry(NixString::VALUE) {
match m.entry(VALUE_S.clone()) {
imbl::ordmap::Entry::Vacant(e) => {
e.insert(value);
}
@ -318,11 +325,7 @@ impl NixAttrs {
match self.0 {
AttrsRep::Empty => IntoIter(IntoIterRepr::Empty),
AttrsRep::KV { name, value } => IntoIter(IntoIterRepr::Finite(
vec![
(NixString::NAME_REF.clone(), name),
(NixString::VALUE_REF.clone(), value),
]
.into_iter(),
vec![(NAME_REF.clone(), name), (VALUE_REF.clone(), value)].into_iter(),
)),
AttrsRep::Im(map) => IntoIter(IntoIterRepr::Im(map.into_iter())),
}
@ -411,17 +414,9 @@ impl NixAttrs {
fn attempt_optimise_kv(slice: &mut [Value]) -> Option<NixAttrs> {
let (name_idx, value_idx) = {
match (&slice[2], &slice[0]) {
(Value::String(s1), Value::String(s2))
if (*s1 == NixString::NAME && *s2 == NixString::VALUE) =>
{
(3, 1)
}
(Value::String(s1), Value::String(s2)) if (*s1 == *NAME_S && *s2 == *VALUE_S) => (3, 1),
(Value::String(s1), Value::String(s2))
if (*s1 == NixString::VALUE && *s2 == NixString::NAME) =>
{
(1, 3)
}
(Value::String(s1), Value::String(s2)) if (*s1 == *VALUE_S && *s2 == *NAME_S) => (1, 3),
// Technically this branch lets type errors pass,
// but they will be caught during normal attribute
@ -502,12 +497,12 @@ impl<'a> Iterator for Iter<KeyValue<'a>> {
KeyValue::KV { name, value, at } => match at {
IterKV::Name => {
at.next();
Some((NixString::NAME_REF, name))
Some((&NAME_REF, name))
}
IterKV::Value => {
at.next();
Some((NixString::VALUE_REF, value))
Some((&VALUE_REF, value))
}
IterKV::Done => None,
@ -542,11 +537,11 @@ impl<'a> Iterator for Keys<'a> {
KeysInner::Empty => None,
KeysInner::KV(at @ IterKV::Name) => {
at.next();
Some(NixString::NAME_REF)
Some(&NAME_REF)
}
KeysInner::KV(at @ IterKV::Value) => {
at.next();
Some(NixString::VALUE_REF)
Some(&VALUE_REF)
}
KeysInner::KV(IterKV::Done) => None,
KeysInner::Im(m) => m.next(),

4
tvix/eval/src/value/attrs/tests.rs
View file

@ -84,10 +84,10 @@ fn test_kv_attrs_iter() {
.into_iter()
.map(|(k, v)| (k, v));
let (k, v) = iter.next().unwrap();
assert!(k == NixString::NAME_REF);
assert!(k == *NAME_REF);
assert!(v.to_str().unwrap() == meaning_val.to_str().unwrap());
let (k, v) = iter.next().unwrap();
assert!(k == NixString::VALUE_REF);
assert!(k == *VALUE_REF);
assert!(v.as_int().unwrap() == forty_two_val.as_int().unwrap());
assert!(iter.next().is_none());
}

49
tvix/eval/src/value/string.rs
View file

@ -1,5 +1,8 @@
//! This module implements Nix language strings and their different
//! backing implementations.
//! This module implements Nix language strings.
//!
//! Nix language strings never need to be modified on the language
//! level, allowing us to shave off some memory overhead and only
//! paying the cost when creating new strings.
use rnix::ast;
use smol_str::SmolStr;
use std::ffi::OsStr;
@ -11,16 +14,9 @@ use std::{borrow::Cow, fmt::Display, str::Chars};
use serde::de::{Deserializer, Visitor};
use serde::{Deserialize, Serialize};
#[derive(Clone, Debug, Serialize)]
#[serde(untagged)]
enum StringRepr {
Smol(SmolStr),
Heap(String),
}
#[repr(transparent)]
#[derive(Clone, Debug, Serialize)]
pub struct NixString(StringRepr);
pub struct NixString(Box<str>);
impl PartialEq for NixString {
fn eq(&self, other: &Self) -> bool {
@ -44,19 +40,19 @@ impl Ord for NixString {
impl From<&str> for NixString {
fn from(s: &str) -> Self {
NixString(StringRepr::Smol(SmolStr::new(s)))
NixString(Box::from(s))
}
}
impl From<String> for NixString {
fn from(s: String) -> Self {
NixString(StringRepr::Heap(s))
NixString(s.into_boxed_str())
}
}
impl From<SmolStr> for NixString {
fn from(s: SmolStr) -> Self {
NixString(StringRepr::Smol(s))
NixString(Box::from(s.as_str()))
}
}
@ -109,7 +105,6 @@ impl<'de> Deserialize<'de> for NixString {
mod arbitrary {
use super::*;
use proptest::prelude::{any_with, Arbitrary};
use proptest::prop_oneof;
use proptest::strategy::{BoxedStrategy, Strategy};
impl Arbitrary for NixString {
@ -118,29 +113,14 @@ mod arbitrary {
type Strategy = BoxedStrategy<Self>;
fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
prop_oneof![
// Either generate `StringRepr::Heap`...
any_with::<String>(args).prop_map(Self::from),
// ...or generate `StringRepr::Smol` (which `impl From<&str> for NixString` returns)
any_with::<String>(args).prop_map(|s| Self::from(s.as_str())),
]
.boxed()
any_with::<String>(args).prop_map(Self::from).boxed()
}
}
}
impl NixString {
pub const NAME: Self = NixString(StringRepr::Smol(SmolStr::new_inline("name")));
pub const NAME_REF: &'static Self = &Self::NAME;
pub const VALUE: Self = NixString(StringRepr::Smol(SmolStr::new_inline("value")));
pub const VALUE_REF: &'static Self = &Self::VALUE;
pub fn as_str(&self) -> &str {
match &self.0 {
StringRepr::Smol(s) => s.as_str(),
StringRepr::Heap(s) => s,
}
&self.0
}
/// Return a displayable representation of the string as an
@ -172,14 +152,11 @@ impl NixString {
pub fn concat(&self, other: &Self) -> Self {
let mut s = self.as_str().to_owned();
s.push_str(other.as_str());
NixString(StringRepr::Heap(s))
NixString(s.into_boxed_str())
}
pub fn chars(&self) -> Chars<'_> {
match &self.0 {
StringRepr::Heap(h) => h.chars(),
StringRepr::Smol(s) => s.chars(),
}
self.0.chars()
}
}