From 9f98b7f2126840b77dfddca63267806d70556e0d Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Fri, 8 Apr 2022 21:28:54 +0500 Subject: [PATCH 1/5] Use AttrValue instead of &'static str in Attributes::IndexMap --- .../yew/src/dom_bundle/btag/attributes.rs | 28 ++++++++++------ packages/yew/src/virtual_dom/mod.rs | 32 +++++++++++++++---- packages/yew/src/virtual_dom/vtag.rs | 4 +-- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index 9d5ded00c37..c3172c315d6 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -86,18 +86,26 @@ impl Attributes { fn apply_diff_index_maps<'a, A, B>( el: &Element, // this makes it possible to diff `&'a IndexMap<_, A>` and `IndexMap<_, &'a A>`. - mut new_iter: impl Iterator, - new: &IndexMap<&'static str, A>, - old: &IndexMap<&'static str, B>, + mut new_iter: impl Iterator, + new: &IndexMap, A>, + old: &IndexMap, B>, ) where A: AsRef, B: AsRef, { let mut old_iter = old.iter(); + let new = new + .iter() + .map(|(k, v)| (k.as_ref(), v)) + .collect::>(); + let old = old + .iter() + .map(|(k, v)| (k.as_ref(), v)) + .collect::>(); loop { match (new_iter.next(), old_iter.next()) { (Some((new_key, new_value)), Some((old_key, old_value))) => { - if new_key != *old_key { + if new_key != old_key.as_ref() { break; } if new_value != old_value.as_ref() { @@ -123,6 +131,7 @@ impl Attributes { // removed attributes (None, Some(attr)) => { for (key, _) in iter::once(attr).chain(old_iter) { + let key = key.as_ref(); if !new.contains_key(key) { Self::remove_attribute(el, key); } @@ -138,7 +147,7 @@ impl Attributes { /// Works with any [Attributes] variants. #[cold] fn apply_diff_as_maps<'a>(el: &Element, new: &'a Self, old: &'a Self) { - fn collect<'a>(src: &'a Attributes) -> HashMap<&'static str, &'a str> { + fn collect(src: &Attributes) -> HashMap<&str, &str> { use Attributes::*; match src { @@ -148,7 +157,7 @@ impl Attributes { .zip(values.iter()) .filter_map(|(k, v)| v.as_ref().map(|v| (*k, v.as_ref()))) .collect(), - IndexMap(m) => m.iter().map(|(k, v)| (*k, v.as_ref())).collect(), + IndexMap(m) => m.iter().map(|(k, v)| (k.as_ref(), v.as_ref())).collect(), } } @@ -233,8 +242,8 @@ impl Apply for Attributes { }, ) if ptr_eq(new_k, old_k) => { // Double zipping does not optimize well, so use asserts and unsafe instead - assert!(new_k.len() == new_v.len()); - assert!(new_k.len() == old_v.len()); + assert_eq!(new_k.len(), new_v.len()); + assert_eq!(new_k.len(), old_v.len()); for i in 0..new_k.len() { macro_rules! key { () => { @@ -263,8 +272,9 @@ impl Apply for Attributes { } // For VTag's constructed outside the html! macro (Self::IndexMap(new), Self::IndexMap(ref old)) => { - let new_iter = new.iter().map(|(k, v)| (*k, v.as_ref())); + let new_iter = new.iter().map(|(k, v)| (k.as_ref(), v.as_ref())); Self::apply_diff_index_maps(el, new_iter, new, old); + todo!(); } // Cold path. Happens only with conditional swapping and reordering of `VTag`s with the // same tag and no keys. diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index b1e43204cba..437f4c305f5 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -41,6 +41,7 @@ pub use self::vtext::VText; use indexmap::IndexMap; use std::borrow::Cow; use std::fmt::Formatter; +use std::hash::{Hash, Hasher}; use std::ops::Deref; use std::rc::Rc; use std::{fmt, hint::unreachable_unchecked}; @@ -65,6 +66,15 @@ impl Deref for AttrValue { } } +impl Hash for AttrValue { + fn hash(&self, state: &mut H) { + match self { + AttrValue::Static(s) => s.hash(&mut *state), + AttrValue::Rc(s) => s.hash(&mut *state), + }; + } +} + impl From<&'static str> for AttrValue { fn from(s: &'static str) -> Self { AttrValue::Static(s) @@ -292,7 +302,7 @@ pub enum Attributes { /// IndexMap is used to provide runtime attribute deduplication in cases where the html! macro /// was not used to guarantee it. - IndexMap(IndexMap<&'static str, AttrValue>), + IndexMap(IndexMap), } impl Attributes { @@ -303,7 +313,7 @@ impl Attributes { /// Return iterator over attribute key-value pairs. /// This function is suboptimal and does not inline well. Avoid on hot paths. - pub fn iter<'a>(&'a self) -> Box + 'a> { + pub fn iter<'a>(&'a self) -> Box + 'a> { match self { Self::Static(arr) => Box::new(arr.iter().map(|kv| (kv[0], kv[1] as &'a str))), Self::Dynamic { keys, values } => Box::new( @@ -311,13 +321,13 @@ impl Attributes { .zip(values.iter()) .filter_map(|(k, v)| v.as_ref().map(|v| (*k, v.as_ref()))), ), - Self::IndexMap(m) => Box::new(m.iter().map(|(k, v)| (*k, v.as_ref()))), + Self::IndexMap(m) => Box::new(m.iter().map(|(k, v)| (k.as_ref(), v.as_ref()))), } } /// Get a mutable reference to the underlying `IndexMap`. /// If the attributes are stored in the `Vec` variant, it will be converted. - pub fn get_mut_index_map(&mut self) -> &mut IndexMap<&'static str, AttrValue> { + pub fn get_mut_index_map(&mut self) -> &mut IndexMap { macro_rules! unpack { () => { match self { @@ -331,7 +341,7 @@ impl Attributes { match self { Self::IndexMap(m) => m, Self::Static(arr) => { - *self = Self::IndexMap(arr.iter().map(|kv| (kv[0], kv[1].into())).collect()); + *self = Self::IndexMap(arr.iter().map(|kv| (kv[0].into(), kv[1].into())).collect()); unpack!() } Self::Dynamic { keys, values } => { @@ -339,7 +349,7 @@ impl Attributes { std::mem::take(values) .iter_mut() .zip(keys.iter()) - .filter_map(|(v, k)| v.take().map(|v| (*k, v))) + .filter_map(|(v, k)| v.take().map(|v| (AttrValue::from(*k), v))) .collect(), ); unpack!() @@ -348,8 +358,18 @@ impl Attributes { } } +impl From> for Attributes { + fn from(v: IndexMap) -> Self { + Self::IndexMap(v) + } +} + impl From> for Attributes { fn from(v: IndexMap<&'static str, AttrValue>) -> Self { + let v = v + .into_iter() + .map(|(k, v)| (AttrValue::Static(k), v)) + .collect(); Self::IndexMap(v) } } diff --git a/packages/yew/src/virtual_dom/vtag.rs b/packages/yew/src/virtual_dom/vtag.rs index eb064c5d156..21e26b7a354 100644 --- a/packages/yew/src/virtual_dom/vtag.rs +++ b/packages/yew/src/virtual_dom/vtag.rs @@ -351,7 +351,7 @@ impl VTag { pub fn add_attribute(&mut self, key: &'static str, value: impl Into) { self.attributes .get_mut_index_map() - .insert(key, value.into()); + .insert(AttrValue::Static(key), value.into()); } /// Sets attributes to a virtual node. @@ -366,7 +366,7 @@ impl VTag { pub fn __macro_push_attr(&mut self, key: &'static str, value: impl IntoPropValue) { self.attributes .get_mut_index_map() - .insert(key, value.into_prop_value()); + .insert(AttrValue::from(key), value.into_prop_value()); } /// Add event listener on the [VTag]'s [Element](web_sys::Element). From 6a247806e27f4b871d7c4e1546057b2a96c9caca Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Tue, 12 Apr 2022 00:41:15 +0500 Subject: [PATCH 2/5] avoid monomorphization to reduce binary size --- packages/yew/src/dom_bundle/btag/attributes.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index c3172c315d6..bae6ee36a66 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -7,6 +7,7 @@ use std::collections::HashMap; use std::iter; use std::ops::Deref; use web_sys::{Element, HtmlInputElement as InputElement, HtmlTextAreaElement as TextAreaElement}; +use yew::AttrValue; impl Apply for Value { type Element = T; @@ -83,15 +84,13 @@ impl Apply for InputFields { impl Attributes { #[cold] - fn apply_diff_index_maps<'a, A, B>( + fn apply_diff_index_maps<'a>( el: &Element, // this makes it possible to diff `&'a IndexMap<_, A>` and `IndexMap<_, &'a A>`. mut new_iter: impl Iterator, - new: &IndexMap, A>, - old: &IndexMap, B>, - ) where - A: AsRef, - B: AsRef, + new: &IndexMap, + old: &IndexMap, + ) { let mut old_iter = old.iter(); let new = new @@ -273,8 +272,7 @@ impl Apply for Attributes { // For VTag's constructed outside the html! macro (Self::IndexMap(new), Self::IndexMap(ref old)) => { let new_iter = new.iter().map(|(k, v)| (k.as_ref(), v.as_ref())); - Self::apply_diff_index_maps(el, new_iter, new, old); - todo!(); + Self::apply_diff_index_maps(el, new_iter, &*new, &*old); } // Cold path. Happens only with conditional swapping and reordering of `VTag`s with the // same tag and no keys. From 6b1a1fcfdca5fd9f51cbeb0eb4db24ab392bc04a Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Tue, 12 Apr 2022 01:14:17 +0500 Subject: [PATCH 3/5] fmt & `#[inline(always)]`s --- packages/yew/src/dom_bundle/btag/attributes.rs | 3 +-- packages/yew/src/virtual_dom/mod.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index bae6ee36a66..1c44713d0f2 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -90,8 +90,7 @@ impl Attributes { mut new_iter: impl Iterator, new: &IndexMap, old: &IndexMap, - ) - { + ) { let mut old_iter = old.iter(); let new = new .iter() diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index 437f4c305f5..a5a7df90d06 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -58,6 +58,7 @@ pub enum AttrValue { impl Deref for AttrValue { type Target = str; + #[inline(always)] fn deref(&self) -> &Self::Target { match self { AttrValue::Static(s) => *s, @@ -67,6 +68,7 @@ impl Deref for AttrValue { } impl Hash for AttrValue { + #[inline(always)] fn hash(&self, state: &mut H) { match self { AttrValue::Static(s) => s.hash(&mut *state), @@ -76,24 +78,28 @@ impl Hash for AttrValue { } impl From<&'static str> for AttrValue { + #[inline(always)] fn from(s: &'static str) -> Self { AttrValue::Static(s) } } impl From for AttrValue { + #[inline(always)] fn from(s: String) -> Self { AttrValue::Rc(Rc::from(s)) } } impl From> for AttrValue { + #[inline(always)] fn from(s: Rc) -> Self { AttrValue::Rc(s) } } impl From> for AttrValue { + #[inline(always)] fn from(s: Cow<'static, str>) -> Self { match s { Cow::Borrowed(s) => s.into(), @@ -112,6 +118,7 @@ impl Clone for AttrValue { } impl AsRef for AttrValue { + #[inline(always)] fn as_ref(&self) -> &str { &*self } @@ -127,6 +134,7 @@ impl fmt::Display for AttrValue { } impl PartialEq for AttrValue { + #[inline(always)] fn eq(&self, other: &Self) -> bool { self.as_ref() == other.as_ref() } From d621406e7050c21770656cf0c1fa120316bc84cb Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Tue, 12 Apr 2022 22:46:49 +0500 Subject: [PATCH 4/5] more attempts to reduce the code size --- .../yew/src/dom_bundle/btag/attributes.rs | 22 +++++-------------- packages/yew/src/virtual_dom/mod.rs | 13 ++++++----- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index 1c44713d0f2..99ed61d00d1 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -86,27 +86,18 @@ impl Attributes { #[cold] fn apply_diff_index_maps<'a>( el: &Element, - // this makes it possible to diff `&'a IndexMap<_, A>` and `IndexMap<_, &'a A>`. - mut new_iter: impl Iterator, new: &IndexMap, old: &IndexMap, ) { let mut old_iter = old.iter(); - let new = new - .iter() - .map(|(k, v)| (k.as_ref(), v)) - .collect::>(); - let old = old - .iter() - .map(|(k, v)| (k.as_ref(), v)) - .collect::>(); + let mut new_iter = new.iter(); loop { match (new_iter.next(), old_iter.next()) { (Some((new_key, new_value)), Some((old_key, old_value))) => { - if new_key != old_key.as_ref() { + if new_key != old_key { break; } - if new_value != old_value.as_ref() { + if new_value != old_value { Self::set_attribute(el, new_key, new_value); } } @@ -115,7 +106,7 @@ impl Attributes { for (key, value) in iter::once(attr).chain(new_iter) { match old.get(key) { Some(old_value) => { - if value != old_value.as_ref() { + if value != old_value { Self::set_attribute(el, key, value); } } @@ -129,7 +120,7 @@ impl Attributes { // removed attributes (None, Some(attr)) => { for (key, _) in iter::once(attr).chain(old_iter) { - let key = key.as_ref(); + let key = key; if !new.contains_key(key) { Self::remove_attribute(el, key); } @@ -270,8 +261,7 @@ impl Apply for Attributes { } // For VTag's constructed outside the html! macro (Self::IndexMap(new), Self::IndexMap(ref old)) => { - let new_iter = new.iter().map(|(k, v)| (k.as_ref(), v.as_ref())); - Self::apply_diff_index_maps(el, new_iter, &*new, &*old); + Self::apply_diff_index_maps(el, &*new, &*old); } // Cold path. Happens only with conditional swapping and reordering of `VTag`s with the // same tag and no keys. diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index a5a7df90d06..f33e3fb614d 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -39,7 +39,7 @@ pub use self::vtag::VTag; pub use self::vtext::VText; use indexmap::IndexMap; -use std::borrow::Cow; +use std::borrow::{Borrow, Cow}; use std::fmt::Formatter; use std::hash::{Hash, Hasher}; use std::ops::Deref; @@ -70,10 +70,7 @@ impl Deref for AttrValue { impl Hash for AttrValue { #[inline(always)] fn hash(&self, state: &mut H) { - match self { - AttrValue::Static(s) => s.hash(&mut *state), - AttrValue::Rc(s) => s.hash(&mut *state), - }; + self.as_ref().hash(state); } } @@ -124,6 +121,12 @@ impl AsRef for AttrValue { } } +impl Borrow for AttrValue { + fn borrow(&self) -> &str { + &*self + } +} + impl fmt::Display for AttrValue { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { From 613927461a09076469b547caa12f999e33ac0c2d Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Tue, 12 Apr 2022 22:56:20 +0500 Subject: [PATCH 5/5] make clippy happy --- packages/yew/src/dom_bundle/btag/attributes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index 99ed61d00d1..587f31b1edf 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -84,7 +84,7 @@ impl Apply for InputFields { impl Attributes { #[cold] - fn apply_diff_index_maps<'a>( + fn apply_diff_index_maps( el: &Element, new: &IndexMap, old: &IndexMap,