Use string interning for PkgNameBuf, etc.

jrray · jrray · commit 686d7beb20bc · 2023-06-19T19:49:05.000-07:00
Use the lasso crate to intern package name strings, so that any instances
of the same name share the same backing memory. These are also very cheap
to clone because it only clones the "Spur" (a u32).

The tradeoff is that there is some contention from using `ThreadedRodeo`
since we need to support accessing the backing store from multiple threads.

Signed-off-by: J Robert Ray &lt;jrray@jrray.org&gt;
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -40,6 +40,7 @@ expanduser = "1.2"
 futures = "0.3.24"
 fuser = "0.12"
 indicatif = "0.16.2"
+lasso = { version = "0.7", features = ["multi-threaded"] }
 lazy_static = "1.4"
 libc = "0.2.80"
 nix = "0.26.2"
diff --git a/crates/parsedbuf/Cargo.toml b/crates/parsedbuf/Cargo.toml
@@ -5,4 +5,6 @@ name = "parsedbuf"
 version = "0.1.0"
 
 [dependencies]
+once_cell = { workspace = true }
 paste = "1.0"
+lasso = { workspace = true }
diff --git a/crates/parsedbuf/src/lib.rs b/crates/parsedbuf/src/lib.rs
@@ -5,8 +5,14 @@
 #![deny(unsafe_op_in_unsafe_fn)]
 #![warn(clippy::fn_params_excessive_bools)]
 
+use std::sync::Arc;
+
 pub use paste;
 
+/// Global string interner for common strings.
+pub static RODEO: once_cell::sync::Lazy<Arc<lasso::ThreadedRodeo>> =
+    once_cell::sync::Lazy::new(|| Arc::new(lasso::ThreadedRodeo::default()));
+
 /// Generate a pair of types to represent a parsed string type.
 ///
 /// A `$type_name::validate()` method must be manually implemented which
@@ -42,9 +48,34 @@ macro_rules! parsed {
         }
 
         $crate::paste::paste! {
-            #[derive(Debug, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
+            #[derive(Debug, Clone, Eq, PartialEq)]
             #[doc = "An owned, mutable, and validated " $what " string"]
-            pub struct $owned_type_name(String);
+            pub struct $owned_type_name(lasso::Spur);
+        }
+
+        impl std::hash::Hash for $owned_type_name {
+            #[inline]
+            fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+                // Hash the interned string, not the Spur, for consistency
+                // with hash() on the borrowed type.
+                $crate::RODEO.resolve(&self.0).hash(state)
+            }
+        }
+
+        impl Ord for $owned_type_name {
+            #[inline]
+            fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+                // Order on the interned string, not the Spur, for consistency
+                // with Ord on the borrowed type (as required by BTreeMap).
+                self.as_str().cmp(other.as_str())
+            }
+        }
+
+        impl PartialOrd for $owned_type_name {
+            #[inline]
+            fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+                Some(self.cmp(other))
+            }
         }
 
         #[cfg(feature = "parsedbuf-serde")]
@@ -77,6 +108,7 @@ macro_rules! parsed {
                 })
             }
 
+            #[inline]
             pub fn as_str(&self) -> &str {
                 &self.0
             }
@@ -93,10 +125,12 @@ macro_rules! parsed {
                 }
             }
 
+            #[inline]
             pub fn is_empty(&self) -> bool {
                 self.0.is_empty()
             }
 
+            #[inline]
             pub fn len(&self) -> usize {
                 self.0.len()
             }
@@ -112,127 +146,139 @@ macro_rules! parsed {
                 #[doc = ""]
                 #[doc = "No validation is performed on `name`."]
                 pub unsafe fn from_string(name: String) -> Self {
-                    Self(name)
-                }
-            }
-
-            $crate::paste::paste! {
-                #[doc = "Consume the [`" $owned_type_name "`], returning the inner [`String`]."]
-                pub fn into_inner(self) -> String {
-                    self.0
+                    let key = $crate::RODEO.try_get_or_intern(name).expect("won't run out of intern slots");
+                    Self(key)
                 }
             }
         }
 
         impl std::borrow::Borrow<$type_name> for $owned_type_name {
+            #[inline]
             fn borrow(&self) -> &$type_name {
                 self.as_ref()
             }
         }
 
