perf: several small improvements

bench/benches/compact_str.rs

@@ -123,7 +123,7 @@ fn bench_str_to_compact_string(c: &mut Criterion) {
 fn bench_repr_creation(c: &mut Criterion) {
     let mut group = c.benchmark_group("Creation");
 
-    let words: Vec<String> = vec![0, 11, 12, 22, 23, 24, 25, 50]
+    let words: Vec<String> = vec![0, 11, 12, 22, 23, 24, 25, 50, 100, 500]
         .into_iter()
         .map(|len| (0..len).into_iter().map(|_| 'a').collect())
         .collect();

compact_str/src/repr/heap.rs

@@ -32,9 +32,10 @@ static_assertions::assert_eq_size!(HeapBuffer, Repr);
 static_assertions::assert_eq_align!(HeapBuffer, Repr);
 
 impl HeapBuffer {
-    /// Create a [`HeapBuffer`] with the provided text
+    /// Allocate a fresh heap buffer and copy `text` into it, the shared core for
+    /// `new`/`new_panic`.
     #[inline]
-    pub(crate) fn new(text: &str) -> Result<Self, ReserveError> {
+    fn build(text: &str) -> Result<Self, ReserveError> {
         let len = text.len();
         let (cap, ptr) = allocate_ptr(len)?;
 
@@ -43,13 +44,30 @@ impl HeapBuffer {
         // SAFETY: We know both `src` and `dest` are valid for respectively reads and writes of
         // length `len` because `len` comes from `src`, and `dest` was allocated to be at least that
         // length. We also know they're non-overlapping because `dest` is newly allocated
-        unsafe { ptr.as_ptr().copy_from_nonoverlapping(text.as_ptr(), len) };
+        unsafe { super::copy_medium(text.as_ptr(), ptr.as_ptr(), len) };
 
         Ok(HeapBuffer { ptr, len, cap })
     }
 
+    /// Create a [`HeapBuffer`] with the provided text.
+    ///
+    /// Note(parker): deliberately not `#[inline]`, this is the cold allocating heap
+    /// construction path, keeping it out-of-line prevents it from bloating every
+    /// `CompactString` construction callsite.
+    pub(crate) fn new(text: &str) -> Result<Self, ReserveError> {
+        Self::build(text)
+    }
+
+    /// Like [`HeapBuffer::new`], but panics on allocation failure instead of returning a
+    /// `Result`.
+    #[track_caller]
+    pub(crate) fn new_panic(text: &str) -> Self {
+        Self::build(text).unwrap_with_msg()
+    }
+
     /// Create an empty [`HeapBuffer`] with a specific capacity
-    #[inline]
+    ///
+    /// Note: deliberately not `#[inline]` — see `HeapBuffer::new`. This is a cold allocating path.
     pub(crate) fn with_capacity(capacity: usize) -> Result<Self, ReserveError> {
         let len = 0;
         let (cap, ptr) = allocate_ptr(capacity)?;
@@ -61,7 +79,8 @@ impl HeapBuffer {
     ///
     /// To prevent frequent re-allocations, this method will create a [`HeapBuffer`] with a capacity
     /// of `text.len() + additional` or `text.len() * 1.5`, whichever is greater
-    #[inline]
+    ///
+    /// Note: deliberately not `#[inline]` — see `HeapBuffer::new`. This is a cold allocating path.
     pub(crate) fn with_additional(text: &str, additional: usize) -> Result<Self, ReserveError> {
         let len = text.len();
         let new_capacity = amortized_growth(len, additional);
@@ -72,7 +91,7 @@ impl HeapBuffer {
         // SAFETY: We know both `src` and `dest` are valid for respectively reads and writes of
         // length `len` because `len` comes from `src`, and `dest` was allocated to be at least that
         // length. We also know they're non-overlapping because `dest` is newly allocated
-        unsafe { ptr.as_ptr().copy_from_nonoverlapping(text.as_ptr(), len) };
+        unsafe { super::copy_medium(text.as_ptr(), ptr.as_ptr(), len) };
 
         Ok(HeapBuffer { ptr, len, cap })
     }
@@ -237,11 +256,7 @@ impl Clone for HeapBuffer {
         // SAFETY:
         // * `src` and `dst` don't overlap because we just created `dst`
         // * `src` and `dst` are both valid for `self.len` bytes because self.len < capacity
-        unsafe {
-            new.ptr
-                .as_ptr()
-                .copy_from_nonoverlapping(self.ptr.as_ptr(), self.len)
-        };
+        unsafe { super::copy_medium(self.ptr.as_ptr(), new.ptr.as_ptr(), self.len) };
         // SAFETY:
         // * We copied the text from self, which is valid UTF-8
         unsafe { new.set_len(self.len) };

compact_str/src/repr/inline.rs

@@ -1,5 +1,3 @@
-use core::ptr;
-
 use super::{Repr, LENGTH_MASK, MAX_SIZE};
 
 /// A buffer stored on the stack whose size is equal to the stack size of `String`
@@ -40,7 +38,7 @@ impl InlineBuffer {
         // * dst (`buffer`) is valid for `len` bytes because we assert src is less than MAX_SIZE
         // * src and dst don't overlap because we created dst
         //
-        copy_small(text.as_ptr(), buffer.0.as_mut_ptr(), len);
+        super::copy_small(text.as_ptr(), buffer.0.as_mut_ptr(), len);
 
         buffer
     }
@@ -118,41 +116,6 @@ impl InlineBuffer {
     }
 }
 
-/// Copies <= [`MAX_SIZE`] bytes `len` bytes from `src` to `dst`.
-///
-/// `copy_nonoverlapping` with a runtime length emits a `memcpy` call into libc. This is
-/// relatively expensive for the short copies used when inlining a string, which can
-/// instead be implemented with a handfull of ASM instructions.
-///
-/// Since an inline string is at most `MAX_SIZE` bytes, we use a ladder of constant-length
-/// overlapping copies which LLVM can best optimize.
-///
-/// # Safety
-///
-/// * `len <= MAX_SIZE`.
-/// * `src` and `dst` are valid for `len` bytes and do not overlap.
-///
-#[inline(always)]
-unsafe fn copy_small(src: *const u8, dst: *mut u8, len: usize) {
-    debug_assert!(len <= MAX_SIZE);
-
-    if len >= 16 {
-        ptr::copy_nonoverlapping(src, dst, 16);
-        ptr::copy_nonoverlapping(src.add(len - 16), dst.add(len - 16), 16);
-    } else if len >= 8 {
-        ptr::copy_nonoverlapping(src, dst, 8);
-        ptr::copy_nonoverlapping(src.add(len - 8), dst.add(len - 8), 8);
-    } else if len >= 4 {
-        ptr::copy_nonoverlapping(src, dst, 4);
-        ptr::copy_nonoverlapping(src.add(len - 4), dst.add(len - 4), 4);
-    } else if len >= 2 {
-        ptr::copy_nonoverlapping(src, dst, 2);
-        ptr::copy_nonoverlapping(src.add(len - 2), dst.add(len - 2), 2);
-    } else if len == 1 {
-        *dst = *src;
-    }
-}
-
 #[cfg(test)]
 mod tests {
     #[test]

compact_str/src/repr/mod.rs

@@ -88,14 +88,19 @@ impl Repr {
     pub(crate) fn new_panic(text: &str) -> Self {
         let len = text.len();
 
-        if len == 0 {
-            EMPTY
-        } else if len <= MAX_SIZE {
+        // Note(parker): `len == 0` is intentionally not special-cased.
+        //
+        // I've gone back and forth on this a few times, but the slight performance win
+        // for empty strings is not worth the instruction bloat. `copy_small(_, _, 0)` in
+        // `InlineBuffer::new` ends up being a no-op as well.
+
+        if len <= MAX_SIZE {
             // SAFETY: We checked that the length of text is less than or equal to MAX_SIZE
             let inline = unsafe { InlineBuffer::new(text) };
             Repr::from_inline(inline)
         } else {
-            Repr::from_heap(HeapBuffer::new(text).unwrap_with_msg())
+            let heap = HeapBuffer::new_panic(text);
+            Repr::from_heap(heap)
         }
     }
 
@@ -961,6 +966,74 @@ const unsafe fn assume(cond: bool) {
     }
 }
 
+/// Copies `len` bytes (`len <= MAX_SIZE`) from `src` to `dst`, for the inline-string path.
+///
+/// A `ptr::copy_nonoverlapping` with a *runtime* length lowers to an out-of-line `memcpy` call
+/// (the backend only inlines the copy when the length is a compile-time constant), which is
+/// wasteful for the tiny copies used when inlining a string. Since an inline string is at most
+/// `MAX_SIZE` bytes we instead use a ladder of *constant*-length overlapping copies, each of which
+/// the compiler lowers to a couple of load/store instructions — and which const-fold away entirely
+/// when `len` is known (e.g. `CompactString::const_new` / string literals).
+///
+/// # Safety
+/// * `len <= MAX_SIZE`.
+/// * `src` is valid for reads of `len` bytes; `dst` is valid for writes of `len` bytes.
+/// * `src` and `dst` do not overlap.
+#[inline(always)]
+pub(crate) unsafe fn copy_small(src: *const u8, dst: *mut u8, len: usize) {
+    debug_assert!(len <= MAX_SIZE);
+
+    // Each branch only touches bytes within `[0, len)` — the tail copy starts at `len - N` only
+    // once we know `len >= N` — so the overlapping copies never read or write out of range.
+    if len >= 16 {
+        ptr::copy_nonoverlapping(src, dst, 16);
+        ptr::copy_nonoverlapping(src.add(len - 16), dst.add(len - 16), 16);
+    } else if len >= 8 {
+        ptr::copy_nonoverlapping(src, dst, 8);
+        ptr::copy_nonoverlapping(src.add(len - 8), dst.add(len - 8), 8);
+    } else if len >= 4 {
+        ptr::copy_nonoverlapping(src, dst, 4);
+        ptr::copy_nonoverlapping(src.add(len - 4), dst.add(len - 4), 4);
+    } else if len >= 2 {
+        ptr::copy_nonoverlapping(src, dst, 2);
+        ptr::copy_nonoverlapping(src.add(len - 2), dst.add(len - 2), 2);
+    } else if len == 1 {
+        *dst = *src;
+    }
+}
+
+/// Copies `len` bytes from `src` to `dst`, for the heap-string path.
+///
+/// The common case for a freshly heap-allocated `CompactString` is a length just over
+/// the inline threshold. For those the medium bands `[16,64]` are copied inline with two
+/// overlapping fixed-size load/stores, skipping the `memcpy` call boundary like
+/// [`copy_small`].
+///
+/// Copies `> 64` bytes delegate to `memcpy`, which has per-CPU-tuned large-copy paths
+/// that any optimization we do won't beat.
+/// Copies `< 16` bytes are rare on this path so they simply delegate to `memcpy` as well
+/// which keeps this correct for *any* length.
+///
+/// # Safety
+///
+/// * `src` is valid for reads of `len` bytes; `dst` is valid for writes of `len` bytes.
+/// * `src` and `dst` do not overlap.
+///
+#[inline]
+pub(crate) unsafe fn copy_medium(src: *const u8, dst: *mut u8, len: usize) {
+    if len > 64 {
+        ptr::copy_nonoverlapping(src, dst, len);
+    } else if len >= 32 {
+        ptr::copy_nonoverlapping(src, dst, 32);
+        ptr::copy_nonoverlapping(src.add(len - 32), dst.add(len - 32), 32);
+    } else if len >= 16 {
+        ptr::copy_nonoverlapping(src, dst, 16);
+        ptr::copy_nonoverlapping(src.add(len - 16), dst.add(len - 16), 16);
+    } else {
+        ptr::copy_nonoverlapping(src, dst, len);
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use alloc::string::{String, ToString};