From 5afb6123290d63c565439860a25a625ba4c17e4f Mon Sep 17 00:00:00 2001 From: Thomas Pendock Date: Thu, 17 Sep 2026 19:58:56 -0700 Subject: [PATCH 1/6] impl ArcColdString --- cold-string/README.md | 29 ++- cold-string/src/arc.rs | 381 ++++++++++++++++++++++++++++++++++ cold-string/src/encoded.rs | 206 ++++++++++++++++++ cold-string/src/heap.rs | 76 +++++++ cold-string/src/lib.rs | 287 ++++--------------------- cold-string/src/vint.rs | 2 +- cold-string/tests/property.rs | 19 ++ 7 files changed, 745 insertions(+), 255 deletions(-) create mode 100644 cold-string/src/arc.rs create mode 100644 cold-string/src/encoded.rs create mode 100644 cold-string/src/heap.rs diff --git a/cold-string/README.md b/cold-string/README.md index aae5cbb..1a90a8e 100644 --- a/cold-string/README.md +++ b/cold-string/README.md @@ -33,6 +33,20 @@ let s = ColdString::new("qwerty"); assert_eq!(s.as_str(), "qwerty"); ``` +Use `ArcColdString` when clones should share long-string storage: + +```rust +use cold_string::ArcColdString; + +let first = ArcColdString::new("a string longer than one machine word"); +let second = first.clone(); +assert_eq!(first, second); +``` + +Both types keep strings up to one machine word inline. For longer strings, +`ArcColdString` stores `[atomic reference count][variable-length length][UTF-8 bytes]` +in one allocation and does not support weak references. + Packs well with other types: ```rust use cold_string::ColdString; @@ -47,19 +61,14 @@ assert_eq!(size_of::>(), size_of::()); ColdString is an 8-byte tagged pointer (4 bytes on 32-bit machines): -```rust -use std::ptr::NonNull; - -#[repr(transparent)] -pub struct ColdString { - encoded: NonNull, -} -``` +Internally, both string types use a one-word tagged pointer representation. +`ColdString` points to a `[variable-length length][UTF-8 bytes]` allocation, +while `ArcColdString` adds an atomic reference count before the same payload. The 8 bytes encode one of three representations indicated by the 1st byte: -- `10xxxxxx`: `encoded` contains a tagged heap pointer. To decode the address, clear the tag bits (`10 → 00`) and rotate so the `00` bits become the least-significant bits. The heap allocation uses [4-byte alignment](https://doc.rust-lang.org/beta/std/alloc/struct.Layout.html#method.from_size_align), guaranteeing the -least-significant 2 bits of the address are `00`. On the heap, the UTF-8 characters are preceded by the variable-length encoding of the size. The size uses 1 byte for 0 - 127, 2 bytes for 128 - 16383, etc. +- `10xxxxxx`: `encoded` contains a tagged heap pointer. To decode the address, clear the tag bits (`10 → 00`) and rotate so the `00` bits become the least-significant bits. The heap allocation uses [4-byte alignment](https://doc.rust-lang.org/beta/std/alloc/struct.Layout.html#method.from_size_align), guaranteeing the least-significant 2 bits of the address are `00`. On the heap, the UTF-8 characters are preceded by the variable-length encoding of the size. The size uses 1 byte for 0 - 127, 2 bytes for 128 - 16383, etc. - `11111xxx`: xxx is the length and the remaining 0-7 bytes are UTF-8 characters. - `xxxxxxxx`: All 8 bytes are UTF-8. + The exception is if `encoded` is `usize::MAX`, the UTF-8 bytes are "\0\0\0\0\0\0\0\0". `10xxxxxx` and `11111xxx` are chosen because they cannot be valid first bytes of UTF-8. diff --git a/cold-string/src/arc.rs b/cold-string/src/arc.rs new file mode 100644 index 0000000..c031254 --- /dev/null +++ b/cold-string/src/arc.rs @@ -0,0 +1,381 @@ +use alloc::{ + borrow::{Cow, ToOwned}, + boxed::Box, + str::Utf8Error, + string::String, +}; +use core::{ + cmp::Ordering, + fmt, + hash::{Hash, Hasher}, + iter::FromIterator, + ops::Deref, + str, + sync::atomic::{fence, AtomicUsize, Ordering as AtomicOrdering}, +}; + +use crate::encoded::Encoded; + +const IMMORTAL: usize = 1; +const REF_ONE: usize = 2; +const MAX_REFCOUNT: usize = usize::MAX / 4; + +/// A one-word, atomically reference-counted immutable UTF-8 string. +/// +/// Strings up to one machine word are stored inline. Longer strings use one +/// allocation containing the reference count, variable-length length, and bytes. +/// +/// ``` +/// use cold_string::ArcColdString; +/// +/// let first = ArcColdString::new("a string longer than one machine word"); +/// let second = first.clone(); +/// assert_eq!(first, second); +/// ``` +#[repr(transparent)] +pub struct ArcColdString { + encoded: Encoded, +} + +impl ArcColdString { + pub fn from_utf8>(bytes: B) -> Result { + Ok(Self::new(str::from_utf8(bytes.as_ref())?)) + } + + /// # Safety + /// + /// `bytes` must contain valid UTF-8. + pub unsafe fn from_utf8_unchecked>(bytes: B) -> Self { + Self::new(str::from_utf8_unchecked(bytes.as_ref())) + } + + pub fn new>(value: T) -> Self { + let s = value.as_ref(); + Self { + encoded: Encoded::new(s, AtomicUsize::new(REF_ONE)), + } + } + + #[rustversion::since(1.61)] + #[inline] + pub const fn new_inline_const(s: &str) -> Self { + Self { + encoded: Encoded::new_inline_const(s), + } + } + + #[inline] + fn count(&self) -> &AtomicUsize { + debug_assert!(!self.is_inline()); + unsafe { &(*self.encoded.heap_ptr().as_ptr()).header } + } + + #[inline] + pub fn is_inline(&self) -> bool { + self.encoded.is_inline() + } + + #[inline] + pub fn len(&self) -> usize { + self.encoded.len() + } + + #[inline] + pub fn as_bytes(&self) -> &[u8] { + self.encoded.as_bytes() + } + + #[inline] + pub fn as_str(&self) -> &str { + // SAFETY: constructors accept only valid UTF-8. + unsafe { str::from_utf8_unchecked(self.as_bytes()) } + } + + #[inline] + pub fn is_empty(&self) -> bool { + self.len() == 0 + } +} + +impl Clone for ArcColdString { + #[inline] + fn clone(&self) -> Self { + if !self.is_inline() { + let count = self.count(); + if count.load(AtomicOrdering::Relaxed) & IMMORTAL == 0 { + let old = count.fetch_add(REF_ONE, AtomicOrdering::Relaxed); + if old >> 1 >= MAX_REFCOUNT { + // Permanently leak the allocation instead of allowing the + // count to wrap. The spare range covers racing increments. + count.fetch_or(IMMORTAL, AtomicOrdering::Relaxed); + } + } + } + + Self { + encoded: self.encoded, + } + } +} + +impl Drop for ArcColdString { + #[inline] + fn drop(&mut self) { + if self.is_inline() { + return; + } + + let count = self.count(); + if count.load(AtomicOrdering::Relaxed) & IMMORTAL != 0 { + return; + } + + let old = count.fetch_sub(REF_ONE, AtomicOrdering::Release); + if old & IMMORTAL != 0 || old != REF_ONE { + return; + } + + fence(AtomicOrdering::Acquire); + // SAFETY: this was the last reference and the count is synchronized. + unsafe { self.encoded.deallocate() } + } +} + +impl Default for ArcColdString { + fn default() -> Self { + Self::new("") + } +} + +impl Deref for ArcColdString { + type Target = str; + + fn deref(&self) -> &str { + self.as_str() + } +} + +impl PartialEq for ArcColdString { + fn eq(&self, other: &Self) -> bool { + self.encoded.addr() == other.encoded.addr() || self.as_bytes() == other.as_bytes() + } +} + +impl Eq for ArcColdString {} + +impl Hash for ArcColdString { + fn hash(&self, state: &mut H) { + self.as_str().hash(state) + } +} + +impl fmt::Debug for ArcColdString { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(self.as_str(), f) + } +} + +impl fmt::Display for ArcColdString { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(self.as_str(), f) + } +} + +impl From<&str> for ArcColdString { + fn from(s: &str) -> Self { + Self::new(s) + } +} + +impl From for ArcColdString { + fn from(s: String) -> Self { + Self::new(&s) + } +} + +impl From> for ArcColdString { + fn from(s: Box) -> Self { + Self::new(&s) + } +} + +impl From for String { + fn from(s: ArcColdString) -> Self { + s.as_str().to_owned() + } +} + +impl From for Cow<'_, str> { + fn from(s: ArcColdString) -> Self { + Self::Owned(s.into()) + } +} + +impl<'a> From<&'a ArcColdString> for Cow<'a, str> { + fn from(s: &'a ArcColdString) -> Self { + Self::Borrowed(s) + } +} + +impl<'a> From> for ArcColdString { + fn from(s: Cow<'a, str>) -> Self { + Self::new(s) + } +} + +impl FromIterator for ArcColdString { + fn from_iter>(iter: I) -> Self { + Self::new(iter.into_iter().collect::()) + } +} + +impl core::borrow::Borrow for ArcColdString { + fn borrow(&self) -> &str { + self.as_str() + } +} + +impl PartialEq for ArcColdString { + fn eq(&self, other: &str) -> bool { + self.as_str() == other + } +} + +impl PartialEq for str { + fn eq(&self, other: &ArcColdString) -> bool { + other == self + } +} + +impl PartialEq<&str> for ArcColdString { + fn eq(&self, other: &&str) -> bool { + self == *other + } +} + +impl PartialEq for &str { + fn eq(&self, other: &ArcColdString) -> bool { + other == *self + } +} + +impl AsRef for ArcColdString { + fn as_ref(&self) -> &str { + self.as_str() + } +} + +impl AsRef<[u8]> for ArcColdString { + fn as_ref(&self) -> &[u8] { + self.as_bytes() + } +} + +impl Ord for ArcColdString { + fn cmp(&self, other: &Self) -> Ordering { + self.as_str().cmp(other.as_str()) + } +} + +impl PartialOrd for ArcColdString { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl str::FromStr for ArcColdString { + type Err = core::convert::Infallible; + + fn from_str(s: &str) -> Result { + Ok(Self::new(s)) + } +} + +#[cfg(feature = "serde")] +impl serde::Serialize for ArcColdString { + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_str(self.as_str()) + } +} + +#[cfg(feature = "serde")] +impl<'de> serde::Deserialize<'de> for ArcColdString { + fn deserialize>(deserializer: D) -> Result { + let s = String::deserialize(deserializer)?; + Ok(Self::new(s)) + } +} + +unsafe impl Send for ArcColdString {} +unsafe impl Sync for ArcColdString {} + +#[cfg(test)] +mod tests { + use super::*; + use core::mem::{align_of, size_of}; + + type ArcInner = crate::heap::VintStringInner; + + #[test] + fn layout() { + assert_eq!(size_of::(), size_of::()); + assert_eq!( + size_of::>(), + size_of::() + ); + assert_eq!(size_of::(), size_of::()); + assert_eq!(align_of::(), align_of::()); + } + + #[test] + fn inline_and_heap_clone() { + let inline = ArcColdString::new("short"); + let inline_clone = inline.clone(); + assert!(inline.is_inline()); + assert_eq!(inline, inline_clone); + + let heap = ArcColdString::new("a string longer than one machine word"); + let heap_clone = heap.clone(); + assert!(!heap.is_inline()); + assert_eq!(heap.encoded.addr(), heap_clone.encoded.addr()); + assert_eq!(heap.count().load(AtomicOrdering::Relaxed), 2 * REF_ONE); + drop(heap_clone); + assert_eq!(heap.count().load(AtomicOrdering::Relaxed), REF_ONE); + } + + #[test] + fn clones_across_threads() { + let value = ArcColdString::new("a shared string longer than one machine word"); + let threads: alloc::vec::Vec<_> = (0..8) + .map(|_| { + let clone = value.clone(); + std::thread::spawn(move || { + assert_eq!(clone, "a shared string longer than one machine word") + }) + }) + .collect(); + + for thread in threads { + thread.join().unwrap(); + } + assert_eq!(value.count().load(AtomicOrdering::Relaxed), REF_ONE); + } + + #[test] + fn const_inline_matches_runtime() { + const VALUE: ArcColdString = ArcColdString::new_inline_const("cold"); + assert_eq!(VALUE, ArcColdString::new("cold")); + } + + #[cfg(feature = "serde")] + #[test] + fn serde_roundtrip_shape() { + use serde_test::{assert_tokens, Token}; + + let value = ArcColdString::new("a shared string longer than one machine word"); + assert_tokens( + &value, + &[Token::Str("a shared string longer than one machine word")], + ); + } +} diff --git a/cold-string/src/encoded.rs b/cold-string/src/encoded.rs new file mode 100644 index 0000000..67f39a1 --- /dev/null +++ b/cold-string/src/encoded.rs @@ -0,0 +1,206 @@ +// On pre-1.84 compilers these calls resolve to the `sptr` polyfill imported below. +#![allow(clippy::incompatible_msrv)] + +use core::{mem, ptr, ptr::NonNull, slice}; + +#[rustversion::before(1.84)] +use sptr::Strict; + +use crate::heap::{VintStringInner, HEAP_ALIGN}; + +pub(crate) const WIDTH: usize = mem::size_of::(); +pub(crate) static EIGHT_NUL: [u8; WIDTH] = [0u8; WIDTH]; + +/// The common one-word representation used by both owning string types. +#[repr(transparent)] +pub(crate) struct Encoded { + ptr: NonNull>, +} + +impl Copy for Encoded {} + +impl Clone for Encoded { + fn clone(&self) -> Self { + *self + } +} + +impl Encoded { + const TAG_MASK: usize = usize::from_ne_bytes(0b11000000usize.to_le_bytes()); + const INLINE_TAG: usize = usize::from_ne_bytes(0b11111000usize.to_le_bytes()); + const PTR_TAG: usize = usize::from_ne_bytes(0b10000000usize.to_le_bytes()); + const LEN_MASK: usize = usize::from_ne_bytes(0b111usize.to_le_bytes()); + pub(crate) const EIGHT_NUL_MAP: usize = usize::MAX; + const ROT: u32 = if cfg!(target_endian = "little") { + 0 + } else { + 8 * (WIDTH - 1) as u32 + }; + + #[inline] + pub(crate) fn new(s: &str, header: H) -> Self { + if s.len() <= WIDTH { + Self::new_inline(s) + } else { + Self::from_heap(VintStringInner::allocate(header, s)) + } + } + + #[inline] + pub(crate) fn new_inline(s: &str) -> Self { + debug_assert!(s.len() <= WIDTH); + if s.as_bytes() == EIGHT_NUL { + return Self::new_eight_nul(); + } + let mut buf = Self::inline_buf(s); + let start = Self::utf8_start(s.len()); + buf[start..s.len() + start].copy_from_slice(s.as_bytes()); + + // SAFETY: short strings contain a non-zero inline tag, while a full-width + // all-zero string was handled above. + unsafe { Self::from_inline_buf(buf) } + } + + #[rustversion::since(1.61)] + #[inline] + pub(crate) const fn new_inline_const(s: &str) -> Self { + if s.len() > WIDTH { + panic!( + "Length for `new_inline_const` must be less than `core::mem::size_of::()`." + ); + } + let mut buf = Self::inline_buf(s); + let start = Self::utf8_start(s.len()); + let mut i = 0; + while i < s.len() { + buf[i + start] = s.as_bytes()[i]; + i += 1; + } + + if usize::from_ne_bytes(buf) == 0 { + return Self::new_eight_nul(); + } + + // SAFETY: the all-zero representation was handled above. + unsafe { Self::from_inline_buf(buf) } + } + + #[inline] + fn from_heap(ptr: NonNull>) -> Self { + let encoded = ptr.as_ptr().map_addr(|addr| { + debug_assert_eq!(addr % HEAP_ALIGN, 0); + addr.rotate_left(6 + Self::ROT) | Self::PTR_TAG + }); + + // SAFETY: the pointer tag is non-zero and `map_addr` preserves provenance. + Self { + ptr: unsafe { NonNull::new_unchecked(encoded) }, + } + } + + #[inline] + pub(crate) fn heap_ptr(&self) -> NonNull> { + debug_assert!(!self.is_inline()); + let ptr = self.ptr.as_ptr(); + let decoded = ptr.map_addr(|addr| (addr ^ Self::PTR_TAG).rotate_right(6 + Self::ROT)); + debug_assert_eq!(decoded.addr() % HEAP_ALIGN, 0); + + // SAFETY: decoding reverses `from_heap`, so the result is non-null and + // retains the allocation's provenance. + unsafe { NonNull::new_unchecked(decoded) } + } + + #[inline] + pub(crate) fn as_bytes(&self) -> &[u8] { + if self.is_inline() { + // SAFETY: inline representations contain their bytes in this word. + unsafe { self.inline_bytes() } + } else { + // SAFETY: heap representations point to a live allocation kept alive + // by the owner of this encoded word. + unsafe { VintStringInner::as_bytes(self.heap_ptr()) } + } + } + + #[inline] + pub(crate) fn len(&self) -> usize { + self.as_bytes().len() + } + + /// The caller must own the heap allocation exclusively. + #[inline] + pub(crate) unsafe fn deallocate(&self) { + debug_assert!(!self.is_inline()); + VintStringInner::deallocate(self.heap_ptr()); + } + + #[inline] + pub(crate) fn is_inline(&self) -> bool { + self.addr() & Self::TAG_MASK != Self::PTR_TAG + } + + #[inline] + fn is_eight_nul(&self) -> bool { + self.addr() == Self::EIGHT_NUL_MAP + } + + #[inline] + fn inline_len(&self) -> usize { + debug_assert!(self.is_inline()); + debug_assert!(!self.is_eight_nul()); + let addr = self.addr(); + match addr & Self::INLINE_TAG { + Self::INLINE_TAG => (addr & Self::LEN_MASK).rotate_right(Self::ROT), + _ => WIDTH, + } + } + + #[inline] + pub(crate) unsafe fn inline_bytes(&self) -> &[u8] { + debug_assert!(self.is_inline()); + if self.is_eight_nul() { + return &EIGHT_NUL; + } + let len = self.inline_len(); + let bytes = ptr::addr_of!(self.ptr).cast::(); + slice::from_raw_parts(bytes.add(Self::utf8_start(len)), len) + } + + #[inline] + pub(crate) fn addr(&self) -> usize { + self.ptr.as_ptr().addr() + } + + #[inline] + const fn inline_buf(s: &str) -> [u8; WIDTH] { + debug_assert!(s.len() <= WIDTH); + let mut buf = [0u8; WIDTH]; + if s.len() < WIDTH { + let tag = + (Self::INLINE_TAG | s.len().rotate_left(Self::ROT)).rotate_right(Self::ROT) as u8; + buf[0] = tag; + } + buf + } + + #[inline] + const fn utf8_start(len: usize) -> usize { + (len < WIDTH) as usize + } + + #[inline] + const fn new_eight_nul() -> Self { + // SAFETY: `EIGHT_NUL_MAP` is non-zero. + unsafe { Self::from_inline_buf(Self::EIGHT_NUL_MAP.to_ne_bytes()) } + } + + /// `buf` must not be all zeroes. + #[inline] + const unsafe fn from_inline_buf(buf: [u8; WIDTH]) -> Self { + let addr = usize::from_ne_bytes(buf); + let ptr = sptr::invalid_mut::>(addr); + Self { + ptr: NonNull::new_unchecked(ptr), + } + } +} diff --git a/cold-string/src/heap.rs b/cold-string/src/heap.rs new file mode 100644 index 0000000..4462f0f --- /dev/null +++ b/cold-string/src/heap.rs @@ -0,0 +1,76 @@ +use alloc::alloc::{alloc, dealloc, handle_alloc_error, Layout}; +use core::{ + cmp, + mem::{align_of, size_of}, + ptr::{self, NonNull}, + slice, +}; + +use crate::vint::VarInt; + +pub(crate) const HEAP_ALIGN: usize = 4; + +/// A heap string with an arbitrary fixed-size header followed by +/// `[vint length][UTF-8 bytes]`. +#[repr(C)] +pub(crate) struct VintStringInner { + pub(crate) header: H, + payload: [u8; 0], +} + +impl VintStringInner { + #[inline] + fn layout(len: usize, vint_len: usize) -> Layout { + let size = size_of::() + .checked_add(vint_len) + .and_then(|size| size.checked_add(len)) + .expect("capacity overflow"); + let align = cmp::max(align_of::(), HEAP_ALIGN); + Layout::from_size_align(size, align).expect("capacity overflow") + } + + #[inline] + unsafe fn payload(ptr: NonNull) -> *mut u8 { + ptr::addr_of_mut!((*ptr.as_ptr()).payload).cast() + } + + #[inline] + pub(crate) fn allocate(header: H, s: &str) -> NonNull { + let (vint_len, len_buf) = VarInt::write(s.len() as u64); + let layout = Self::layout(s.len(), vint_len); + + unsafe { + // SAFETY: `layout` has non-zero size because the vint is at least one byte. + let raw = alloc(layout).cast::(); + let ptr = match NonNull::new(raw) { + Some(ptr) => ptr, + None => handle_alloc_error(layout), + }; + + ptr::addr_of_mut!((*raw).header).write(header); + let payload = Self::payload(ptr); + ptr::copy_nonoverlapping(len_buf.as_ptr(), payload, vint_len); + ptr::copy_nonoverlapping(s.as_ptr(), payload.add(vint_len), s.len()); + ptr + } + } + + /// The caller must keep the allocation alive and immutable for `'a`. + #[inline] + pub(crate) unsafe fn as_bytes<'a>(ptr: NonNull) -> &'a [u8] { + let payload = Self::payload(ptr); + let (len, vint_len) = VarInt::read(payload); + slice::from_raw_parts(payload.add(vint_len), len) + } + + /// The caller must have exclusive ownership of this allocation. + #[inline] + pub(crate) unsafe fn deallocate(ptr: NonNull) { + let payload = Self::payload(ptr); + let (len, vint_len) = VarInt::read(payload); + let layout = Self::layout(len, vint_len); + + ptr::drop_in_place(ptr::addr_of_mut!((*ptr.as_ptr()).header)); + dealloc(ptr.as_ptr().cast(), layout); + } +} diff --git a/cold-string/src/lib.rs b/cold-string/src/lib.rs index 96bbcb2..1ba9622 100644 --- a/cold-string/src/lib.rs +++ b/cold-string/src/lib.rs @@ -5,11 +5,10 @@ extern crate alloc; -#[rustversion::before(1.84)] -use sptr::Strict; +#[cfg(test)] +extern crate std; use alloc::{ - alloc::{alloc, dealloc, Layout}, borrow::{Cow, ToOwned}, boxed::Box, str::Utf8Error, @@ -20,22 +19,24 @@ use core::{ fmt, hash::{Hash, Hasher}, iter::FromIterator, - mem, ops::Deref, - ptr, - ptr::NonNull, - slice, str, + str, }; +#[cfg(test)] +use core::{mem, ptr}; + +mod arc; +mod encoded; +mod heap; mod vint; -use crate::vint::VarInt; + +pub use crate::arc::ArcColdString; +use crate::encoded::Encoded; #[cfg(feature = "rkyv")] mod rkyv; -const HEAP_ALIGN: usize = 4; -const WIDTH: usize = mem::size_of::(); - /// Compact representation of immutable UTF-8 strings. Optimized for memory usage and struct packing. /// /// # Example @@ -57,24 +58,12 @@ pub struct ColdString { /// with the LSB bits of the tag byte. The address is always a multiple of 4 (`HEAP_ALIGN`). /// - 11111xxx: xxx is the length in range 0..=7, followed by length UTF-8 bytes. /// - xxxxxxxx (valid UTF-8): 8 UTF-8 bytes. + /// /// The exception is if `encoded` is `usize::MAX`, the UTF-8 bytes are "\0\0\0\0\0\0\0\0". - encoded: NonNull, + encoded: Encoded<()>, } -static EIGHT_NUL: [u8; WIDTH] = [0u8; WIDTH]; - impl ColdString { - const TAG_MASK: usize = usize::from_ne_bytes(0b11000000usize.to_le_bytes()); - const INLINE_TAG: usize = usize::from_ne_bytes(0b11111000usize.to_le_bytes()); - const PTR_TAG: usize = usize::from_ne_bytes(0b10000000usize.to_le_bytes()); - const LEN_MASK: usize = usize::from_ne_bytes(0b111usize.to_le_bytes()); - const EIGHT_NUL_MAP: usize = usize::MAX; - const ROT: u32 = if cfg!(target_endian = "little") { - 0 - } else { - 8 * (WIDTH - 1) as u32 - }; - /// Convert a slice of bytes into a [`ColdString`]. /// /// A [`ColdString`] is a contiguous collection of bytes (`u8`s) that is valid [`UTF-8`](https://en.wikipedia.org/wiki/UTF-8). @@ -123,6 +112,10 @@ impl ColdString { /// /// assert_eq!("💖", sparkle_heart); /// ``` + /// + /// # Safety + /// + /// `v` must contain valid UTF-8. pub unsafe fn from_utf8_unchecked>(v: B) -> Self { Self::new(str::from_utf8_unchecked(v.as_ref())) } @@ -132,65 +125,11 @@ impl ColdString { /// will be inlined on the stack. pub fn new>(x: T) -> Self { let s = x.as_ref(); - if s.len() <= WIDTH { - Self::new_inline(s) - } else { - Self::new_heap(s) + Self { + encoded: Encoded::new(s, ()), } } - #[rustversion::attr(since(1.61), const)] - #[inline] - fn new_eight_nul() -> Self { - // SAFETY: PTR_TAG is non-zero - unsafe { Self::from_inline_buf(Self::EIGHT_NUL_MAP.to_ne_bytes()) } - } - - #[inline] - fn is_eight_nul(&self) -> bool { - self.addr() == Self::EIGHT_NUL_MAP - } - - #[inline] - const fn inline_buf(s: &str) -> [u8; WIDTH] { - debug_assert!(s.len() <= WIDTH); - let mut buf = [0u8; WIDTH]; - if s.len() < WIDTH { - let tag = - (Self::INLINE_TAG | s.len().rotate_left(Self::ROT)).rotate_right(Self::ROT) as u8; - buf[0] = tag; - } - buf - } - - /// SAFETY: b must not be all-zero - #[rustversion::attr(since(1.61), const)] - #[inline] - unsafe fn from_inline_buf(b: [u8; WIDTH]) -> Self { - let encoded = ptr::null_mut::().wrapping_add(usize::from_ne_bytes(b)); - let encoded = NonNull::new_unchecked(encoded); - Self { encoded } - } - - #[inline] - const fn utf8_start(l: usize) -> usize { - (l < WIDTH) as usize - } - - #[inline] - fn new_inline(s: &str) -> Self { - if s.as_bytes() == EIGHT_NUL { - return Self::new_eight_nul(); - } - let mut buf = Self::inline_buf(s); - let start = Self::utf8_start(s.len()); - buf[start..s.len() + start].copy_from_slice(s.as_bytes()); - // SAFETY: - // it is checked at the top of the function than s is not all NUL - // and the inline tag is not 0, so shorter strings will also be not all NUL - unsafe { Self::from_inline_buf(buf) } - } - /// Creates a new inline [`ColdString`] from `&'static str` at compile time. /// /// In a dynamic context you can use the method [`ColdString::new()`]. @@ -209,102 +148,15 @@ impl ColdString { #[rustversion::since(1.61)] #[inline] pub const fn new_inline_const(s: &str) -> Self { - if s.len() > WIDTH { - panic!( - "Length for `new_inline_const` must be less than `core::mem::size_of::()`." - ); - } - if s.len() == WIDTH { - // can't do a slice comparison in const context - let bytes = unsafe { *(s.as_bytes() as *const _ as *const [u8; WIDTH]) }; - let int = usize::from_ne_bytes(bytes); - if int == 0 { - return Self::new_eight_nul(); - } + Self { + encoded: Encoded::new_inline_const(s), } - let mut buf = Self::inline_buf(s); - let start = Self::utf8_start(s.len()); - let mut i = 0; - while i < s.len() { - buf[i + start] = s.as_bytes()[i]; - i += 1; - } - // SAFETY: - // It is checked at the top of the function than s is not all NUL, - // and the inline tag is not 0, so shorter strings will also be not all NUL. - unsafe { Self::from_inline_buf(buf) } - } - - #[rustversion::attr(since(1.71), const)] - #[inline] - fn ptr(&self) -> *const u8 { - self.encoded.as_ptr() - } - - #[inline] - fn addr(&self) -> usize { - self.ptr().addr() - } - - #[inline] - fn tag(&self) -> usize { - self.addr() & Self::TAG_MASK } /// Returns `true` if the string bytes are inlined. #[inline] pub fn is_inline(&self) -> bool { - self.tag() != Self::PTR_TAG - } - - #[inline] - fn new_heap(s: &str) -> Self { - let len = s.len(); - let (vint_len, len_buf) = VarInt::write(len as u64); - let total = vint_len + len; - let layout = Layout::from_size_align(total, HEAP_ALIGN).unwrap(); - - unsafe { - // SAFETY: the layout size is non-zero, since the smallest VarInt is one byte - let ptr = alloc(layout); - if ptr.is_null() { - alloc::alloc::handle_alloc_error(layout); - } - - // TODO: can optimize this - ptr::copy_nonoverlapping(len_buf.as_ptr(), ptr, vint_len); - ptr::copy_nonoverlapping(s.as_ptr(), ptr.add(vint_len), len); - let encoded = ptr.map_addr(|addr| { - debug_assert!(addr % HEAP_ALIGN == 0); - let mut addr = addr.rotate_left(6 + Self::ROT); - addr |= Self::PTR_TAG; - addr - }); - // SAFETY: encoded != 0 because Self::PTR_TAG != 0 - let encoded = NonNull::new_unchecked(encoded); - Self { encoded } - } - } - - #[inline] - fn heap_ptr(&self) -> *const u8 { - debug_assert!(!self.is_inline()); - self.ptr().map_addr(|mut addr| { - addr ^= Self::PTR_TAG; - let addr = addr.rotate_right(6 + Self::ROT); - debug_assert!(addr % HEAP_ALIGN == 0); - addr - }) - } - - #[inline] - fn inline_len(&self) -> usize { - debug_assert!(!self.is_eight_nul()); - let addr = self.addr(); - match addr & Self::INLINE_TAG { - Self::INLINE_TAG => (addr & Self::LEN_MASK).rotate_right(Self::ROT), - _ => WIDTH, - } + self.encoded.is_inline() } /// Returns the length of this `ColdString`, in bytes, not [`char`]s or @@ -325,39 +177,7 @@ impl ColdString { /// ``` #[inline] pub fn len(&self) -> usize { - if self.is_eight_nul() { - return WIDTH; - } else if self.is_inline() { - self.inline_len() - } else { - unsafe { - let ptr = self.heap_ptr(); - let (len, _) = VarInt::read(ptr); - len as usize - } - } - } - - #[allow(unsafe_op_in_unsafe_fn)] - #[inline] - unsafe fn decode_inline(&self) -> &[u8] { - if self.is_eight_nul() { - return &EIGHT_NUL; - } - let len = self.inline_len(); - // SAFETY: addr_of! avoids &self.ptr (which is UB due to alignment) - let self_bytes_ptr = ptr::addr_of!(self.encoded) as *const u8; - let start = Self::utf8_start(len); - slice::from_raw_parts(self_bytes_ptr.add(start), len) - } - - #[allow(unsafe_op_in_unsafe_fn)] - #[inline] - unsafe fn decode_heap(&self) -> &[u8] { - let ptr = self.heap_ptr(); - let (len, header) = VarInt::read(ptr); - let data = ptr.add(header); - slice::from_raw_parts(data, len) + self.encoded.len() } /// Returns a byte slice of this `ColdString`'s contents. @@ -375,10 +195,7 @@ impl ColdString { /// ``` #[inline] pub fn as_bytes(&self) -> &[u8] { - match self.is_inline() { - true => unsafe { self.decode_inline() }, - false => unsafe { self.decode_heap() }, - } + self.encoded.as_bytes() } /// Returns a string slice containing the entire [`ColdString`]. @@ -410,7 +227,7 @@ impl ColdString { impl Default for ColdString { fn default() -> Self { - Self::new_inline("") + Self::new("") } } @@ -424,14 +241,8 @@ impl Deref for ColdString { impl Drop for ColdString { fn drop(&mut self) { if !self.is_inline() { - let ptr = self.heap_ptr(); - unsafe { - let (len, header) = VarInt::read(ptr); - let total = header + len; - let layout = Layout::from_size_align(total, HEAP_ALIGN).unwrap(); - // SAFETY: if ptr is non-null then it was allocated by alloc() in new_heap() - dealloc(ptr as *mut u8, layout); - } + // SAFETY: a non-inline `ColdString` uniquely owns its allocation. + unsafe { self.encoded.deallocate() } } } } @@ -439,22 +250,18 @@ impl Drop for ColdString { impl Clone for ColdString { fn clone(&self) -> Self { if self.is_inline() { - let ptr = self.ptr(); - let encoded = unsafe { NonNull::new_unchecked(ptr as *mut _) }; - Self { encoded } + Self { + encoded: self.encoded, + } } else { - Self::new_heap(self.as_str()) + Self::new(self.as_str()) } } } impl PartialEq for ColdString { fn eq(&self, other: &Self) -> bool { - match (self.is_inline(), other.is_inline()) { - (true, true) => self.ptr() == other.ptr(), - (false, false) => unsafe { self.decode_heap() == other.decode_heap() }, - _ => false, - } + self.encoded.addr() == other.encoded.addr() || self.as_bytes() == other.as_bytes() } } @@ -512,10 +319,7 @@ impl<'a> From<&'a ColdString> for Cow<'a, str> { impl<'a> From> for ColdString { fn from(cow: Cow<'a, str>) -> Self { - match cow { - Cow::Borrowed(s) => s.into(), - Cow::Owned(s) => s.into(), - } + Self::new(cow) } } @@ -529,8 +333,7 @@ impl From> for ColdString { impl FromIterator for ColdString { fn from_iter>(iter: I) -> Self { - let s: String = iter.into_iter().collect(); - ColdString::new(&s) + Self::new(iter.into_iter().collect::()) } } @@ -545,11 +348,7 @@ impl core::borrow::Borrow for ColdString { impl PartialEq for ColdString { fn eq(&self, other: &str) -> bool { - if self.is_inline() { - unsafe { self.decode_inline() == other.as_bytes() } - } else { - unsafe { self.decode_heap() == other.as_bytes() } - } + self.as_str() == other } } @@ -593,7 +392,7 @@ impl Ord for ColdString { impl PartialOrd for ColdString { fn partial_cmp(&self, other: &Self) -> Option { - self.as_str().partial_cmp(other.as_str()) + Some(self.cmp(other)) } } @@ -676,7 +475,7 @@ mod tests { assert_eq!(*s, cs); let opt_s = Some(cs.clone()); assert_eq!(opt_s, Some(ColdString::new(s))); - assert!(opt_s != None); + assert!(opt_s.is_some()); } #[test] @@ -781,21 +580,21 @@ mod tests { #[test] fn ensure_zero_repr() { - assert!(str::from_utf8(&ColdString::EIGHT_NUL_MAP.to_ne_bytes()).is_err()); + assert!(str::from_utf8(&Encoded::<()>::EIGHT_NUL_MAP.to_ne_bytes()).is_err()); } #[test] fn test_const_8nul_vs_non_const() { - let nul8 = str::from_utf8(&EIGHT_NUL).unwrap(); + let nul8 = str::from_utf8(&encoded::EIGHT_NUL).unwrap(); let const8 = ColdString::new_inline_const(nul8); let non_const = ColdString::new(nul8); let cloned = non_const.clone(); - assert_eq!(const8.ptr(), non_const.ptr()); - assert_eq!(const8.ptr(), cloned.ptr()); + assert_eq!(const8.encoded.addr(), non_const.encoded.addr()); + assert_eq!(const8.encoded.addr(), cloned.encoded.addr()); // check that a null pointer will return a str pointing to EIGHT_NUL assert_eq!( &const8.as_str().as_bytes()[0] as *const u8, - (&EIGHT_NUL) as *const u8 + (&encoded::EIGHT_NUL) as *const u8 ); } } diff --git a/cold-string/src/vint.rs b/cold-string/src/vint.rs index ac35a39..dcdc091 100644 --- a/cold-string/src/vint.rs +++ b/cold-string/src/vint.rs @@ -44,7 +44,7 @@ mod tests { fn assert_correct(x: u64) { let (wrote, b) = VarInt::write(x); - assert!(wrote >= 1 && wrote <= 10); + assert!((1..=10).contains(&wrote)); let ptr = b.as_ptr(); let (y, read) = unsafe { VarInt::read(ptr) }; assert_eq!(wrote, read); diff --git a/cold-string/tests/property.rs b/cold-string/tests/property.rs index 7d0ed56..23a8b51 100644 --- a/cold-string/tests/property.rs +++ b/cold-string/tests/property.rs @@ -48,4 +48,23 @@ proptest! { assert_eq!(opt_s.as_ref().map(|x| x.as_str()), Some(s.as_str())); } + #[test] + fn arb_arc_string(s in any::()) { + let cold = ArcColdString::new(s.as_str()); + assert_eq!(s.len() <= core::mem::size_of::(), cold.is_inline()); + assert_eq!(cold.len(), s.len()); + assert_eq!(cold.as_bytes(), s.as_bytes()); + assert_eq!(cold.as_str(), s.as_str()); + assert_eq!(cold, cold.clone()); + assert_eq!(cold, s.as_str()); + assert_eq!(s.as_str(), cold); + assert_eq!(unsafe { ArcColdString::from_utf8_unchecked(s.as_bytes()).as_bytes() }, s.as_bytes()); + if s.len() <= core::mem::size_of::() { + assert_eq!(ArcColdString::new_inline_const(&s), cold); + } + let opt_s = Some(cold.clone()); + assert_eq!(opt_s.as_ref().unwrap(), &cold); + assert_eq!(opt_s.as_ref().map(|x| x.as_str()), Some(s.as_str())); + } + } From fd6b30b3a4f3d3e63cc09c82a4f7215922cd3d67 Mon Sep 17 00:00:00 2001 From: Thomas Pendock Date: Fri, 18 Sep 2026 16:26:16 -0700 Subject: [PATCH 2/6] loom test --- .github/workflows/rust.yml | 4 + cold-string/Cargo.lock | 216 +++++++++++++++++++++++++++++++++++++ cold-string/Cargo.toml | 1 + cold-string/src/arc.rs | 35 +++++- cold-string/src/lib.rs | 1 + 5 files changed, 256 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 1f28e1c..167bc7f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -26,6 +26,10 @@ jobs: run: cargo +1.74 hack test --manifest-path cold-string/Cargo.toml --verbose --feature-powerset --exclude-features rkyv - name: Tests run: cargo hack test --manifest-path cold-string/Cargo.toml --verbose --feature-powerset + - name: Loom + env: + RUSTFLAGS: --cfg loom + run: cargo test --manifest-path cold-string/Cargo.toml --lib loom_clone_drop - name: Install nightly + Miri run: | rustup toolchain install nightly diff --git a/cold-string/Cargo.lock b/cold-string/Cargo.lock index d0e401a..4813fff 100644 --- a/cold-string/Cargo.lock +++ b/cold-string/Cargo.lock @@ -13,6 +13,15 @@ dependencies = [ "version_check", ] +[[package]] +name = "aho-corasick" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c982642fa9e8606056828ee9a8505737230110bb1099153c79efe865c59d12ba" +dependencies = [ + "memchr", +] + [[package]] name = "autocfg" version = "1.5.0" @@ -63,6 +72,16 @@ dependencies = [ "syn", ] +[[package]] +name = "cc" +version = "1.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54413ede23c2daf518f35156dfde027feb2374004d63bd497f983c8db9c0e313" +dependencies = [ + "find-msvc-tools", + "shlex", +] + [[package]] name = "cfg-if" version = "1.0.4" @@ -74,6 +93,7 @@ name = "cold-string" version = "0.3.0" dependencies = [ "hashbrown 0.12.3", + "loom", "proptest", "rkyv", "rustversion", @@ -82,6 +102,27 @@ dependencies = [ "sptr", ] +[[package]] +name = "find-msvc-tools" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef25905e51abafe4dcea6c15fec58c57b601cdbd0ee53d22ea1d3016c587d39b" + +[[package]] +name = "generator" +version = "0.8.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54ade96dc9003043bce7c035c85a9df5a858bfb2039c5a2e6fdf00f324f6c551" +dependencies = [ + "cc", + "cfg-if", + "libc", + "log", + "rustversion", + "windows-link", + "windows-result", +] + [[package]] name = "getrandom" version = "0.2.17" @@ -132,6 +173,40 @@ version = "0.2.182" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6800badb6cb2082ffd7b6a67e6125bb39f18782f793520caee8cb8846be06112" +[[package]] +name = "log" +version = "0.4.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f9f8bd3e56ce4dfc153cf470fffbfa98c7620958b312ca5c3a4b8d5181fd13c6" + +[[package]] +name = "loom" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "419e0dc8046cb947daa77eb95ae174acfbddb7673b4151f56d1eed8e93fbfaca" +dependencies = [ + "cfg-if", + "generator", + "scoped-tls", + "tracing", + "tracing-subscriber", +] + +[[package]] +name = "matchers" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d1525a2a28c7f4fa0fc98bb91ae755d1e2d1505079e05539e35bc876b5d65ae9" +dependencies = [ + "regex-automata", +] + +[[package]] +name = "memchr" +version = "2.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" + [[package]] name = "munge" version = "0.4.7" @@ -152,6 +227,15 @@ dependencies = [ "syn", ] +[[package]] +name = "nu-ansi-term" +version = "0.50.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" +dependencies = [ + "windows-sys", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -167,6 +251,12 @@ version = "1.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" +[[package]] +name = "pin-project-lite" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" + [[package]] name = "ppv-lite86" version = "0.2.21" @@ -285,6 +375,17 @@ dependencies = [ "rand_core", ] +[[package]] +name = "regex-automata" +version = "0.4.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ad8553b9b26413251cbf30e620595c7a41b3887f03da04579c0e6b0d6a06b4b2" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + [[package]] name = "regex-syntax" version = "0.8.9" @@ -333,6 +434,12 @@ version = "1.0.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" +[[package]] +name = "scoped-tls" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1cf6437eb19a8f4a6cc0f7dca544973b0b78843adbfeb3683d1a94a0024a294" + [[package]] name = "serde" version = "1.0.228" @@ -371,12 +478,33 @@ dependencies = [ "serde", ] +[[package]] +name = "sharded-slab" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6" +dependencies = [ + "lazy_static", +] + +[[package]] +name = "shlex" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" + [[package]] name = "simdutf8" version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e3a9fe34e3e7a50316060351f37187a3f546bce95496156754b601a5fa71b76e" +[[package]] +name = "smallvec" +version = "1.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba467056f1b547ed52077911161fc86985becbc60e8e1857c8a144dab0def891" + [[package]] name = "sptr" version = "0.3.2" @@ -394,6 +522,15 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "thread_local" +version = "1.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ad99c4c6d32803332c548b1af0540b357b3f5fc0be8f6c6bfe8b2e6ae784070" +dependencies = [ + "cfg-if", +] + [[package]] name = "tinyvec" version = "1.10.0" @@ -409,6 +546,55 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" +[[package]] +name = "tracing" +version = "0.1.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100" +dependencies = [ + "pin-project-lite", + "tracing-core", +] + +[[package]] +name = "tracing-core" +version = "0.1.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db97caf9d906fbde555dd62fa95ddba9eecfd14cb388e4f491a66d74cd5fb79a" +dependencies = [ + "once_cell", + "valuable", +] + +[[package]] +name = "tracing-log" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee855f1f400bd0e5c02d150ae5de3840039a3f54b025156404e34c23c03f47c3" +dependencies = [ + "log", + "once_cell", + "tracing-core", +] + +[[package]] +name = "tracing-subscriber" +version = "0.3.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb7f578e5945fb242538965c2d0b04418d38ec25c79d160cd279bf0731c8d319" +dependencies = [ + "matchers", + "nu-ansi-term", + "once_cell", + "regex-automata", + "sharded-slab", + "smallvec", + "thread_local", + "tracing", + "tracing-core", + "tracing-log", +] + [[package]] name = "unarray" version = "0.1.4" @@ -421,6 +607,12 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +[[package]] +name = "valuable" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" + [[package]] name = "version_check" version = "0.9.5" @@ -442,6 +634,30 @@ dependencies = [ "wit-bindgen", ] +[[package]] +name = "windows-link" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" + +[[package]] +name = "windows-result" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" +dependencies = [ + "windows-link", +] + +[[package]] +name = "windows-sys" +version = "0.61.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" +dependencies = [ + "windows-link", +] + [[package]] name = "wit-bindgen" version = "0.51.0" diff --git a/cold-string/Cargo.toml b/cold-string/Cargo.toml index 7c02b67..1a9218a 100644 --- a/cold-string/Cargo.toml +++ b/cold-string/Cargo.toml @@ -28,5 +28,6 @@ rustversion = "1.0.22" [dev-dependencies] hashbrown = "0.12.3" +loom = "0.7.2" serde_test = "1.0.177" proptest = {version = "=1.8.0", default-features = false, features = ["std", "bit-set"] } diff --git a/cold-string/src/arc.rs b/cold-string/src/arc.rs index c031254..40d758f 100644 --- a/cold-string/src/arc.rs +++ b/cold-string/src/arc.rs @@ -11,9 +11,13 @@ use core::{ iter::FromIterator, ops::Deref, str, - sync::atomic::{fence, AtomicUsize, Ordering as AtomicOrdering}, }; +#[cfg(not(all(loom, test)))] +use core::sync::atomic::{fence, AtomicUsize, Ordering as AtomicOrdering}; +#[cfg(all(loom, test))] +use loom::sync::atomic::{fence, AtomicUsize, Ordering as AtomicOrdering}; + use crate::encoded::Encoded; const IMMORTAL: usize = 1; @@ -361,6 +365,35 @@ mod tests { assert_eq!(value.count().load(AtomicOrdering::Relaxed), REF_ONE); } + #[cfg(loom)] + #[test] + fn loom_clone_drop() { + loom::model(|| { + const TEXT: &str = "a shared string longer than one machine word"; + + let value = ArcColdString::new(TEXT); + let left = value.clone(); + let right = value.clone(); + + let left = loom::thread::spawn(move || { + let clone = left.clone(); + assert_eq!(clone.as_str(), TEXT); + drop(clone); + drop(left); + }); + let right = loom::thread::spawn(move || { + let clone = right.clone(); + assert_eq!(clone.as_str(), TEXT); + drop(right); + drop(clone); + }); + + left.join().unwrap(); + right.join().unwrap(); + assert_eq!(value.count().load(AtomicOrdering::Relaxed), REF_ONE); + }); + } + #[test] fn const_inline_matches_runtime() { const VALUE: ArcColdString = ArcColdString::new_inline_const("cold"); diff --git a/cold-string/src/lib.rs b/cold-string/src/lib.rs index 1ba9622..6dad4e4 100644 --- a/cold-string/src/lib.rs +++ b/cold-string/src/lib.rs @@ -1,5 +1,6 @@ #![allow(rustdoc::bare_urls)] #![doc = include_str!("../README.md")] +#![allow(unknown_lints, unexpected_cfgs)] #![allow(unstable_name_collisions)] #![no_std] From 48e7a102a5a58f9ed54fbdf2c44af0a628592d05 Mon Sep 17 00:00:00 2001 From: Thomas Pendock Date: Fri, 18 Sep 2026 17:08:47 -0700 Subject: [PATCH 3/6] different sizes ref counts --- Cargo.lock | 7 + cold-string/Cargo.lock | 7 + cold-string/Cargo.toml | 1 + cold-string/README.md | 4 + cold-string/src/arc.rs | 319 ++++++++++++++++++++++++---------- cold-string/src/lib.rs | 165 +++++++++++++++--- cold-string/tests/property.rs | 2 +- 7 files changed, 389 insertions(+), 116 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7237bbb..4fdb939 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -162,6 +162,7 @@ checksum = "3a822ea5bc7590f9d40f1ba12c0dc3c2760f3482c6984db1573ad11031420831" name = "cold-string" version = "0.3.0" dependencies = [ + "portable-atomic", "rustversion", "sptr", ] @@ -423,6 +424,12 @@ dependencies = [ "plotters-backend", ] +[[package]] +name = "portable-atomic" +version = "1.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05c8b63e8d9609db387f0324918f81d68fe27748f084ef092fb35954d0539a85" + [[package]] name = "proc-macro2" version = "1.0.106" diff --git a/cold-string/Cargo.lock b/cold-string/Cargo.lock index 4813fff..6c4ff6c 100644 --- a/cold-string/Cargo.lock +++ b/cold-string/Cargo.lock @@ -94,6 +94,7 @@ version = "0.3.0" dependencies = [ "hashbrown 0.12.3", "loom", + "portable-atomic", "proptest", "rkyv", "rustversion", @@ -257,6 +258,12 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" +[[package]] +name = "portable-atomic" +version = "1.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05c8b63e8d9609db387f0324918f81d68fe27748f084ef092fb35954d0539a85" + [[package]] name = "ppv-lite86" version = "0.2.21" diff --git a/cold-string/Cargo.toml b/cold-string/Cargo.toml index 1a9218a..3f0a884 100644 --- a/cold-string/Cargo.toml +++ b/cold-string/Cargo.toml @@ -23,6 +23,7 @@ rkyv = ["dep:rkyv", "rkyv/alloc", "rkyv/bytecheck"] [dependencies] serde = { version = "1.0.228", optional = true, default-features = false } rkyv = { version = "0.8.15", optional = true, default-features = false } +portable-atomic = { version = "1.15.0", default-features = false, features = ["require-cas"] } sptr = { version = "0.3.2", default-features = false } rustversion = "1.0.22" diff --git a/cold-string/README.md b/cold-string/README.md index 1a90a8e..6ba85c8 100644 --- a/cold-string/README.md +++ b/cold-string/README.md @@ -43,6 +43,10 @@ let second = first.clone(); assert_eq!(first, second); ``` +`ArcColdString` uses a native-width reference count. `ArcColdString8`, +`ArcColdString16`, and `ArcColdString32` use 1-, 2-, and 4-byte counts when a +smaller heap header is preferable. All variants remain one word in size. + Both types keep strings up to one machine word inline. For longer strings, `ArcColdString` stores `[atomic reference count][variable-length length][UTF-8 bytes]` in one allocation and does not support weak references. diff --git a/cold-string/src/arc.rs b/cold-string/src/arc.rs index 40d758f..2d563fc 100644 --- a/cold-string/src/arc.rs +++ b/cold-string/src/arc.rs @@ -13,16 +13,102 @@ use core::{ str, }; -#[cfg(not(all(loom, test)))] -use core::sync::atomic::{fence, AtomicUsize, Ordering as AtomicOrdering}; #[cfg(all(loom, test))] -use loom::sync::atomic::{fence, AtomicUsize, Ordering as AtomicOrdering}; +use loom::sync::atomic::{ + fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering as AtomicOrdering, +}; +#[cfg(not(all(loom, test)))] +use portable_atomic::{ + fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering as AtomicOrdering, +}; use crate::encoded::Encoded; -const IMMORTAL: usize = 1; -const REF_ONE: usize = 2; -const MAX_REFCOUNT: usize = usize::MAX / 4; +#[doc(hidden)] +pub trait RefCount: Send + Sync + 'static { + fn new() -> Self; + fn increment(&self); + fn decrement(&self) -> bool; + + #[cfg(test)] + fn refs(&self) -> usize; + + #[cfg(all(test, not(loom)))] + fn near_overflow() -> Self; + + #[cfg(all(test, not(loom)))] + fn is_immortal(&self) -> bool; +} + +macro_rules! impl_ref_count { + ($atomic:ty, $int:ty) => { + impl RefCount for $atomic { + #[inline] + fn new() -> Self { + <$atomic>::new(2) + } + + #[inline] + fn increment(&self) { + let mut old = self.load(AtomicOrdering::Relaxed); + loop { + if old & 1 != 0 { + return; + } + + let new = if old == <$int>::MAX - 1 { + <$int>::MAX + } else { + old + 2 + }; + match self.compare_exchange_weak( + old, + new, + AtomicOrdering::Relaxed, + AtomicOrdering::Relaxed, + ) { + Ok(_) => return, + Err(actual) => old = actual, + } + } + } + + #[inline] + fn decrement(&self) -> bool { + if self.load(AtomicOrdering::Relaxed) & 1 != 0 { + return false; + } + self.fetch_sub(2, AtomicOrdering::Release) == 2 + } + + #[cfg(test)] + fn refs(&self) -> usize { + (self.load(AtomicOrdering::Relaxed) >> 1) as usize + } + + #[cfg(all(test, not(loom)))] + fn near_overflow() -> Self { + Self::new(<$int>::MAX - 1) + } + + #[cfg(all(test, not(loom)))] + fn is_immortal(&self) -> bool { + self.load(AtomicOrdering::Relaxed) & 1 != 0 + } + } + }; +} + +impl_ref_count!(AtomicU8, u8); +impl_ref_count!(AtomicU16, u16); +impl_ref_count!(AtomicU32, u32); +impl_ref_count!(AtomicUsize, usize); + +#[doc(hidden)] +#[repr(transparent)] +pub struct ArcColdStringInner { + encoded: Encoded, +} /// A one-word, atomically reference-counted immutable UTF-8 string. /// @@ -36,12 +122,18 @@ const MAX_REFCOUNT: usize = usize::MAX / 4; /// let second = first.clone(); /// assert_eq!(first, second); /// ``` -#[repr(transparent)] -pub struct ArcColdString { - encoded: Encoded, -} +pub type ArcColdString = ArcColdStringInner; -impl ArcColdString { +/// An [`ArcColdString`] with an 8-bit reference count. +pub type ArcColdString8 = ArcColdStringInner; + +/// An [`ArcColdString`] with a 16-bit reference count. +pub type ArcColdString16 = ArcColdStringInner; + +/// An [`ArcColdString`] with a 32-bit reference count. +pub type ArcColdString32 = ArcColdStringInner; + +impl ArcColdStringInner { pub fn from_utf8>(bytes: B) -> Result { Ok(Self::new(str::from_utf8(bytes.as_ref())?)) } @@ -56,7 +148,7 @@ impl ArcColdString { pub fn new>(value: T) -> Self { let s = value.as_ref(); Self { - encoded: Encoded::new(s, AtomicUsize::new(REF_ONE)), + encoded: Encoded::new(s, A::new()), } } @@ -69,11 +161,16 @@ impl ArcColdString { } #[inline] - fn count(&self) -> &AtomicUsize { + fn count(&self) -> &A { debug_assert!(!self.is_inline()); unsafe { &(*self.encoded.heap_ptr().as_ptr()).header } } + #[cfg(test)] + pub(crate) fn encoded_addr(&self) -> usize { + self.encoded.addr() + } + #[inline] pub fn is_inline(&self) -> bool { self.encoded.is_inline() @@ -101,19 +198,11 @@ impl ArcColdString { } } -impl Clone for ArcColdString { +impl Clone for ArcColdStringInner { #[inline] fn clone(&self) -> Self { if !self.is_inline() { - let count = self.count(); - if count.load(AtomicOrdering::Relaxed) & IMMORTAL == 0 { - let old = count.fetch_add(REF_ONE, AtomicOrdering::Relaxed); - if old >> 1 >= MAX_REFCOUNT { - // Permanently leak the allocation instead of allowing the - // count to wrap. The spare range covers racing increments. - count.fetch_or(IMMORTAL, AtomicOrdering::Relaxed); - } - } + self.count().increment(); } Self { @@ -122,20 +211,14 @@ impl Clone for ArcColdString { } } -impl Drop for ArcColdString { +impl Drop for ArcColdStringInner { #[inline] fn drop(&mut self) { if self.is_inline() { return; } - let count = self.count(); - if count.load(AtomicOrdering::Relaxed) & IMMORTAL != 0 { - return; - } - - let old = count.fetch_sub(REF_ONE, AtomicOrdering::Release); - if old & IMMORTAL != 0 || old != REF_ONE { + if !self.count().decrement() { return; } @@ -145,13 +228,13 @@ impl Drop for ArcColdString { } } -impl Default for ArcColdString { +impl Default for ArcColdStringInner { fn default() -> Self { Self::new("") } } -impl Deref for ArcColdString { +impl Deref for ArcColdStringInner { type Target = str; fn deref(&self) -> &str { @@ -159,135 +242,135 @@ impl Deref for ArcColdString { } } -impl PartialEq for ArcColdString { +impl PartialEq for ArcColdStringInner { fn eq(&self, other: &Self) -> bool { self.encoded.addr() == other.encoded.addr() || self.as_bytes() == other.as_bytes() } } -impl Eq for ArcColdString {} +impl Eq for ArcColdStringInner {} -impl Hash for ArcColdString { +impl Hash for ArcColdStringInner { fn hash(&self, state: &mut H) { self.as_str().hash(state) } } -impl fmt::Debug for ArcColdString { +impl fmt::Debug for ArcColdStringInner { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(self.as_str(), f) } } -impl fmt::Display for ArcColdString { +impl fmt::Display for ArcColdStringInner { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(self.as_str(), f) } } -impl From<&str> for ArcColdString { +impl From<&str> for ArcColdStringInner { fn from(s: &str) -> Self { Self::new(s) } } -impl From for ArcColdString { +impl From for ArcColdStringInner { fn from(s: String) -> Self { Self::new(&s) } } -impl From> for ArcColdString { +impl From> for ArcColdStringInner { fn from(s: Box) -> Self { Self::new(&s) } } -impl From for String { - fn from(s: ArcColdString) -> Self { +impl From> for String { + fn from(s: ArcColdStringInner) -> Self { s.as_str().to_owned() } } -impl From for Cow<'_, str> { - fn from(s: ArcColdString) -> Self { +impl From> for Cow<'_, str> { + fn from(s: ArcColdStringInner) -> Self { Self::Owned(s.into()) } } -impl<'a> From<&'a ArcColdString> for Cow<'a, str> { - fn from(s: &'a ArcColdString) -> Self { +impl<'a, A: RefCount> From<&'a ArcColdStringInner> for Cow<'a, str> { + fn from(s: &'a ArcColdStringInner) -> Self { Self::Borrowed(s) } } -impl<'a> From> for ArcColdString { +impl<'a, A: RefCount> From> for ArcColdStringInner { fn from(s: Cow<'a, str>) -> Self { Self::new(s) } } -impl FromIterator for ArcColdString { +impl FromIterator for ArcColdStringInner { fn from_iter>(iter: I) -> Self { Self::new(iter.into_iter().collect::()) } } -impl core::borrow::Borrow for ArcColdString { +impl core::borrow::Borrow for ArcColdStringInner { fn borrow(&self) -> &str { self.as_str() } } -impl PartialEq for ArcColdString { +impl PartialEq for ArcColdStringInner { fn eq(&self, other: &str) -> bool { self.as_str() == other } } -impl PartialEq for str { - fn eq(&self, other: &ArcColdString) -> bool { +impl PartialEq> for str { + fn eq(&self, other: &ArcColdStringInner) -> bool { other == self } } -impl PartialEq<&str> for ArcColdString { +impl PartialEq<&str> for ArcColdStringInner { fn eq(&self, other: &&str) -> bool { self == *other } } -impl PartialEq for &str { - fn eq(&self, other: &ArcColdString) -> bool { +impl PartialEq> for &str { + fn eq(&self, other: &ArcColdStringInner) -> bool { other == *self } } -impl AsRef for ArcColdString { +impl AsRef for ArcColdStringInner { fn as_ref(&self) -> &str { self.as_str() } } -impl AsRef<[u8]> for ArcColdString { +impl AsRef<[u8]> for ArcColdStringInner { fn as_ref(&self) -> &[u8] { self.as_bytes() } } -impl Ord for ArcColdString { +impl Ord for ArcColdStringInner { fn cmp(&self, other: &Self) -> Ordering { self.as_str().cmp(other.as_str()) } } -impl PartialOrd for ArcColdString { +impl PartialOrd for ArcColdStringInner { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } -impl str::FromStr for ArcColdString { +impl str::FromStr for ArcColdStringInner { type Err = core::convert::Infallible; fn from_str(s: &str) -> Result { @@ -296,60 +379,77 @@ impl str::FromStr for ArcColdString { } #[cfg(feature = "serde")] -impl serde::Serialize for ArcColdString { +impl serde::Serialize for ArcColdStringInner { fn serialize(&self, serializer: S) -> Result { serializer.serialize_str(self.as_str()) } } #[cfg(feature = "serde")] -impl<'de> serde::Deserialize<'de> for ArcColdString { +impl<'de, A: RefCount> serde::Deserialize<'de> for ArcColdStringInner { fn deserialize>(deserializer: D) -> Result { let s = String::deserialize(deserializer)?; Ok(Self::new(s)) } } -unsafe impl Send for ArcColdString {} -unsafe impl Sync for ArcColdString {} +unsafe impl Send for ArcColdStringInner {} +unsafe impl Sync for ArcColdStringInner {} #[cfg(test)] mod tests { use super::*; use core::mem::{align_of, size_of}; - type ArcInner = crate::heap::VintStringInner; + macro_rules! each_ref_count { + ($test:ident) => { + $test::(); + $test::(); + $test::(); + $test::(); + }; + } - #[test] - fn layout() { - assert_eq!(size_of::(), size_of::()); + fn assert_layout() { + assert_eq!(size_of::>(), size_of::()); + assert_eq!( + size_of::>>(), + size_of::>() + ); + assert_eq!(size_of::>(), size_of::()); assert_eq!( - size_of::>(), - size_of::() + align_of::>(), + align_of::() ); - assert_eq!(size_of::(), size_of::()); - assert_eq!(align_of::(), align_of::()); } #[test] - fn inline_and_heap_clone() { - let inline = ArcColdString::new("short"); + fn layout() { + each_ref_count!(assert_layout); + } + + fn assert_inline_and_heap_clone() { + let inline = ArcColdStringInner::::new("short"); let inline_clone = inline.clone(); assert!(inline.is_inline()); assert_eq!(inline, inline_clone); - let heap = ArcColdString::new("a string longer than one machine word"); + let heap = ArcColdStringInner::::new("a string longer than one machine word"); let heap_clone = heap.clone(); assert!(!heap.is_inline()); assert_eq!(heap.encoded.addr(), heap_clone.encoded.addr()); - assert_eq!(heap.count().load(AtomicOrdering::Relaxed), 2 * REF_ONE); + assert_eq!(heap.count().refs(), 2); drop(heap_clone); - assert_eq!(heap.count().load(AtomicOrdering::Relaxed), REF_ONE); + assert_eq!(heap.count().refs(), 1); } #[test] - fn clones_across_threads() { - let value = ArcColdString::new("a shared string longer than one machine word"); + fn inline_and_heap_clone() { + each_ref_count!(assert_inline_and_heap_clone); + } + + fn assert_clones_across_threads() { + let value = ArcColdStringInner::::new("a shared string longer than one machine word"); let threads: alloc::vec::Vec<_> = (0..8) .map(|_| { let clone = value.clone(); @@ -362,16 +462,20 @@ mod tests { for thread in threads { thread.join().unwrap(); } - assert_eq!(value.count().load(AtomicOrdering::Relaxed), REF_ONE); + assert_eq!(value.count().refs(), 1); } - #[cfg(loom)] #[test] - fn loom_clone_drop() { + fn clones_across_threads() { + each_ref_count!(assert_clones_across_threads); + } + + #[cfg(loom)] + fn model_clone_drop() { loom::model(|| { const TEXT: &str = "a shared string longer than one machine word"; - let value = ArcColdString::new(TEXT); + let value = ArcColdStringInner::::new(TEXT); let left = value.clone(); let right = value.clone(); @@ -390,25 +494,60 @@ mod tests { left.join().unwrap(); right.join().unwrap(); - assert_eq!(value.count().load(AtomicOrdering::Relaxed), REF_ONE); + assert_eq!(value.count().refs(), 1); }); } + #[cfg(loom)] + #[test] + fn loom_clone_drop() { + each_ref_count!(model_clone_drop); + } + + #[cfg(not(loom))] + fn assert_count_becomes_immortal() { + let count = A::near_overflow(); + count.increment(); + assert!(count.is_immortal()); + assert!(!count.decrement()); + } + + #[cfg(not(loom))] + #[test] + fn count_becomes_immortal_without_wrapping() { + each_ref_count!(assert_count_becomes_immortal); + } + #[test] fn const_inline_matches_runtime() { - const VALUE: ArcColdString = ArcColdString::new_inline_const("cold"); - assert_eq!(VALUE, ArcColdString::new("cold")); + macro_rules! assert_const_inline { + ($atomic:ty) => {{ + const VALUE: ArcColdStringInner<$atomic> = + ArcColdStringInner::<$atomic>::new_inline_const("cold"); + assert_eq!(VALUE, ArcColdStringInner::<$atomic>::new("cold")); + }}; + } + + assert_const_inline!(AtomicU8); + assert_const_inline!(AtomicU16); + assert_const_inline!(AtomicU32); + assert_const_inline!(AtomicUsize); } #[cfg(feature = "serde")] - #[test] - fn serde_roundtrip_shape() { + fn assert_serde_roundtrip() { use serde_test::{assert_tokens, Token}; - let value = ArcColdString::new("a shared string longer than one machine word"); + let value = ArcColdStringInner::::new("a shared string longer than one machine word"); assert_tokens( &value, &[Token::Str("a shared string longer than one machine word")], ); } + + #[cfg(feature = "serde")] + #[test] + fn serde_roundtrip_shape() { + each_ref_count!(assert_serde_roundtrip); + } } diff --git a/cold-string/src/lib.rs b/cold-string/src/lib.rs index 6dad4e4..ba4be3e 100644 --- a/cold-string/src/lib.rs +++ b/cold-string/src/lib.rs @@ -33,6 +33,9 @@ mod heap; mod vint; pub use crate::arc::ArcColdString; +pub use crate::arc::ArcColdString16; +pub use crate::arc::ArcColdString32; +pub use crate::arc::ArcColdString8; use crate::encoded::Encoded; #[cfg(feature = "rkyv")] @@ -419,22 +422,113 @@ impl<'de> serde::Deserialize<'de> for ColdString { } } +#[cfg(test)] +trait TestString: + Clone + Default + fmt::Debug + Eq + Hash + PartialEq + for<'a> PartialEq<&'a str> +{ + fn new(s: &str) -> Self; + fn new_inline(s: &str) -> Self; + fn is_inline(&self) -> bool; + fn len(&self) -> usize; + fn as_bytes(&self) -> &[u8]; + fn as_str(&self) -> &str; + fn encoded_addr(&self) -> usize; +} + +#[cfg(test)] +impl TestString for ColdString { + fn new(s: &str) -> Self { + Self::new(s) + } + + fn new_inline(s: &str) -> Self { + Self::new_inline_const(s) + } + + fn is_inline(&self) -> bool { + self.is_inline() + } + + fn len(&self) -> usize { + self.len() + } + + fn as_bytes(&self) -> &[u8] { + self.as_bytes() + } + + fn as_str(&self) -> &str { + self.as_str() + } + + fn encoded_addr(&self) -> usize { + self.encoded.addr() + } +} + +#[cfg(test)] +impl TestString for arc::ArcColdStringInner { + fn new(s: &str) -> Self { + Self::new(s) + } + + fn new_inline(s: &str) -> Self { + Self::new_inline_const(s) + } + + fn is_inline(&self) -> bool { + self.is_inline() + } + + fn len(&self) -> usize { + self.len() + } + + fn as_bytes(&self) -> &[u8] { + self.as_bytes() + } + + fn as_str(&self) -> &str { + self.as_str() + } + + fn encoded_addr(&self) -> usize { + self.encoded_addr() + } +} + +#[cfg(test)] +macro_rules! each_string { + ($test:ident $(, $arg:expr)*) => { + $test::($($arg),*); + $test::($($arg),*); + $test::($($arg),*); + $test::($($arg),*); + $test::($($arg),*); + }; +} + #[cfg(all(test, feature = "serde"))] mod serde_tests { use super::*; use serde_test::{assert_tokens, Token}; + fn assert_serde(s: &'static str) + where + T: TestString + serde::Serialize + for<'de> serde::Deserialize<'de>, + { + assert_tokens(&T::new(s), &[Token::Str(s)]); + } + #[test] fn test_serde_cold_string_inline() { - let cs = ColdString::new("ferris"); - assert_tokens(&cs, &[Token::Str("ferris")]); + each_string!(assert_serde, "ferris"); } #[test] fn test_serde_cold_string_heap() { let long_str = "This is a significantly longer string for heap testing"; - let cs = ColdString::new(long_str); - assert_tokens(&cs, &[Token::Str(long_str)]); + each_string!(assert_serde, long_str); } } @@ -444,25 +538,38 @@ mod tests { use core::hash::BuildHasher; use hashbrown::hash_map::DefaultHashBuilder; + fn assert_layout() { + assert_eq!(mem::size_of::(), mem::size_of::()); + assert_eq!(mem::size_of::>(), mem::size_of::()); + } + #[test] fn test_layout() { - assert_eq!(mem::size_of::(), mem::size_of::()); + each_string!(assert_layout); + } + + fn assert_default() { + assert!(T::default().as_str().is_empty()); + assert_eq!(T::default().len(), 0); + assert_eq!(T::default(), ""); + assert_eq!(T::default(), T::new("")); } #[test] fn test_default() { - assert!(ColdString::default().is_empty()); - assert_eq!(ColdString::default().len(), 0); - assert_eq!(ColdString::default(), ""); - assert_eq!(ColdString::default(), ColdString::new("")); + each_string!(assert_default); } - fn assert_correct(s: &str) { - let cs = ColdString::new(s); + fn assert_correct(s: &str) + where + str: PartialEq, + for<'a> &'a str: PartialEq, + { + let cs = T::new(s); assert_eq!(s.len() <= mem::size_of::(), cs.is_inline()); assert_eq!(cs.len(), s.len()); assert_eq!(cs.as_bytes(), s.as_bytes()); - assert_eq!(cs.as_str(), s); + assert_eq!(cs.as_str().as_bytes(), s.as_bytes()); assert_eq!(cs.clone(), cs); let bh = DefaultHashBuilder::new(); let mut hasher1 = bh.build_hasher(); @@ -475,7 +582,7 @@ mod tests { assert_eq!(cs, *s); assert_eq!(*s, cs); let opt_s = Some(cs.clone()); - assert_eq!(opt_s, Some(ColdString::new(s))); + assert_eq!(opt_s, Some(T::new(s))); assert!(opt_s.is_some()); } @@ -513,7 +620,7 @@ mod tests { "AaAa0 ® ", str::from_utf8(&[240, 158, 186, 128, 240, 145, 143, 151]).unwrap(), ] { - assert_correct(s); + each_string!(assert_correct, s); } } @@ -558,19 +665,18 @@ mod tests { s.push(c); } - assert_correct(&s); + each_string!(assert_correct, &s); } } } - #[test] - fn test_unaligned_placement() { + fn assert_unaligned_placement() { for s_content in ["torture", "tor", "tortures", "tort", "torture torture"] { let mut buffer = [0u8; 32]; for offset in 0..8 { unsafe { - let dst = buffer.as_mut_ptr().add(offset) as *mut ColdString; - let s = ColdString::new(s_content); + let dst = buffer.as_mut_ptr().add(offset).cast::(); + let s = T::new(s_content); ptr::write_unaligned(dst, s); let recovered = ptr::read_unaligned(dst); assert_eq!(recovered.as_str(), s_content); @@ -579,23 +685,32 @@ mod tests { } } + #[test] + fn test_unaligned_placement() { + each_string!(assert_unaligned_placement); + } + #[test] fn ensure_zero_repr() { assert!(str::from_utf8(&Encoded::<()>::EIGHT_NUL_MAP.to_ne_bytes()).is_err()); } - #[test] - fn test_const_8nul_vs_non_const() { + fn assert_const_8nul_vs_non_const() { let nul8 = str::from_utf8(&encoded::EIGHT_NUL).unwrap(); - let const8 = ColdString::new_inline_const(nul8); - let non_const = ColdString::new(nul8); + let const8 = T::new_inline(nul8); + let non_const = T::new(nul8); let cloned = non_const.clone(); - assert_eq!(const8.encoded.addr(), non_const.encoded.addr()); - assert_eq!(const8.encoded.addr(), cloned.encoded.addr()); + assert_eq!(const8.encoded_addr(), non_const.encoded_addr()); + assert_eq!(const8.encoded_addr(), cloned.encoded_addr()); // check that a null pointer will return a str pointing to EIGHT_NUL assert_eq!( &const8.as_str().as_bytes()[0] as *const u8, (&encoded::EIGHT_NUL) as *const u8 ); } + + #[test] + fn test_const_8nul_vs_non_const() { + each_string!(assert_const_8nul_vs_non_const); + } } diff --git a/cold-string/tests/property.rs b/cold-string/tests/property.rs index 23a8b51..eebc68e 100644 --- a/cold-string/tests/property.rs +++ b/cold-string/tests/property.rs @@ -5,7 +5,7 @@ use proptest::prelude::*; fn proptest_config() -> ProptestConfig { ProptestConfig { failure_persistence: None, - cases: 16, + cases: 8, ..Default::default() } } From 2319bb049f3a0b1d0d0739d261623cff86c98b3c Mon Sep 17 00:00:00 2001 From: Thomas Pendock Date: Fri, 18 Sep 2026 23:32:49 -0700 Subject: [PATCH 4/6] fix miri tests --- .github/workflows/rust.yml | 2 +- cold-string/Cargo.toml | 5 ++++- cold-string/src/arc.rs | 22 +++++++++++----------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 167bc7f..3c71df6 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -39,6 +39,6 @@ jobs: - name: Miri 64 bit BE run: cargo +nightly miri test --all-features --manifest-path cold-string/Cargo.toml --target powerpc64-unknown-linux-gnu - name: Miri 32 bit LE - run: cargo +nightly miri test --all-features --target i686-unknown-linux-gnu + run: cargo +nightly miri test --all-features --manifest-path cold-string/Cargo.toml --target i686-unknown-linux-gnu - name: Miri 32 bit BE run: cargo +nightly miri test --all-features --manifest-path cold-string/Cargo.toml --target mips-unknown-linux-gnu diff --git a/cold-string/Cargo.toml b/cold-string/Cargo.toml index 3f0a884..c1ea60d 100644 --- a/cold-string/Cargo.toml +++ b/cold-string/Cargo.toml @@ -29,6 +29,9 @@ rustversion = "1.0.22" [dev-dependencies] hashbrown = "0.12.3" -loom = "0.7.2" serde_test = "1.0.177" proptest = {version = "=1.8.0", default-features = false, features = ["std", "bit-set"] } + +# Keep Loom's native generator dependency out of cross-target Miri builds. +[target.'cfg(target_arch = "x86_64")'.dev-dependencies] +loom = "0.7.2" diff --git a/cold-string/src/arc.rs b/cold-string/src/arc.rs index 2d563fc..d737432 100644 --- a/cold-string/src/arc.rs +++ b/cold-string/src/arc.rs @@ -13,11 +13,11 @@ use core::{ str, }; -#[cfg(all(loom, test))] +#[cfg(all(loom, test, target_arch = "x86_64"))] use loom::sync::atomic::{ fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering as AtomicOrdering, }; -#[cfg(not(all(loom, test)))] +#[cfg(not(all(loom, test, target_arch = "x86_64")))] use portable_atomic::{ fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering as AtomicOrdering, }; @@ -33,10 +33,10 @@ pub trait RefCount: Send + Sync + 'static { #[cfg(test)] fn refs(&self) -> usize; - #[cfg(all(test, not(loom)))] + #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] fn near_overflow() -> Self; - #[cfg(all(test, not(loom)))] + #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] fn is_immortal(&self) -> bool; } @@ -86,12 +86,12 @@ macro_rules! impl_ref_count { (self.load(AtomicOrdering::Relaxed) >> 1) as usize } - #[cfg(all(test, not(loom)))] + #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] fn near_overflow() -> Self { Self::new(<$int>::MAX - 1) } - #[cfg(all(test, not(loom)))] + #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] fn is_immortal(&self) -> bool { self.load(AtomicOrdering::Relaxed) & 1 != 0 } @@ -429,7 +429,7 @@ mod tests { } fn assert_inline_and_heap_clone() { - let inline = ArcColdStringInner::::new("short"); + let inline = ArcColdStringInner::::new("tiny"); let inline_clone = inline.clone(); assert!(inline.is_inline()); assert_eq!(inline, inline_clone); @@ -470,7 +470,7 @@ mod tests { each_ref_count!(assert_clones_across_threads); } - #[cfg(loom)] + #[cfg(all(loom, target_arch = "x86_64"))] fn model_clone_drop() { loom::model(|| { const TEXT: &str = "a shared string longer than one machine word"; @@ -498,13 +498,13 @@ mod tests { }); } - #[cfg(loom)] + #[cfg(all(loom, target_arch = "x86_64"))] #[test] fn loom_clone_drop() { each_ref_count!(model_clone_drop); } - #[cfg(not(loom))] + #[cfg(not(all(loom, target_arch = "x86_64")))] fn assert_count_becomes_immortal() { let count = A::near_overflow(); count.increment(); @@ -512,7 +512,7 @@ mod tests { assert!(!count.decrement()); } - #[cfg(not(loom))] + #[cfg(not(all(loom, target_arch = "x86_64")))] #[test] fn count_becomes_immortal_without_wrapping() { each_ref_count!(assert_count_becomes_immortal); From 436e022933a1b0f0126f71e70e4ded8512c8fa3b Mon Sep 17 00:00:00 2001 From: Thomas Pendock Date: Sat, 19 Sep 2026 11:50:12 -0700 Subject: [PATCH 5/6] minor simplifying --- cold-string/src/arc.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/cold-string/src/arc.rs b/cold-string/src/arc.rs index d737432..93fac12 100644 --- a/cold-string/src/arc.rs +++ b/cold-string/src/arc.rs @@ -15,11 +15,11 @@ use core::{ #[cfg(all(loom, test, target_arch = "x86_64"))] use loom::sync::atomic::{ - fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering as AtomicOrdering, + fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering::*, }; #[cfg(not(all(loom, test, target_arch = "x86_64")))] use portable_atomic::{ - fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering as AtomicOrdering, + fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering::*, }; use crate::encoded::Encoded; @@ -50,7 +50,7 @@ macro_rules! impl_ref_count { #[inline] fn increment(&self) { - let mut old = self.load(AtomicOrdering::Relaxed); + let mut old = self.load(Relaxed); loop { if old & 1 != 0 { return; @@ -61,12 +61,7 @@ macro_rules! impl_ref_count { } else { old + 2 }; - match self.compare_exchange_weak( - old, - new, - AtomicOrdering::Relaxed, - AtomicOrdering::Relaxed, - ) { + match self.compare_exchange_weak(old, new, Relaxed, Relaxed) { Ok(_) => return, Err(actual) => old = actual, } @@ -75,15 +70,12 @@ macro_rules! impl_ref_count { #[inline] fn decrement(&self) -> bool { - if self.load(AtomicOrdering::Relaxed) & 1 != 0 { - return false; - } - self.fetch_sub(2, AtomicOrdering::Release) == 2 + self.fetch_sub(2, Release) == 2 } #[cfg(test)] fn refs(&self) -> usize { - (self.load(AtomicOrdering::Relaxed) >> 1) as usize + (self.load(Relaxed) >> 1) as usize } #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] @@ -93,7 +85,7 @@ macro_rules! impl_ref_count { #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] fn is_immortal(&self) -> bool { - self.load(AtomicOrdering::Relaxed) & 1 != 0 + self.load(Relaxed) & 1 != 0 } } }; @@ -222,7 +214,7 @@ impl Drop for ArcColdStringInner { return; } - fence(AtomicOrdering::Acquire); + fence(Acquire); // SAFETY: this was the last reference and the count is synchronized. unsafe { self.encoded.deallocate() } } From b63819245d78a316c41bfe0b938eb679e1d7b6a6 Mon Sep 17 00:00:00 2001 From: Thomas Pendock Date: Sat, 19 Sep 2026 16:20:36 -0700 Subject: [PATCH 6/6] more tests --- cold-string/README.md | 2 +- cold-string/src/arc.rs | 101 ++++++++++++++++++++++++++++++++----- cold-string/src/encoded.rs | 28 +++++----- cold-string/src/lib.rs | 70 +++++++++++++++++++------ cold-string/src/vint.rs | 11 ++-- 5 files changed, 165 insertions(+), 47 deletions(-) diff --git a/cold-string/README.md b/cold-string/README.md index 6ba85c8..77c2eae 100644 --- a/cold-string/README.md +++ b/cold-string/README.md @@ -73,7 +73,7 @@ The 8 bytes encode one of three representations indicated by the 1st byte: - `11111xxx`: xxx is the length and the remaining 0-7 bytes are UTF-8 characters. - `xxxxxxxx`: All 8 bytes are UTF-8. -The exception is if `encoded` is `usize::MAX`, the UTF-8 bytes are "\0\0\0\0\0\0\0\0". +The exception is if `encoded` is `usize::MAX`, which represents one word of NUL bytes. `10xxxxxx` and `11111xxx` are chosen because they cannot be valid first bytes of UTF-8. diff --git a/cold-string/src/arc.rs b/cold-string/src/arc.rs index 93fac12..a0add1e 100644 --- a/cold-string/src/arc.rs +++ b/cold-string/src/arc.rs @@ -14,13 +14,9 @@ use core::{ }; #[cfg(all(loom, test, target_arch = "x86_64"))] -use loom::sync::atomic::{ - fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering::*, -}; +use loom::sync::atomic::{fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering::*}; #[cfg(not(all(loom, test, target_arch = "x86_64")))] -use portable_atomic::{ - fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering::*, -}; +use portable_atomic::{fence, AtomicU16, AtomicU32, AtomicU8, AtomicUsize, Ordering::*}; use crate::encoded::Encoded; @@ -33,11 +29,14 @@ pub trait RefCount: Send + Sync + 'static { #[cfg(test)] fn refs(&self) -> usize; - #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] + #[cfg(test)] fn near_overflow() -> Self; - #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] + #[cfg(test)] fn is_immortal(&self) -> bool; + + #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] + fn min_immortal() -> Self; } macro_rules! impl_ref_count { @@ -78,15 +77,20 @@ macro_rules! impl_ref_count { (self.load(Relaxed) >> 1) as usize } - #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] + #[cfg(test)] fn near_overflow() -> Self { Self::new(<$int>::MAX - 1) } - #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] + #[cfg(test)] fn is_immortal(&self) -> bool { self.load(Relaxed) & 1 != 0 } + + #[cfg(all(test, not(all(loom, target_arch = "x86_64"))))] + fn min_immortal() -> Self { + Self::new(1) + } } }; } @@ -106,6 +110,8 @@ pub struct ArcColdStringInner { /// /// Strings up to one machine word are stored inline. Longer strings use one /// allocation containing the reference count, variable-length length, and bytes. +/// If the reference count saturates, the allocation becomes immortal (and is +/// never deallocated) rather than allowing the count to wrap. /// /// ``` /// use cold_string::ArcColdString; @@ -117,12 +123,21 @@ pub struct ArcColdStringInner { pub type ArcColdString = ArcColdStringInner; /// An [`ArcColdString`] with an 8-bit reference count. +/// +/// It supports up to 127 live references. Cloning beyond that makes the +/// allocation immortal. pub type ArcColdString8 = ArcColdStringInner; /// An [`ArcColdString`] with a 16-bit reference count. +/// +/// It supports up to 32,767 live references. Cloning beyond that makes the +/// allocation immortal. pub type ArcColdString16 = ArcColdStringInner; /// An [`ArcColdString`] with a 32-bit reference count. +/// +/// It supports up to 2,147,483,647 live references. Cloning beyond that makes +/// the allocation immortal. pub type ArcColdString32 = ArcColdStringInner; impl ArcColdStringInner { @@ -430,6 +445,11 @@ mod tests { let heap_clone = heap.clone(); assert!(!heap.is_inline()); assert_eq!(heap.encoded.addr(), heap_clone.encoded.addr()); + assert_eq!( + heap.encoded.heap_ptr().as_ptr() as usize + % core::cmp::max(align_of::(), crate::heap::HEAP_ALIGN), + 0 + ); assert_eq!(heap.count().refs(), 2); drop(heap_clone); assert_eq!(heap.count().refs(), 1); @@ -490,24 +510,79 @@ mod tests { }); } + #[cfg(all(loom, target_arch = "x86_64"))] + fn model_final_drop() { + loom::model(|| { + let first = + ArcColdStringInner::::new("a shared string longer than one machine word"); + let second = first.clone(); + + let first = loom::thread::spawn(move || drop(first)); + let second = loom::thread::spawn(move || drop(second)); + + first.join().unwrap(); + second.join().unwrap(); + }); + } + + #[cfg(all(loom, target_arch = "x86_64"))] + fn model_overflow_race() { + loom::model(|| { + let count = loom::sync::Arc::new(A::near_overflow()); + let expected_refs = count.refs(); + let increment = count.clone(); + let decrement = count.clone(); + + let increment = loom::thread::spawn(move || increment.increment()); + let decrement = loom::thread::spawn(move || assert!(!decrement.decrement())); + + increment.join().unwrap(); + decrement.join().unwrap(); + + if count.is_immortal() { + assert!(!count.decrement()); + assert!(count.is_immortal()); + } else { + assert_eq!(count.refs(), expected_refs); + } + }); + } + #[cfg(all(loom, target_arch = "x86_64"))] #[test] fn loom_clone_drop() { each_ref_count!(model_clone_drop); + each_ref_count!(model_final_drop); + each_ref_count!(model_overflow_race); } #[cfg(not(all(loom, target_arch = "x86_64")))] - fn assert_count_becomes_immortal() { + fn assert_refcount_edges() { + let count = A::new(); + assert_eq!(count.refs(), 1); + count.increment(); + assert_eq!(count.refs(), 2); + assert!(!count.decrement()); + assert_eq!(count.refs(), 1); + assert!(count.decrement()); + assert_eq!(count.refs(), 0); + let count = A::near_overflow(); count.increment(); assert!(count.is_immortal()); assert!(!count.decrement()); + assert!(count.is_immortal()); + + let count = A::min_immortal(); + assert!(count.is_immortal()); + assert!(!count.decrement()); + assert!(count.is_immortal()); } #[cfg(not(all(loom, target_arch = "x86_64")))] #[test] - fn count_becomes_immortal_without_wrapping() { - each_ref_count!(assert_count_becomes_immortal); + fn refcount_edges() { + each_ref_count!(assert_refcount_edges); } #[test] diff --git a/cold-string/src/encoded.rs b/cold-string/src/encoded.rs index 67f39a1..560fa2e 100644 --- a/cold-string/src/encoded.rs +++ b/cold-string/src/encoded.rs @@ -9,7 +9,7 @@ use sptr::Strict; use crate::heap::{VintStringInner, HEAP_ALIGN}; pub(crate) const WIDTH: usize = mem::size_of::(); -pub(crate) static EIGHT_NUL: [u8; WIDTH] = [0u8; WIDTH]; +pub(crate) static WORD_NUL: [u8; WIDTH] = [0u8; WIDTH]; /// The common one-word representation used by both owning string types. #[repr(transparent)] @@ -30,7 +30,7 @@ impl Encoded { const INLINE_TAG: usize = usize::from_ne_bytes(0b11111000usize.to_le_bytes()); const PTR_TAG: usize = usize::from_ne_bytes(0b10000000usize.to_le_bytes()); const LEN_MASK: usize = usize::from_ne_bytes(0b111usize.to_le_bytes()); - pub(crate) const EIGHT_NUL_MAP: usize = usize::MAX; + pub(crate) const WORD_NUL_MAP: usize = usize::MAX; const ROT: u32 = if cfg!(target_endian = "little") { 0 } else { @@ -49,8 +49,8 @@ impl Encoded { #[inline] pub(crate) fn new_inline(s: &str) -> Self { debug_assert!(s.len() <= WIDTH); - if s.as_bytes() == EIGHT_NUL { - return Self::new_eight_nul(); + if s.as_bytes() == WORD_NUL { + return Self::new_word_nul(); } let mut buf = Self::inline_buf(s); let start = Self::utf8_start(s.len()); @@ -66,7 +66,7 @@ impl Encoded { pub(crate) const fn new_inline_const(s: &str) -> Self { if s.len() > WIDTH { panic!( - "Length for `new_inline_const` must be less than `core::mem::size_of::()`." + "Length for `new_inline_const` must be at most `core::mem::size_of::()`." ); } let mut buf = Self::inline_buf(s); @@ -78,7 +78,7 @@ impl Encoded { } if usize::from_ne_bytes(buf) == 0 { - return Self::new_eight_nul(); + return Self::new_word_nul(); } // SAFETY: the all-zero representation was handled above. @@ -140,14 +140,14 @@ impl Encoded { } #[inline] - fn is_eight_nul(&self) -> bool { - self.addr() == Self::EIGHT_NUL_MAP + fn is_word_nul(&self) -> bool { + self.addr() == Self::WORD_NUL_MAP } #[inline] fn inline_len(&self) -> usize { debug_assert!(self.is_inline()); - debug_assert!(!self.is_eight_nul()); + debug_assert!(!self.is_word_nul()); let addr = self.addr(); match addr & Self::INLINE_TAG { Self::INLINE_TAG => (addr & Self::LEN_MASK).rotate_right(Self::ROT), @@ -158,8 +158,8 @@ impl Encoded { #[inline] pub(crate) unsafe fn inline_bytes(&self) -> &[u8] { debug_assert!(self.is_inline()); - if self.is_eight_nul() { - return &EIGHT_NUL; + if self.is_word_nul() { + return &WORD_NUL; } let len = self.inline_len(); let bytes = ptr::addr_of!(self.ptr).cast::(); @@ -189,9 +189,9 @@ impl Encoded { } #[inline] - const fn new_eight_nul() -> Self { - // SAFETY: `EIGHT_NUL_MAP` is non-zero. - unsafe { Self::from_inline_buf(Self::EIGHT_NUL_MAP.to_ne_bytes()) } + const fn new_word_nul() -> Self { + // SAFETY: `WORD_NUL_MAP` is non-zero. + unsafe { Self::from_inline_buf(Self::WORD_NUL_MAP.to_ne_bytes()) } } /// `buf` must not be all zeroes. diff --git a/cold-string/src/lib.rs b/cold-string/src/lib.rs index ba4be3e..3f2c4c0 100644 --- a/cold-string/src/lib.rs +++ b/cold-string/src/lib.rs @@ -63,7 +63,7 @@ pub struct ColdString { /// - 11111xxx: xxx is the length in range 0..=7, followed by length UTF-8 bytes. /// - xxxxxxxx (valid UTF-8): 8 UTF-8 bytes. /// - /// The exception is if `encoded` is `usize::MAX`, the UTF-8 bytes are "\0\0\0\0\0\0\0\0". + /// The exception is if `encoded` is `usize::MAX`, which represents one word of NUL bytes. encoded: Encoded<()>, } @@ -125,7 +125,7 @@ impl ColdString { } /// Creates a new [`ColdString`] from any type that implements `AsRef`. - /// If the string is shorter than `core::mem::size_of::()`, then it + /// If the string is at most `core::mem::size_of::()` bytes, then it /// will be inlined on the stack. pub fn new>(x: T) -> Self { let s = x.as_ref(); @@ -139,7 +139,7 @@ impl ColdString { /// In a dynamic context you can use the method [`ColdString::new()`]. /// /// # Panics - /// The string must be less than `core::mem::size_of::()`. Creating + /// The string must be at most `core::mem::size_of::()`. Creating /// a [`ColdString`] larger than that is not supported. /// /// @@ -427,8 +427,10 @@ trait TestString: Clone + Default + fmt::Debug + Eq + Hash + PartialEq + for<'a> PartialEq<&'a str> { fn new(s: &str) -> Self; + fn from_utf8(bytes: &[u8]) -> Result; fn new_inline(s: &str) -> Self; fn is_inline(&self) -> bool; + fn is_empty(&self) -> bool; fn len(&self) -> usize; fn as_bytes(&self) -> &[u8]; fn as_str(&self) -> &str; @@ -441,6 +443,10 @@ impl TestString for ColdString { Self::new(s) } + fn from_utf8(bytes: &[u8]) -> Result { + Self::from_utf8(bytes) + } + fn new_inline(s: &str) -> Self { Self::new_inline_const(s) } @@ -449,6 +455,10 @@ impl TestString for ColdString { self.is_inline() } + fn is_empty(&self) -> bool { + self.is_empty() + } + fn len(&self) -> usize { self.len() } @@ -472,6 +482,10 @@ impl TestString for arc::ArcColdStringInner { Self::new(s) } + fn from_utf8(bytes: &[u8]) -> Result { + Self::from_utf8(bytes) + } + fn new_inline(s: &str) -> Self { Self::new_inline_const(s) } @@ -480,6 +494,10 @@ impl TestString for arc::ArcColdStringInner { self.is_inline() } + fn is_empty(&self) -> bool { + self.is_empty() + } + fn len(&self) -> usize { self.len() } @@ -549,7 +567,7 @@ mod tests { } fn assert_default() { - assert!(T::default().as_str().is_empty()); + assert!(T::default().is_empty()); assert_eq!(T::default().len(), 0); assert_eq!(T::default(), ""); assert_eq!(T::default(), T::new("")); @@ -560,6 +578,26 @@ mod tests { each_string!(assert_default); } + fn assert_utf8_validation() { + for valid in ["", "🦀", "valid UTF-8 🦀 longer than one word"] { + assert_eq!(T::from_utf8(valid.as_bytes()).unwrap().as_str(), valid); + } + + for invalid in [ + &[0x80][..], + &[0xff][..], + &[0xc0, 0x80][..], + &[0xe2, 0x82][..], + ] { + assert!(T::from_utf8(invalid).is_err()); + } + } + + #[test] + fn test_utf8_validation() { + each_string!(assert_utf8_validation); + } + fn assert_correct(s: &str) where str: PartialEq, @@ -692,25 +730,25 @@ mod tests { #[test] fn ensure_zero_repr() { - assert!(str::from_utf8(&Encoded::<()>::EIGHT_NUL_MAP.to_ne_bytes()).is_err()); + assert!(str::from_utf8(&Encoded::<()>::WORD_NUL_MAP.to_ne_bytes()).is_err()); } - fn assert_const_8nul_vs_non_const() { - let nul8 = str::from_utf8(&encoded::EIGHT_NUL).unwrap(); - let const8 = T::new_inline(nul8); - let non_const = T::new(nul8); + fn assert_const_word_nul() { + let nul = str::from_utf8(&encoded::WORD_NUL).unwrap(); + let const_value = T::new_inline(nul); + let non_const = T::new(nul); let cloned = non_const.clone(); - assert_eq!(const8.encoded_addr(), non_const.encoded_addr()); - assert_eq!(const8.encoded_addr(), cloned.encoded_addr()); - // check that a null pointer will return a str pointing to EIGHT_NUL + assert_eq!(const_value.encoded_addr(), non_const.encoded_addr()); + assert_eq!(const_value.encoded_addr(), cloned.encoded_addr()); + // The sentinel returns a slice into the shared word-sized NUL array. assert_eq!( - &const8.as_str().as_bytes()[0] as *const u8, - (&encoded::EIGHT_NUL) as *const u8 + &const_value.as_str().as_bytes()[0] as *const u8, + (&encoded::WORD_NUL) as *const u8 ); } #[test] - fn test_const_8nul_vs_non_const() { - each_string!(assert_const_8nul_vs_non_const); + fn test_const_word_nul() { + each_string!(assert_const_word_nul); } } diff --git a/cold-string/src/vint.rs b/cold-string/src/vint.rs index dcdc091..3c7c8e5 100644 --- a/cold-string/src/vint.rs +++ b/cold-string/src/vint.rs @@ -53,12 +53,17 @@ mod tests { #[test] fn vint_round_trip() { - for x in [0, 1, 42, 59243, 5, 8, 7, 63, 64] { + for x in 0..=1024 { assert_correct(x); } - for x in 0..=u16::MAX { - assert_correct(x as u64); + let mut shift = 7; + while shift < usize::BITS { + let boundary = 1usize << shift; + for x in boundary - 1..=boundary + 1 { + assert_correct(x as u64); + } + shift += 7; } for x in 0..=100 {