Repository: ParkMyCar/compact_str
Pull request #219 (2022-08-24 13:09:14)

Copy inline string reversed
It seem like some optimization step goes berserk if you copy a byte
slice from start to end. The while-loop is partially unrolled which
results in an excessively long, extremely cache unfriendly code.

If you copy the string in the reverse direction, starting with the last
byte, the optimizer understands the pattern, and uses `memcpy()`, which
is a sane decision.

If you only use `CompactString::new_inline()` in a `const` context, the
difference does not matter. But if you use it for runtime code, the
difference is enormous. Valid use cases for `new_inline()` in a
non-const context include `const` function that may be also useful in a
dynamic context.


Patch:
diff --git a/compact_str/src/lib.rs b/compact_str/src/lib.rs
index 20638cc0..17984856 100644
--- a/compact_str/src/lib.rs
+++ b/compact_str/src/lib.rs
@@ -1372,6 +1372,20 @@ impl CompactString {
     pub fn from_utf16be_lossy(v: impl AsRef<[u8]>) -> Self {
         CompactString::from_utf16x_lossy(v.as_ref(), u16::from_be, u16::from_be_bytes)
     }
+
+    /// Convert the [`CompactString`] into a [`String`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use compact_str::CompactString;
+    /// let s = CompactString::new("Hello world");
+    /// let s = s.into_string();
+    /// assert_eq!(s, "Hello world");
+    /// ```
+    pub fn into_string(self) -> String {
+        self.repr.into_string()
+    }
 }
 
 impl Default for CompactString {
@@ -1505,8 +1519,9 @@ impl From<Box<str>> for CompactString {
 }
 
 impl From<CompactString> for String {
+    #[inline]
     fn from(s: CompactString) -> Self {
-        s.repr.into_string()
+        s.into_string()
     }
 }
 
@@ -1557,6 +1572,13 @@ impl FromIterator<Box<str>> for CompactString {
     }
 }
 
