buck2: switch to compact_str instead of smartstring

Summary: Upsides: - It seems more actively maintained. - Note: on Github issues, they have a couple open reports of fuzz failures (from their automation) but those are from a branch, not master. - It doesn't have this frustrating bug: bodil/smartstring#7 Downsides: - It doesn't convert String to an inlined value in `From`, but we don't actually rely on this (we use small strings mostly when working with directories where those elements are produced via an iterator of FileName). Reviewed By: ndmitchell Differential Revision: D42313656 fbshipit-source-id: 0708866fced3b32941047755b770244287f6ad94
facebook · Jan 4, 2023 · 2754a60 · 2754a60
1 parent 2165bcf
commit 2754a60
Show file tree

Hide file tree

Showing 10 changed files with 23 additions and 21 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -87,6 +87,7 @@ crossbeam-channel = "0.5"
 crossbeam-epoch = "0.9.7"
 crossterm = "0.23"
 ctor = "0.1.26"
+compact_str = "0.6.1"
 dashmap = "4.0.2"
 derivative = "2.1.1"
 derive_more = "0.99.3"
@@ -164,7 +165,6 @@ shlex = "1.1"
 siphasher = "0.3.5"
 slog = "2.7.0"
 smallvec = "1.7"
-smartstring = "0.2.9"
 static_assertions = "1.1.0"
 strsim = "0.10.0"
 structopt = "0.3.0"
@@ -208,7 +208,7 @@ sorted_vector_map.version = "0.1"
 
 allocative.version = "0.2"
 allocative.path = "allocative/allocative"
-allocative.features = ["anyhow", "bumpalo", "dashmap", "either", "futures", "hashbrown", "indexmap", "num-bigint", "once_cell", "owning_ref", "parking_lot", "prost-types", "relative-path", "sequence_trie", "smallvec", "smartstring", "sorted_vector_map"]
+allocative.features = ["anyhow", "bumpalo", "dashmap", "either", "futures", "hashbrown", "indexmap", "num-bigint", "once_cell", "owning_ref", "parking_lot", "prost-types", "relative-path", "sequence_trie", "smallvec", "compact_str", "sorted_vector_map"]
 gazebo.version = "0.8.1"
 gazebo.features = ["str_pattern_extensions"]
 # @oss-disable: gazebo.path = "gazebo/gazebo"

diff --git a/allocative/allocative/Cargo.toml b/allocative/allocative/Cargo.toml
@@ -22,7 +22,7 @@ hashbrown = { version = "0.12.3", optional = true }
 indexmap = { version = "1.9.1", optional = true }
 num-bigint = { version = "0.4.3", optional = true }
 parking_lot = { version = "0.11.2", optional = true }
-smartstring = { version = "0.2.10", optional = true }
+compact_str = { version = "0.6.1", optional = true }
 once_cell = { version = "1.16.0", optional = true }
 owning_ref = { version = "0.4.1", optional = true }
 prost-types = { version = "0.11.2", optional = true }

