Repository: ParkMyCar/compact_str
Pull request #254 (2023-02-13 15:01:34)

perf: Inline strings when Clone-ing
This PR implements a performance improvement for `Clone`-ing a `CompactString`. If the capacity of the original `CompactString` is "inline-able", i.e. it's <= 24 bytes, then the new `CompactString` we return will be inlined.

Also included in this PR is a change to the fuzzing harness to assert we inline `CompactString`s when possible, and to limit the number of actions per run, which allows to improve the total number of runs.


Patch:
diff --git a/compact_str/src/repr/mod.rs b/compact_str/src/repr/mod.rs
index 8e168c5a..84fc7428 100644
--- a/compact_str/src/repr/mod.rs
+++ b/compact_str/src/repr/mod.rs
@@ -534,8 +534,19 @@ impl Clone for Repr {
         #[cold]
         fn clone_heap(this: &Repr) -> Repr {
             let heap = unsafe { &*(this as *const _ as *const HeapBuffer) };
-            let new = heap.clone();
-            unsafe { mem::transmute(new) }
+
+            // If the contained string is small enough, we will inline it instead of allocating
+            if heap.capacity() <= MAX_SIZE {
+                // SAFETY: Checked to make sure the capacity is <= MAX_SIZE, which means the length
+                // of the string is also <= MAX_SIZE
+                let inline = unsafe { InlineBuffer::new(this.as_str()) };
+                // SAFETY: InlineBuffer and Repr have the same layout
+                unsafe { mem::transmute(inline) }
+            } else {
+                let new = heap.clone();
+                // SAFETY: HeapBuffer and Repr have the same layout
+                unsafe { mem::transmute(new) }
+            }
         }
 
         if last_byte == HEAP_MASK {
diff --git a/fuzz/src/actions.rs b/fuzz/src/actions.rs
index 67b5e26d..39d7711e 100644
--- a/fuzz/src/actions.rs
+++ b/fuzz/src/actions.rs
@@ -335,6 +335,11 @@ impl Action<'_> {
                 drop(og);
 
                 let compact_clone = compact.clone();
+                // when cloning, even if the original CompactString was heap allocated, we should
+                // inline the new one, if possible
+                if compact.capacity() <= super::MAX_INLINE_LENGTH {
+                    assert!(!compact_clone.is_heap_allocated())
+                }
                 let og = std::mem::replace(compact, compact_clone);
                 drop(og);
             }


Test patch:
diff --git a/compact_str/src/tests.rs b/compact_str/src/tests.rs
index bda16e04..d1abc2c9 100644
--- a/compact_str/src/tests.rs
+++ b/compact_str/src/tests.rs
@@ -1458,3 +1458,23 @@ fn test_into_cow() {
 
     assert_eq!(og, cow);
 }
+
+#[test]
+fn test_from_string_inlines_on_push() {
+    let mut compact = CompactString::from("hello".to_string());
+    assert!(compact.is_heap_allocated());
+
+    compact.push_str(" world");
+    // when growing the CompactString we should inline it
+    assert!(!compact.is_heap_allocated());
+}
+
+#[test]
+fn test_from_string_inlines_on_clone() {
+    let a = CompactString::from("hello".to_string());
+    assert!(a.is_heap_allocated());
+
+    let b = a.clone();
+    // when cloning the CompactString we should inline it
+    assert!(!b.is_heap_allocated());
+}