Various improvements to Classes, oriented around reducing allocations (#2870)

* Various improvements to Classes, oriented around reducing allocations

- `push` no longer performs unnecessary allocations if `self` is empty.
    - Most constructors (FromIterator, From, Extend) are now oriented around push, to take advantage of this
- `to_string` and `into_prop_value`:
  - No longer allocate an unnecessary `Vec`; they instead preallocate a string of the correct length and push into it directly.
  - No longer use a length check + `unsafe`; they instead match over a fallible .next() and proceed from there.
  - Additionally, `into_prop_value` no longer builds a `String` or `Rc` if `Classes` contains a single `&'static str` or is empty.
- `From<String>` no longer clones the string if it contains a single class.
- `impl Eq for Classes`

* Fix duplicated is_empty test

packages/yew/src/html/classes.rs

@@ -1,5 +1,4 @@
 use std::borrow::{Borrow, Cow};
-use std::hint::unreachable_unchecked;
 use std::iter::FromIterator;
 use std::rc::Rc;
 
@@ -16,8 +15,28 @@ pub struct Classes {
     set: IndexSet<Cow<'static, str>>,
 }
 
+/// helper method to efficiently turn a set of classes into a space-separated
+/// string. Abstracts differences between ToString and IntoPropValue. The
+/// `rest` iterator is cloned to pre-compute the length of the String; it
+/// should be cheap to clone.
+fn build_string<'a>(first: &'a str, rest: impl Iterator<Item = &'a str> + Clone) -> String {
+    // The length of the string is known to be the length of all the
+    // components, plus one space for each element in `rest`.
+    let mut s = String::with_capacity(
+        rest.clone()
+            .map(|class| class.len())
+            .chain([first.len(), rest.size_hint().0])
+            .sum(),
+    );
+
+    s.push_str(first);
+    s.extend(rest.flat_map(|class| [" ", class]));
+    s
+}
+
 impl Classes {
     /// Creates an empty set of classes. (Does not allocate.)
+    #[inline]
     pub fn new() -> Self {
         Self {
             set: IndexSet::new(),
@@ -26,6 +45,7 @@ impl Classes {
 
     /// Creates an empty set of classes with capacity for n elements. (Does not allocate if n is
     /// zero.)
+    #[inline]
     pub fn with_capacity(n: usize) -> Self {
         Self {
             set: IndexSet::with_capacity(n),
@@ -37,7 +57,11 @@ impl Classes {
     /// If the provided class has already been added, this method will ignore it.
     pub fn push<T: Into<Self>>(&mut self, class: T) {
         let classes_to_add: Self = class.into();
-        self.set.extend(classes_to_add.set);
+        if self.is_empty() {
+            *self = classes_to_add
+        } else {
+            self.set.extend(classes_to_add.set)
+        }
     }
 
     /// Adds a class to a set.
@@ -49,18 +73,20 @@ impl Classes {
     /// # Safety
     ///
     /// This function will not split the string into multiple classes. Please do not use it unless
-    /// you are absolutely certain that the string does not contain any whitespace. Using `push()`
+    /// you are absolutely certain that the string does not contain any whitespace and it is not
-    /// is preferred.
+    /// empty. Using `push()`  is preferred.
     pub unsafe fn unchecked_push<T: Into<Cow<'static, str>>>(&mut self, class: T) {
         self.set.insert(class.into());
     }
 
     /// Check the set contains a class.
+    #[inline]
     pub fn contains<T: AsRef<str>>(&self, class: T) -> bool {
         self.set.contains(class.as_ref())
     }
 
     /// Check the set is empty.
+    #[inline]
     pub fn is_empty(&self) -> bool {
         self.set.is_empty()
     }
@@ -68,15 +94,16 @@ impl Classes {
 
 impl IntoPropValue<AttrValue> for Classes {
     #[inline]
-    fn into_prop_value(mut self) -> AttrValue {
+    fn into_prop_value(self) -> AttrValue {
-        if self.set.len() == 1 {
+        let mut classes = self.set.iter();
-            match self.set.pop() {
+
-                Some(attr) => AttrValue::Rc(Rc::from(attr)),
+        match classes.next() {
-                // SAFETY: the collection is checked to be non-empty above
+            None => AttrValue::Static(""),
-                None => unsafe { unreachable_unchecked() },
+            Some(class) if classes.len() == 0 => match *class {
-            }
+                Cow::Borrowed(class) => AttrValue::Static(class),
-        } else {
+                Cow::Owned(ref class) => AttrValue::Rc(Rc::from(class.as_str())),
-            AttrValue::Rc(Rc::from(self.to_string()))
+            },
+            Some(first) => AttrValue::Rc(Rc::from(build_string(first, classes.map(Cow::borrow)))),
         }
     }
 }
@@ -93,6 +120,7 @@ impl IntoPropValue<Option<AttrValue>> for Classes {
 }
 
 impl IntoPropValue<Classes> for &'static str {
+    #[inline]
     fn into_prop_value(self) -> Classes {
         self.into()
     }
@@ -100,11 +128,7 @@ impl IntoPropValue<Classes> for &'static str {
 
 impl<T: Into<Classes>> Extend<T> for Classes {
     fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I) {
-        let classes = iter
+        iter.into_iter().for_each(|classes| self.push(classes))
-            .into_iter()
-            .map(Into::into)
-            .flat_map(|classes| classes.set);
-        self.set.extend(classes);
     }
 }
 
@@ -120,6 +144,7 @@ impl IntoIterator for Classes {
     type IntoIter = indexmap::set::IntoIter<Cow<'static, str>>;
     type Item = Cow<'static, str>;
 
+    #[inline]
     fn into_iter(self) -> Self::IntoIter {
         self.set.into_iter()
     }
@@ -127,11 +152,11 @@ impl IntoIterator for Classes {
 
 impl ToString for Classes {
     fn to_string(&self) -> String {
-        self.set
+        let mut iter = self.set.iter().map(Cow::borrow);
-            .iter()
+
-            .map(Borrow::borrow)
+        iter.next()
-            .collect::<Vec<_>>()
+            .map(|first| build_string(first, iter))
-            .join(" ")
+            .unwrap_or_default()
     }
 }
 
@@ -153,7 +178,18 @@ impl From<&'static str> for Classes {
 
 impl From<String> for Classes {
     fn from(t: String) -> Self {
-        Self::from(&t)
+        match t.contains(|c: char| c.is_whitespace()) {
+            // If the string only contains a single class, we can just use it
+            // directly (rather than cloning it into a new string). Need to make
+            // sure it's not empty, though.
+            false => match t.is_empty() {
+                true => Self::new(),
+                false => Self {
+                    set: IndexSet::from_iter([Cow::Owned(t)]),
+                },
+            },
+            true => Self::from(&t),
+        }
     }
 }
 
@@ -198,6 +234,8 @@ impl PartialEq for Classes {
     }
 }
 
+impl Eq for Classes {}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -285,4 +323,11 @@ mod tests {
         assert!(subject.contains("foo"));
         assert!(subject.contains("bar"));
     }
+
+    #[test]
+    fn ignores_empty_string() {
+        let classes = String::from("");
+        let subject = Classes::from(classes);
+        assert!(subject.is_empty())
+    }
 }