diff --git a/allocative/allocative/TARGETS b/allocative/allocative/TARGETS
@@ -24,7 +24,7 @@ rust_library(
         "relative-path",
         "sequence_trie",
         "smallvec",
-        "smartstring",
+        "compact_str",
         "sorted_vector_map",
     ],
     rustc_flags = [
@@ -33,6 +33,7 @@ rust_library(
     deps = [
         "fbsource//third-party/rust:anyhow",
         "fbsource//third-party/rust:bumpalo",
+        "fbsource//third-party/rust:compact_str",
         "fbsource//third-party/rust:ctor",
         "fbsource//third-party/rust:dashmap",
         "fbsource//third-party/rust:either",
@@ -47,7 +48,6 @@ rust_library(
         "fbsource//third-party/rust:relative-path",
         "fbsource//third-party/rust:sequence_trie",
         "fbsource//third-party/rust:smallvec",
-        "fbsource//third-party/rust:smartstring",
         "//buck2/allocative/allocative_derive:allocative_derive",
         "//common/rust/shed/sorted_vector_map:sorted_vector_map",
     ],

diff --git a/...ative/allocative/src/impls/smartstring.rs → ...ative/allocative/src/impls/compact_str.rs b/...ative/allocative/src/impls/smartstring.rs → ...ative/allocative/src/impls/compact_str.rs
@@ -7,21 +7,20 @@
  * of this source tree.
  */
 
-#![cfg(feature = "smartstring")]
+#![cfg(feature = "compact_str")]
 
-use smartstring::SmartString;
-use smartstring::SmartStringMode;
+use compact_str::CompactString;
 
 use crate::allocative_trait::Allocative;
 use crate::impls::common::PTR_NAME;
 use crate::impls::common::UNUSED_CAPACITY_NAME;
 use crate::key::Key;
 use crate::visitor::Visitor;
 
-impl<M: SmartStringMode + 'static> Allocative for SmartString<M> {
+impl Allocative for CompactString {
     fn visit<'a, 'b: 'a>(&self, visitor: &'a mut Visitor<'b>) {
         let mut visitor = visitor.enter_self_sized::<Self>();
-        if !self.is_inline() {
+        if self.is_heap_allocated() {
             let mut visitor = visitor.enter_unique(PTR_NAME, std::mem::size_of::<*const u8>());
             visitor.visit_simple(Key::new("str"), self.len());
             let unused_capacity = self.capacity() - self.len();

diff --git a/allocative/allocative/src/impls/mod.rs b/allocative/allocative/src/impls/mod.rs
@@ -12,6 +12,7 @@
 mod anyhow;
 mod bumpalo;
 pub(crate) mod common;
+mod compact_str;
 mod dashmap;
 mod either;
 mod futures;
@@ -27,6 +28,5 @@ mod prost_types;
 mod relative_path;
 mod sequence_trie;
 mod smallvec;
-mod smartstring;
 mod sorted_vector_map;
 mod std;
diff --git a/app/buck2_core/Cargo.toml b/app/buck2_core/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2021"
 anyhow = { workspace = true }
 async-trait = { workspace = true }
 blake3 = { workspace = true }
+compact_str = { workspace = true }
 tempfile = { workspace = true }
 derive_more = { workspace = true }
 derivative = { workspace = true }
@@ -34,7 +35,6 @@ static_assertions = { workspace = true }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true }
 rand = { workspace = true }
-smartstring = { workspace = true }
 starlark_map = { workspace = true }
 
 gazebo = { workspace = true }

diff --git a/app/buck2_core/TARGETS b/app/buck2_core/TARGETS
@@ -25,6 +25,7 @@ rust_library(
     deps = [
         "fbsource//third-party/blake3:blake3-rust",
         "fbsource//third-party/rust:anyhow",
+        "fbsource//third-party/rust:compact_str",
         "fbsource//third-party/rust:dashmap",
         "fbsource//third-party/rust:derivative",
         "fbsource//third-party/rust:derive_more",
@@ -44,7 +45,6 @@ rust_library(
         "fbsource//third-party/rust:relative-path",
         "fbsource//third-party/rust:sequence_trie",
         "fbsource//third-party/rust:serde",
-        "fbsource//third-party/rust:smartstring",
         "fbsource//third-party/rust:static_assertions",
         "fbsource//third-party/rust:tempfile",
         "fbsource//third-party/rust:thiserror",

diff --git a/app/buck2_core/src/fs/paths/file_name.rs b/app/buck2_core/src/fs/paths/file_name.rs
@@ -14,11 +14,10 @@ use std::ops::Deref;
 use std::path::Path;
 
 use allocative::Allocative;
+use compact_str::CompactString;
 use derive_more::Display;
 use ref_cast::RefCast;
 use relative_path::RelativePath;
-use smartstring::LazyCompact;
-use smartstring::SmartString;
 use thiserror::Error;
 
 use crate::fs::paths::forward_rel_path::ForwardRelativePath;
@@ -177,23 +176,24 @@ impl ToOwned for FileName {
     type Owned = FileNameBuf;
 
     fn to_owned(&self) -> FileNameBuf {
-        FileNameBuf(self.0.into())
+        FileNameBuf(CompactString::new(&self.0))
     }
 }
 
 /// Owned version of [`FileName`].
 #[derive(Ord, PartialOrd, Eq, Display, Debug, Clone, Allocative)]
-pub struct FileNameBuf(SmartString<LazyCompact>);
+pub struct FileNameBuf(CompactString);
 
 impl FileNameBuf {
     pub fn unchecked_new<T>(s: T) -> Self
     where
-        T: Into<SmartString<LazyCompact>>,
+        T: Into<CompactString>,
     {
+        // NOTE: This does not turn a String into an inlined string if it's already allocated.
         Self(s.into())
     }
 
-    pub fn into_inner(self) -> SmartString<LazyCompact> {
+    pub fn into_inner(self) -> CompactString {
         self.0
     }
 
@@ -271,6 +271,7 @@ impl TryFrom<String> for FileNameBuf {
     type Error = anyhow::Error;
 
     fn try_from(value: String) -> anyhow::Result<FileNameBuf> {
+        // NOTE: This does not turn a String into an inlined string.
         verify_file_name(value.as_str())?;
         Ok(FileNameBuf(value.into()))
     }

diff --git a/shim/third-party/rust/Cargo.toml b/shim/third-party/rust/Cargo.toml
@@ -41,8 +41,9 @@ byteorder = "1.4.3"
 bytes = "1.0"
 bytesize = "1.1.0"
 chrono = "=0.4.19"  # Avoid a dependency iana-time-zone, which requires a fixup
-clap = { version = "3.1.18", features = ["derive", "env"] }
 clap-4 = { package = "clap", version = "4.0.7", features = ["derive", "env"] }
+clap = { version = "3.1.18", features = ["derive", "env"] }
+compact_str = "0.6"
 constant_time_eq = "0.2.4"
 convert_case = "0.4.0"
 criterion = { version = "0.3.4", features = ["async", "async_tokio", "html_reports"] }
@@ -133,7 +134,6 @@ shlex = "1.1"
 siphasher = "0.3.5"
 slog = "2.7.0"
 smallvec = "1.7"
-smartstring = "0.2.9"
 static_assertions = "1.1.0"
 strsim = "0.10.0"
 structopt = "0.3.23"

diff --git a/shim/third-party/rust/fixups/compact_str/fixups.toml b/shim/third-party/rust/fixups/compact_str/fixups.toml
@@ -0,0 +1,2 @@
+# Includes inline document files
+extra_srcs = ["README.md"]
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		# Includes inline document files
		extra_srcs = ["README.md"]