-        impl std::borrow::Borrow<String> for $owned_type_name {
-            fn borrow(&self) -> &String {
-                &self.0
+        impl std::borrow::Borrow<str> for $owned_type_name {
+            #[inline]
+            fn borrow(&self) -> &str {
+                $crate::RODEO.resolve(&self.0)
             }
         }
 
         impl std::borrow::ToOwned for $type_name {
             type Owned = $owned_type_name;
 
             fn to_owned(&self) -> Self::Owned {
-                $owned_type_name(self.0.to_owned())
+                let key = $crate::RODEO.try_get_or_intern(&self.0).expect("won't run out of intern slots");
+                $owned_type_name(key)
             }
         }
 
         impl std::cmp::PartialEq<$type_name> for $owned_type_name {
+            #[inline]
             fn eq(&self, other: &$type_name) -> bool {
-                &**self == other
+                self.as_str() == &other.0
             }
         }
 
         impl std::cmp::PartialEq<$owned_type_name> for $type_name {
+            #[inline]
             fn eq(&self, other: &$owned_type_name) -> bool {
                 &self.0 == other.as_str()
             }
         }
 
         impl std::cmp::PartialEq<$owned_type_name> for &$type_name {
+            #[inline]
             fn eq(&self, other: &$owned_type_name) -> bool {
                 &self.0 == other.as_str()
             }
         }
 
         impl std::cmp::PartialEq<str> for $type_name {
+            #[inline]
             fn eq(&self, other: &str) -> bool {
-                self.as_str() == other
+                &self.0 == other
             }
         }
 
         impl std::cmp::PartialEq<str> for $owned_type_name {
+            #[inline]
             fn eq(&self, other: &str) -> bool {
-                &**self == other
+                self.as_str() == other
             }
         }
 
         impl std::convert::AsRef<$type_name> for $type_name {
+            #[inline]
             fn as_ref(&self) -> &$type_name {
                 self
             }
         }
 
         impl std::convert::AsRef<$type_name> for $owned_type_name {
+            #[inline]
             fn as_ref(&self) -> &$type_name {
                 // Safety: from_str bypasses validation but the contents
                 // of owned instance must already be valid
-                unsafe { $type_name::from_str(&self.0) }
+                unsafe { $type_name::from_str($crate::RODEO.resolve(&self.0)) }
             }
         }
 
         impl std::convert::AsRef<std::ffi::OsStr> for $type_name {
+            #[inline]
             fn as_ref(&self) -> &std::ffi::OsStr {
                 std::ffi::OsStr::new(&self.0)
             }
         }
 
         impl std::convert::AsRef<std::path::Path> for $type_name {
+            #[inline]
             fn as_ref(&self) -> &std::path::Path {
                 std::path::Path::new(&self.0)
             }
         }
 
         impl std::convert::AsRef<std::path::Path> for $owned_type_name {
+            #[inline]
             fn as_ref(&self) -> &std::path::Path {
-                std::path::Path::new(&self.0)
+                std::path::Path::new($crate::RODEO.resolve(&self.0))
             }
         }
 
         impl std::convert::AsRef<str> for $owned_type_name {
+            #[inline]
             fn as_ref(&self) -> &str {
-                &self.0
+                $crate::RODEO.resolve(&self.0)
             }
         }
 
         impl std::cmp::PartialEq<&str> for $type_name {
+            #[inline]
             fn eq(&self, other: &&str) -> bool {
-                self.as_str() == *other
+                &self.0 == *other
             }
         }
 
         impl std::cmp::PartialEq<&str> for $owned_type_name {
+            #[inline]
             fn eq(&self, other: &&str) -> bool {
-                &**self == other
+                self.as_str() == *other
             }
         }
 
         impl std::convert::From<&$type_name> for $owned_type_name {
+            #[inline]
             fn from(name: &$type_name) -> Self {
                 name.to_owned()
             }
         }
 
         impl std::convert::From<$owned_type_name> for String {
+            #[inline]
             fn from(val: $owned_type_name) -> Self {
-                val.0
+                $crate::RODEO.resolve(&val.0).to_string()
             }
         }
 
@@ -276,7 +322,7 @@ macro_rules! parsed {
 
         impl std::fmt::Display for $owned_type_name {
             fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-                self.0.fmt(f)
+                $crate::RODEO.resolve(&self.0).fmt(f)
             }
         }
 
diff --git a/crates/spk-schema/crates/foundation/Cargo.toml b/crates/spk-schema/crates/foundation/Cargo.toml
@@ -17,6 +17,7 @@ enum_dispatch = "0.3.8"
 format_serde_error = {version = "0.3", default_features = false, features = ["serde_yaml", "colored"]}
 ignore = "0.4.18"
 itertools = "0.10"
+lasso = { workspace = true }
 nom = { workspace = true }
 nom-supreme = { workspace = true }
 once_cell = { workspace = true }
diff --git a/crates/spk-schema/crates/foundation/src/name/error.rs b/crates/spk-schema/crates/foundation/src/name/error.rs
@@ -10,4 +10,6 @@ pub type Result<T> = std::result::Result<T, Error>;
 pub enum Error {
     #[error(transparent)]
     InvalidNameError(#[from] super::InvalidNameError),
+    #[error(transparent)]
+    LassoError(#[from] lasso::LassoError),
 }
diff --git a/crates/spk-schema/crates/foundation/src/name/mod.rs b/crates/spk-schema/crates/foundation/src/name/mod.rs
@@ -208,17 +208,23 @@ impl OptName {
 
     /// Return a copy of this option, adding the provided namespace if there isn't already one set
     pub fn with_default_namespace<N: AsRef<PkgName>>(&self, ns: N) -> OptNameBuf {
-        OptNameBuf(format!(
-            "{}{}{}",
-            self.namespace().unwrap_or_else(|| ns.as_ref()),
-            Self::SEP,
-            self.base_name()
-        ))
+        // Safety: the individual components are already validated.
+        unsafe {
+            OptNameBuf::from_string(format!(
+                "{}{}{}",
+                self.namespace().unwrap_or_else(|| ns.as_ref()),
+                Self::SEP,
+                self.base_name()
+            ))
+        }
     }
 
     /// Return a copy of this option, replacing any namespace with the provided one
     pub fn with_namespace<N: AsRef<PkgName>>(&self, ns: N) -> OptNameBuf {
-        OptNameBuf(format!("{}{}{}", ns.as_ref(), Self::SEP, self.base_name()))
+        // Safety: the individual components are already validated.
+        unsafe {
+            OptNameBuf::from_string(format!("{}{}{}", ns.as_ref(), Self::SEP, self.base_name()))
+        }
     }
 
     /// Return an option with the same name but no associated namespace
@@ -336,6 +342,6 @@ impl RepositoryName {
 impl RepositoryNameBuf {
     /// Return if this RepositoryName names the "local" repository
     pub fn is_local(&self) -> bool {
-        self.0 == "local"
+        self.as_str() == "local"
     }
 }
diff --git a/crates/spk-schema/crates/ident/src/parsing/parsing_test.rs b/crates/spk-schema/crates/ident/src/parsing/parsing_test.rs

Original file line number	Diff line number	Diff line change
`@@ -5,8 +5,14 @@`
`5`	`5`	`#![deny(unsafe_op_in_unsafe_fn)]`
`6`	`6`	`#![warn(clippy::fn_params_excessive_bools)]`
`7`	`7`
	`8`	`+use std::sync::Arc;`
	`9`	`+`
`8`	`10`	`pub use paste;`
`9`	`11`
	`12`	`+/// Global string interner for common strings.`
	`13`	`+pub static RODEO: once_cell::sync::Lazy<Arc<lasso::ThreadedRodeo>> =`
	`14`	`+ once_cell::sync::Lazy::new(\|\| Arc::new(lasso::ThreadedRodeo::default()));`
	`15`	`+`
`10`	`16`	`/// Generate a pair of types to represent a parsed string type.`
`11`	`17`	`///`
`12`	`18`	/// A `$type_name::validate()` method must be manually implemented which
`@@ -42,9 +48,34 @@ macro_rules! parsed {`
`42`	`48`	`}`
`43`	`49`
`44`	`50`	`$crate::paste::paste! {`
`45`		`- #[derive(Debug, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]`
	`51`	`+ #[derive(Debug, Clone, Eq, PartialEq)]`
`46`	`52`	`#[doc = "An owned, mutable, and validated " $what " string"]`
`47`		`- pub struct $owned_type_name(String);`
	`53`	`+ pub struct $owned_type_name(lasso::Spur);`
	`54`	`+ }`
	`55`	`+`
	`56`	`+ impl std::hash::Hash for $owned_type_name {`
	`57`	`+ #[inline]`
	`58`	`+ fn hash<H: std::hash::Hasher>(&self, state: &mut H) {`
	`59`	`+ // Hash the interned string, not the Spur, for consistency`
	`60`	`+ // with hash() on the borrowed type.`
	`61`	`+ $crate::RODEO.resolve(&self.0).hash(state)`
	`62`	`+ }`
	`63`	`+ }`
	`64`	`+`
	`65`	`+ impl Ord for $owned_type_name {`
	`66`	`+ #[inline]`
	`67`	`+ fn cmp(&self, other: &Self) -> std::cmp::Ordering {`
	`68`	`+ // Order on the interned string, not the Spur, for consistency`
	`69`	`+ // with Ord on the borrowed type (as required by BTreeMap).`
	`70`	`+ self.as_str().cmp(other.as_str())`
	`71`	`+ }`
	`72`	`+ }`
	`73`	`+`
	`74`	`+ impl PartialOrd for $owned_type_name {`
	`75`	`+ #[inline]`
	`76`	`+ fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {`
	`77`	`+ Some(self.cmp(other))`
	`78`	`+ }`
`48`	`79`	`}`
`49`	`80`
`50`	`81`	`#[cfg(feature = "parsedbuf-serde")]`
`@@ -77,6 +108,7 @@ macro_rules! parsed {`
`77`	`108`	`})`
`78`	`109`	`}`
`79`	`110`
	`111`	`+ #[inline]`
`80`	`112`	`pub fn as_str(&self) -> &str {`
`81`	`113`	`&self.0`
`82`	`114`	`}`
`@@ -93,10 +125,12 @@ macro_rules! parsed {`
`93`	`125`	`}`
`94`	`126`	`}`
`95`	`127`
	`128`	`+ #[inline]`
`96`	`129`	`pub fn is_empty(&self) -> bool {`
`97`	`130`	`self.0.is_empty()`
`98`	`131`	`}`
`99`	`132`
	`133`	`+ #[inline]`
`100`	`134`	`pub fn len(&self) -> usize {`
`101`	`135`	`self.0.len()`
`102`	`136`	`}`
`@@ -112,127 +146,139 @@ macro_rules! parsed {`
`112`	`146`	`#[doc = ""]`
`113`	`147`	#[doc = "No validation is performed on `name`."]
`114`	`148`	`pub unsafe fn from_string(name: String) -> Self {`
`115`		`- Self(name)`
`116`		`- }`
`117`		`- }`
`118`		`-`
`119`		`- $crate::paste::paste! {`
`120`		- #[doc = "Consume the [`" $owned_type_name "`], returning the inner [`String`]."]
`121`		`- pub fn into_inner(self) -> String {`
`122`		`- self.0`
	`149`	`+ let key = $crate::RODEO.try_get_or_intern(name).expect("won't run out of intern slots");`
	`150`	`+ Self(key)`
`123`	`151`	`}`
`124`	`152`	`}`
`125`	`153`	`}`
`126`	`154`
`127`	`155`	`impl std::borrow::Borrow<$type_name> for $owned_type_name {`
	`156`	`+ #[inline]`
`128`	`157`	`fn borrow(&self) -> &$type_name {`
`129`	`158`	`self.as_ref()`
`130`	`159`	`}`
`131`	`160`	`}`
`132`	`161`
`133`		`- impl std::borrow::Borrow<String> for $owned_type_name {`
`134`		`- fn borrow(&self) -> &String {`
`135`		`- &self.0`
	`162`	`+ impl std::borrow::Borrow<str> for $owned_type_name {`
	`163`	`+ #[inline]`
	`164`	`+ fn borrow(&self) -> &str {`
	`165`	`+ $crate::RODEO.resolve(&self.0)`
`136`	`166`	`}`
`137`	`167`	`}`
`138`	`168`
`139`	`169`	`impl std::borrow::ToOwned for $type_name {`
`140`	`170`	`type Owned = $owned_type_name;`
`141`	`171`
`142`	`172`	`fn to_owned(&self) -> Self::Owned {`
`143`		`- $owned_type_name(self.0.to_owned())`
	`173`	`+ let key = $crate::RODEO.try_get_or_intern(&self.0).expect("won't run out of intern slots");`
	`174`	`+ $owned_type_name(key)`
`144`	`175`	`}`
`145`	`176`	`}`
`146`	`177`
`147`	`178`	`impl std::cmp::PartialEq<$type_name> for $owned_type_name {`
	`179`	`+ #[inline]`
`148`	`180`	`fn eq(&self, other: &$type_name) -> bool {`
`149`		`- &**self == other`
	`181`	`+ self.as_str() == &other.0`
`150`	`182`	`}`
`151`	`183`	`}`
`152`	`184`
`153`	`185`	`impl std::cmp::PartialEq<$owned_type_name> for $type_name {`
	`186`	`+ #[inline]`
`154`	`187`	`fn eq(&self, other: &$owned_type_name) -> bool {`
`155`	`188`	`&self.0 == other.as_str()`
`156`	`189`	`}`
`157`	`190`	`}`
`158`	`191`
`159`	`192`	`impl std::cmp::PartialEq<$owned_type_name> for &$type_name {`
	`193`	`+ #[inline]`
`160`	`194`	`fn eq(&self, other: &$owned_type_name) -> bool {`
`161`	`195`	`&self.0 == other.as_str()`
`162`	`196`	`}`
`163`	`197`	`}`
`164`	`198`
`165`	`199`	`impl std::cmp::PartialEq<str> for $type_name {`
	`200`	`+ #[inline]`
`166`	`201`	`fn eq(&self, other: &str) -> bool {`
`167`		`- self.as_str() == other`
	`202`	`+ &self.0 == other`
`168`	`203`	`}`
`169`	`204`	`}`
`170`	`205`
`171`	`206`	`impl std::cmp::PartialEq<str> for $owned_type_name {`
	`207`	`+ #[inline]`
`172`	`208`	`fn eq(&self, other: &str) -> bool {`
`173`		`- &**self == other`
	`209`	`+ self.as_str() == other`
`174`	`210`	`}`
`175`	`211`	`}`
`176`	`212`
`177`	`213`	`impl std::convert::AsRef<$type_name> for $type_name {`
	`214`	`+ #[inline]`
`178`	`215`	`fn as_ref(&self) -> &$type_name {`
`179`	`216`	`self`
`180`	`217`	`}`
`181`	`218`	`}`
`182`	`219`
`183`	`220`	`impl std::convert::AsRef<$type_name> for $owned_type_name {`
	`221`	`+ #[inline]`
`184`	`222`	`fn as_ref(&self) -> &$type_name {`
`185`	`223`	`// Safety: from_str bypasses validation but the contents`
`186`	`224`	`// of owned instance must already be valid`
`187`		`- unsafe { $type_name::from_str(&self.0) }`
	`225`	`+ unsafe { $type_name::from_str($crate::RODEO.resolve(&self.0)) }`
`188`	`226`	`}`
`189`	`227`	`}`
`190`	`228`
`191`	`229`	`impl std::convert::AsRef<std::ffi::OsStr> for $type_name {`
	`230`	`+ #[inline]`
`192`	`231`	`fn as_ref(&self) -> &std::ffi::OsStr {`
`193`	`232`	`std::ffi::OsStr::new(&self.0)`
`194`	`233`	`}`
`195`	`234`	`}`
`196`	`235`
`197`	`236`	`impl std::convert::AsRef<std::path::Path> for $type_name {`
	`237`	`+ #[inline]`
`198`	`238`	`fn as_ref(&self) -> &std::path::Path {`
`199`	`239`	`std::path::Path::new(&self.0)`
`200`	`240`	`}`
`201`	`241`	`}`
`202`	`242`
`203`	`243`	`impl std::convert::AsRef<std::path::Path> for $owned_type_name {`
	`244`	`+ #[inline]`
`204`	`245`	`fn as_ref(&self) -> &std::path::Path {`
`205`		`- std::path::Path::new(&self.0)`
	`246`	`+ std::path::Path::new($crate::RODEO.resolve(&self.0))`
`206`	`247`	`}`
`207`	`248`	`}`
`208`	`249`
`209`	`250`	`impl std::convert::AsRef<str> for $owned_type_name {`
	`251`	`+ #[inline]`
`210`	`252`	`fn as_ref(&self) -> &str {`
`211`		`- &self.0`
	`253`	`+ $crate::RODEO.resolve(&self.0)`
`212`	`254`	`}`
`213`	`255`	`}`
`214`	`256`
`215`	`257`	`impl std::cmp::PartialEq<&str> for $type_name {`
	`258`	`+ #[inline]`
`216`	`259`	`fn eq(&self, other: &&str) -> bool {`
`217`		`- self.as_str() == *other`
	`260`	`+ &self.0 == *other`
`218`	`261`	`}`
`219`	`262`	`}`
`220`	`263`
`221`	`264`	`impl std::cmp::PartialEq<&str> for $owned_type_name {`
	`265`	`+ #[inline]`
`222`	`266`	`fn eq(&self, other: &&str) -> bool {`
`223`		`- &**self == other`
	`267`	`+ self.as_str() == *other`
`224`	`268`	`}`
`225`	`269`	`}`
`226`	`270`
`227`	`271`	`impl std::convert::From<&$type_name> for $owned_type_name {`
	`272`	`+ #[inline]`
`228`	`273`	`fn from(name: &$type_name) -> Self {`
`229`	`274`	`name.to_owned()`
`230`	`275`	`}`
`231`	`276`	`}`
`232`	`277`
`233`	`278`	`impl std::convert::From<$owned_type_name> for String {`
	`279`	`+ #[inline]`
`234`	`280`	`fn from(val: $owned_type_name) -> Self {`
`235`		`- val.0`
	`281`	`+ $crate::RODEO.resolve(&val.0).to_string()`
`236`	`282`	`}`
`237`	`283`	`}`
`238`	`284`
`@@ -276,7 +322,7 @@ macro_rules! parsed {`
`276`	`322`
`277`	`323`	`impl std::fmt::Display for $owned_type_name {`
`278`	`324`	`fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {`
`279`		`- self.0.fmt(f)`
	`325`	`+ $crate::RODEO.resolve(&self.0).fmt(f)`
`280`	`326`	`}`
`281`	`327`	`}`
`282`	`328`
Original file line number	Diff line number	Diff line change
`@@ -10,4 +10,6 @@ pub type Result<T> = std::result::Result<T, Error>;`
`10`	`10`	`pub enum Error {`
`11`	`11`	`#[error(transparent)]`
`12`	`12`	`InvalidNameError(#[from] super::InvalidNameError),`
	`13`	`+ #[error(transparent)]`
	`14`	`+ LassoError(#[from] lasso::LassoError),`
`13`	`15`	`}`