From 2754a60407767ebe3438932609dc40b3a9f1da48 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Wed, 4 Jan 2023 01:02:51 -0800 Subject: [PATCH] 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: https://github.com/bodil/smartstring/issues/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 --- Cargo.toml | 4 ++-- allocative/allocative/Cargo.toml | 2 +- allocative/allocative/TARGETS | 4 ++-- .../src/impls/{smartstring.rs => compact_str.rs} | 9 ++++----- allocative/allocative/src/impls/mod.rs | 2 +- app/buck2_core/Cargo.toml | 2 +- app/buck2_core/TARGETS | 2 +- app/buck2_core/src/fs/paths/file_name.rs | 13 +++++++------ shim/third-party/rust/Cargo.toml | 4 ++-- .../third-party/rust/fixups/compact_str/fixups.toml | 2 ++ 10 files changed, 23 insertions(+), 21 deletions(-) rename allocative/allocative/src/impls/{smartstring.rs => compact_str.rs} (83%) create mode 100644 shim/third-party/rust/fixups/compact_str/fixups.toml diff --git a/Cargo.toml b/Cargo.toml index 4e231fba04b1..79419013f44a 100644 --- 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 index ce9f576f6404..f0225ef36fe2 100644 --- 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 index b22010c41264..0477ab3e1ef3 100644 --- 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/allocative/allocative/src/impls/smartstring.rs b/allocative/allocative/src/impls/compact_str.rs similarity index 83% rename from allocative/allocative/src/impls/smartstring.rs rename to allocative/allocative/src/impls/compact_str.rs index 1b65768fdf89..f848df4ccdca 100644 --- a/allocative/allocative/src/impls/smartstring.rs +++ b/allocative/allocative/src/impls/compact_str.rs @@ -7,10 +7,9 @@ * 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; @@ -18,10 +17,10 @@ use crate::impls::common::UNUSED_CAPACITY_NAME; use crate::key::Key; use crate::visitor::Visitor; -impl Allocative for SmartString { +impl Allocative for CompactString { fn visit<'a, 'b: 'a>(&self, visitor: &'a mut Visitor<'b>) { let mut visitor = visitor.enter_self_sized::(); - 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 index 122ae96ce014..3e1b3914bc56 100644 --- 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 index 5b01687ae6ff..9124ca0b62f0 100644 --- 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 index 7342be41e7b1..c870c122257e 100644 --- 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 index 7ca4f81ebb3a..d7e733203213 100644 --- 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); +pub struct FileNameBuf(CompactString); impl FileNameBuf { pub fn unchecked_new(s: T) -> Self where - T: Into>, + T: Into, { + // 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 { + pub fn into_inner(self) -> CompactString { self.0 } @@ -271,6 +271,7 @@ impl TryFrom for FileNameBuf { type Error = anyhow::Error; fn try_from(value: String) -> anyhow::Result { + // 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 index cebd05e29ef5..0f02b06c9b6f 100644 --- 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 new file mode 100644 index 000000000000..7b68a94d3512 --- /dev/null +++ b/shim/third-party/rust/fixups/compact_str/fixups.toml @@ -0,0 +1,2 @@ +# Includes inline document files +extra_srcs = ["README.md"]