+impl<'a> FromIterator<Cow<'a, str>> for CompactString {
+    fn from_iter<T: IntoIterator<Item = Cow<'a, str>>>(iter: T) -> Self {
+        let repr = iter.into_iter().collect();
+        CompactString { repr }
+    }
+}
+
 impl FromIterator<String> for CompactString {
     fn from_iter<T: IntoIterator<Item = String>>(iter: T) -> Self {
         let repr = iter.into_iter().collect();
@@ -1564,6 +1586,33 @@ impl FromIterator<String> for CompactString {
     }
 }
 
+impl FromIterator<CompactString> for CompactString {
+    fn from_iter<T: IntoIterator<Item = CompactString>>(iter: T) -> Self {
+        let repr = iter.into_iter().collect();
+        CompactString { repr }
+    }
+}
+
+impl FromIterator<CompactString> for String {
+    fn from_iter<T: IntoIterator<Item = CompactString>>(iter: T) -> Self {
+        let mut iterator = iter.into_iter();
+        match iterator.next() {
+            None => String::new(),
+            Some(buf) => {
+                let mut buf = buf.into_string();
+                buf.extend(iterator);
+                buf
+            }
+        }
+    }
+}
+
+impl FromIterator<CompactString> for Cow<'_, str> {
+    fn from_iter<T: IntoIterator<Item = CompactString>>(iter: T) -> Self {
+        String::from_iter(iter).into()
+    }
+}
+
 impl Extend<char> for CompactString {
     fn extend<T: IntoIterator<Item = char>>(&mut self, iter: T) {
         self.repr.extend(iter)
@@ -1600,6 +1649,28 @@ impl Extend<String> for CompactString {
     }
 }
 
+impl Extend<CompactString> for String {
+    fn extend<T: IntoIterator<Item = CompactString>>(&mut self, iter: T) {
+        for s in iter {
+            self.push_str(&s);
+        }
+    }
+}
+
+impl Extend<CompactString> for CompactString {
+    fn extend<T: IntoIterator<Item = CompactString>>(&mut self, iter: T) {
+        for s in iter {
+            self.push_str(&s);
+        }
+    }
+}
+
+impl<'a> Extend<CompactString> for Cow<'a, str> {
+    fn extend<T: IntoIterator<Item = CompactString>>(&mut self, iter: T) {
+        self.to_mut().extend(iter);
+    }
+}
+
 impl fmt::Write for CompactString {
     fn write_str(&mut self, s: &str) -> fmt::Result {
         self.push_str(s);
diff --git a/compact_str/src/repr/inline.rs b/compact_str/src/repr/inline.rs
index 98e6ecd3..758d114f 100644
--- a/compact_str/src/repr/inline.rs
+++ b/compact_str/src/repr/inline.rs
@@ -41,11 +41,14 @@ impl InlineString {
         // set the length
         buffer[MAX_SIZE - 1] = len as u8 | LENGTH_MASK;
 
-        // Note: for loops aren't allowed in `const fn`, hence the while
-        let mut i = 0;
-        while i < len {
-            buffer[i] = text.as_bytes()[i];
-            i += 1;
+        // Note: for loops aren't allowed in `const fn`, hence the while.
+        // Note: Iterating forward results in badly optimized code, because the compiler tries to
+        //       unroll the loop.
+        let text = text.as_bytes();
+        let mut i = len;
+        while i > 0 {
+            buffer[i - 1] = text[i - 1];
+            i -= 1;
         }
 
         InlineString { buffer }
diff --git a/compact_str/src/repr/iter.rs b/compact_str/src/repr/iter.rs
index 20a2c6e6..f53989e3 100644
--- a/compact_str/src/repr/iter.rs
+++ b/compact_str/src/repr/iter.rs
@@ -1,6 +1,7 @@
 //! Implementations of the [`FromIterator`] trait to make building `CompactString`s more ergonomic
 
 use core::iter::FromIterator;
+use std::borrow::Cow;
 
 use super::{
     HeapString,
@@ -8,6 +9,7 @@ use super::{
     Repr,
     MAX_SIZE,
 };
+use crate::CompactString;
 
 impl FromIterator<char> for Repr {
     fn from_iter<T: IntoIterator<Item = char>>(iter: T) -> Self {
@@ -125,6 +127,18 @@ impl FromIterator<String> for Repr {
     }
 }
 
+impl FromIterator<CompactString> for Repr {
+    fn from_iter<T: IntoIterator<Item = CompactString>>(iter: T) -> Self {
+        from_as_ref_str_iterator(iter.into_iter())
+    }
+}
+
+impl<'a> FromIterator<Cow<'a, str>> for Repr {
+    fn from_iter<T: IntoIterator<Item = Cow<'a, str>>>(iter: T) -> Self {
+        from_as_ref_str_iterator(iter.into_iter())
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::Repr;


Test patch:
diff --git a/compact_str/src/tests.rs b/compact_str/src/tests.rs
index 185e0555..171478f5 100644
--- a/compact_str/src/tests.rs
+++ b/compact_str/src/tests.rs
@@ -1409,3 +1409,57 @@ fn test_from_utf16x_lossy() {
         "👯\u{200d}♀�",
     );
 }
+
+#[test]
+fn test_collect() {
+    const VALUES: &[&str] = &["foo", "bar", "baz"];
+
+    assert_eq!(
+        VALUES
+            .iter()
+            .copied()
+            .map(Cow::Borrowed)
+            .collect::<CompactString>(),
+        "foobarbaz",
+    );
+    assert_eq!(
+        VALUES
+            .iter()
+            .copied()
+            .map(|s| Cow::Owned(s.into()))
+            .collect::<CompactString>(),
+        "foobarbaz",
+    );
+    assert_eq!(
+        VALUES
+            .iter()
+            .copied()
+            .map(Box::<str>::from)
+            .collect::<CompactString>(),
+        "foobarbaz",
+    );
+    assert_eq!(
+        VALUES
+            .iter()
+            .copied()
+            .map(CompactString::from)
+            .collect::<String>(),
+        "foobarbaz",
+    );
+    assert_eq!(
+        VALUES
+            .iter()
+            .copied()
+            .map(CompactString::from)
+            .collect::<Cow<'_, str>>(),
+        "foobarbaz",
+    );
+    assert_eq!(
+        VALUES
+            .iter()
+            .copied()
+            .flat_map(|s| s.chars())
+            .collect::<Cow<'_, str>>(),
+        "foobarbaz",
+    );
+}