xilem_web: Optimize modifiers (allocs mostly), and fix hydrating SVG …

…elements (#620)

With the risk of wasting time with micro-optimizations...

Honestly the diff bigger and more complex than I would like it to be for
the effect that this change does, so I'm sorry to reviewers.
FWIW, I tested all the examples with the feature "hydration" disabled
and enabled.

In effect this reduces the amount of allocs and the size of allocs of
all the modifiers (since Vec allocs at least 4 elements) by:

* Adding a size hint in modifier views, to give the actual element props
a hint how much memory is needed in the relevant modifier, so that only
one allocation occurs when the actual modifier is added/created.
* Using bitflags encoded in the `start_idx` to avoid extra 4 bytes for
just two booleans in each modifier, and also use `u16` instead of
`usize` to save another 4 bytes.
* Avoiding `updated_modifiers` allocations when building the element
(with the `CREATING` bitflag).
* Reduce allocations when rebuilding, by skipping non-changed modifiers
(before this, `Cow` was cloned, which for example with a `String` as an
attribute value resulted in an additional allocation).

This minimally increases the wasm binary size (though I'm pretty sure in
a constant fashion, roughly 500bytes compressed) in exchange for faster
creation of elements and less memory usage (in the 10k
js-framework-bench roughly 10% less memory usage, and 7-8% faster).

I've also noticed that all the kurbo SVG views weren't hydrated yet,
which I think results in buggy behavior when using it e.g. in a
`Templated` view, this is kind of drive-by fix (which I didn't want to
separate because of additional work...)

xilem_web/src/attribute.rs

@@ -26,7 +26,7 @@ pub trait WithAttributes {
     /// Sets or removes (when value is `None`) an attribute from the underlying element.
     ///
     /// When in [`View::rebuild`] this has to be invoked *after* traversing the inner `View` with [`View::rebuild`]
-    fn set_attribute(&mut self, name: CowStr, value: Option<AttributeValue>);
+    fn set_attribute(&mut self, name: &CowStr, value: &Option<AttributeValue>);
 
     // TODO first find a use-case for this...
     // fn get_attr(&self, name: &str) -> Option<&AttributeValue>;
@@ -36,25 +36,35 @@ pub trait WithAttributes {
 enum AttributeModifier {
     Remove(CowStr),
     Set(CowStr, AttributeValue),
-    EndMarker(usize),
+    EndMarker(u16),
 }
 
+const HYDRATING: u16 = 1 << 14;
+const CREATING: u16 = 1 << 15;
+const RESERVED_BIT_MASK: u16 = HYDRATING | CREATING;
+
 /// This contains all the current attributes of an [`Element`](`crate::interfaces::Element`)
 #[derive(Debug, Default)]
 pub struct Attributes {
     attribute_modifiers: Vec<AttributeModifier>,
     updated_attributes: VecMap<CowStr, ()>,
-    idx: usize, // To save some memory, this could be u16 or even u8 (but this is risky)
-    start_idx: usize, // same here
-    #[cfg(feature = "hydration")]
-    pub(crate) in_hydration: bool,
+    idx: u16,
+    /// the two most significant bits are reserved for whether this was just created (bit 15) and if it's currently being hydrated (bit 14)
+    start_idx: u16,
 }
 
-#[cfg(feature = "hydration")]
 impl Attributes {
-    pub(crate) fn new(in_hydration: bool) -> Self {
+    pub(crate) fn new(size_hint: usize, #[cfg(feature = "hydration")] in_hydration: bool) -> Self {
+        #[allow(unused_mut)]
+        let mut start_idx = CREATING;
+        #[cfg(feature = "hydration")]
+        if in_hydration {
+            start_idx |= HYDRATING;
+        }
+
         Self {
-            in_hydration,
+            attribute_modifiers: Vec::with_capacity(size_hint),
+            start_idx,
             ..Default::default()
         }
     }
@@ -116,10 +126,25 @@ fn remove_attribute(element: &web_sys::Element, name: &str) {
 impl Attributes {
     /// applies potential changes of the attributes of an element to the underlying DOM node
     pub fn apply_attribute_changes(&mut self, element: &web_sys::Element) {
-        #[cfg(feature = "hydration")]
-        if self.in_hydration {
-            self.updated_attributes.clear();
-            self.in_hydration = false;
+        if (self.start_idx & HYDRATING) == HYDRATING {
+            self.start_idx &= !RESERVED_BIT_MASK;
+            return;
+        }
+
+        if (self.start_idx & CREATING) == CREATING {
+            for modifier in self.attribute_modifiers.iter().rev() {
+                match modifier {
+                    AttributeModifier::Remove(name) => {
+                        remove_attribute(element, name);
+                    }
+                    AttributeModifier::Set(name, value) => {
+                        set_attribute(element, name, &value.serialize());
+                    }
+                    AttributeModifier::EndMarker(_) => (),
+                }
+            }
+            self.start_idx &= !RESERVED_BIT_MASK;
+            debug_assert!(self.updated_attributes.is_empty());
             return;
         }
 
@@ -145,60 +170,92 @@ impl Attributes {
 }
 
 impl WithAttributes for Attributes {
-    fn set_attribute(&mut self, name: CowStr, value: Option<AttributeValue>) {
-        let new_modifier = if let Some(value) = value {
-            AttributeModifier::Set(name.clone(), value)
-        } else {
-            AttributeModifier::Remove(name.clone())
-        };
-
-        if let Some(modifier) = self.attribute_modifiers.get_mut(self.idx) {
-            if modifier != &new_modifier {
-                if let AttributeModifier::Remove(previous_name)
-                | AttributeModifier::Set(previous_name, _) = modifier
+    fn set_attribute(&mut self, name: &CowStr, value: &Option<AttributeValue>) {
+        if (self.start_idx & RESERVED_BIT_MASK) != 0 {
+            let modifier = if let Some(value) = value {
+                AttributeModifier::Set(name.clone(), value.clone())
+            } else {
+                AttributeModifier::Remove(name.clone())
+            };
+            self.attribute_modifiers.push(modifier);
+        } else if let Some(modifier) = self.attribute_modifiers.get_mut(self.idx as usize) {
+            let dirty = match (&modifier, value) {
+                // early return if nothing has changed, avoids allocations
+                (AttributeModifier::Set(old_name, old_value), Some(new_value))
+                    if old_name == name =>
                 {
-                    if &name != previous_name {
-                        self.updated_attributes.insert(previous_name.clone(), ());
+                    if old_value == new_value {
+                        false
+                    } else {
+                        self.updated_attributes.insert(name.clone(), ());
+                        true
                     }
                 }
-                self.updated_attributes.insert(name, ());
-                *modifier = new_modifier;
+                (AttributeModifier::Remove(removed), None) if removed == name => false,
+                (AttributeModifier::Set(old_name, _), None)
+                | (AttributeModifier::Remove(old_name), Some(_))
+                    if old_name == name =>
+                {
+                    self.updated_attributes.insert(name.clone(), ());
+                    true
+                }
+                (AttributeModifier::EndMarker(_), None)
+                | (AttributeModifier::EndMarker(_), Some(_)) => {
+                    self.updated_attributes.insert(name.clone(), ());
+                    true
+                }
+                (AttributeModifier::Set(old_name, _), _)
+                | (AttributeModifier::Remove(old_name), _) => {
+                    self.updated_attributes.insert(name.clone(), ());
+                    self.updated_attributes.insert(old_name.clone(), ());
+                    true
+                }
+            };
+            if dirty {
+                *modifier = if let Some(value) = value {
+                    AttributeModifier::Set(name.clone(), value.clone())
+                } else {
+                    AttributeModifier::Remove(name.clone())
+                };
             }
             // else remove it out of updated_attributes? (because previous attributes are overwritten) not sure if worth it because potentially worse perf
         } else {
-            self.updated_attributes.insert(name, ());
+            let new_modifier = if let Some(value) = value {
+                AttributeModifier::Set(name.clone(), value.clone())
+            } else {
+                AttributeModifier::Remove(name.clone())
+            };
+            self.updated_attributes.insert(name.clone(), ());
             self.attribute_modifiers.push(new_modifier);
         }
         self.idx += 1;
     }
 
     fn rebuild_attribute_modifier(&mut self) {
         if self.idx == 0 {
-            self.start_idx = 0;
+            self.start_idx &= RESERVED_BIT_MASK;
         } else {
-            let AttributeModifier::EndMarker(start_idx) = self.attribute_modifiers[self.idx - 1]
+            let AttributeModifier::EndMarker(start_idx) =
+                self.attribute_modifiers[(self.idx - 1) as usize]
             else {
                 unreachable!("this should not happen, as either `rebuild_attribute_modifier` happens first, or follows an `mark_end_of_attribute_modifier`")
             };
             self.idx = start_idx;
-            self.start_idx = start_idx;
+            self.start_idx = start_idx | (self.start_idx & RESERVED_BIT_MASK);
         }
     }
 
     fn mark_end_of_attribute_modifier(&mut self) {
-        match self.attribute_modifiers.get_mut(self.idx) {
-            Some(AttributeModifier::EndMarker(prev_start_idx))
-                if *prev_start_idx == self.start_idx => {} // attribute modifier hasn't changed
-            Some(modifier) => {
-                *modifier = AttributeModifier::EndMarker(self.start_idx);
-            }
-            None => {
-                self.attribute_modifiers
-                    .push(AttributeModifier::EndMarker(self.start_idx));
-            }
+        let start_idx = self.start_idx & !RESERVED_BIT_MASK;
+        match self.attribute_modifiers.get_mut(self.idx as usize) {
+            Some(AttributeModifier::EndMarker(prev_start_idx)) if *prev_start_idx == start_idx => {} // attribute modifier hasn't changed
+            Some(modifier) => *modifier = AttributeModifier::EndMarker(start_idx),
+            None => self
+                .attribute_modifiers
+                .push(AttributeModifier::EndMarker(start_idx)),
         }
         self.idx += 1;
-        self.start_idx = self.idx;
+        self.start_idx = self.idx | (self.start_idx & RESERVED_BIT_MASK);
     }
 }
 
@@ -211,7 +268,7 @@ impl WithAttributes for ElementProps {
         self.attributes().mark_end_of_attribute_modifier();
     }
 
-    fn set_attribute(&mut self, name: CowStr, value: Option<AttributeValue>) {
+    fn set_attribute(&mut self, name: &CowStr, value: &Option<AttributeValue>) {
         self.attributes().set_attribute(name, value);
     }
 }
@@ -229,7 +286,7 @@ where
         self.props.mark_end_of_attribute_modifier();
     }
 
-    fn set_attribute(&mut self, name: CowStr, value: Option<AttributeValue>) {
+    fn set_attribute(&mut self, name: &CowStr, value: &Option<AttributeValue>) {
         self.props.set_attribute(name, value);
     }
 }
@@ -247,7 +304,7 @@ where
         self.props.mark_end_of_attribute_modifier();
     }
 
-    fn set_attribute(&mut self, name: CowStr, value: Option<AttributeValue>) {
+    fn set_attribute(&mut self, name: &CowStr, value: &Option<AttributeValue>) {
         self.props.set_attribute(name, value);
     }
 }
@@ -297,8 +354,9 @@ where
     type ViewState = E::ViewState;
 
     fn build(&self, ctx: &mut ViewCtx) -> (Self::Element, Self::ViewState) {
+        ctx.add_modifier_size_hint::<Attributes>(1);
         let (mut element, state) = self.el.build(ctx);
-        element.set_attribute(self.name.clone(), self.value.clone());
+        element.set_attribute(&self.name, &self.value);
         element.mark_end_of_attribute_modifier();
         (element, state)
     }
@@ -312,7 +370,7 @@ where
     ) -> Mut<'e, Self::Element> {
         element.rebuild_attribute_modifier();
         let mut element = self.el.rebuild(&prev.el, view_state, ctx, element);
-        element.set_attribute(self.name.clone(), self.value.clone());
+        element.set_attribute(&self.name, &self.value);
         element.mark_end_of_attribute_modifier();
         element
     }