Skip to content

Commit

Permalink
Add the ability to use non-literal string as attribute names (#2593)
Browse files Browse the repository at this point in the history 
* Use AttrValue instead of &'static str in Attributes::IndexMap

* avoid monomorphization to reduce binary size

* fmt & `#[inline(always)]`s

* more attempts to reduce the code size

* make clippy happy
  • Loading branch information
@hamza1311
hamza1311 committed Apr 12, 2022
1 parent 7377156 commit e9b64e0
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 27 deletions.
33 changes: 15 additions & 18 deletions packages/yew/src/dom_bundle/btag/attributes.rs
View file
Expand Up @@ -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<T: AccessValue> Apply for Value<T> {
type Element = T;
Expand Down Expand Up @@ -83,24 +84,20 @@ impl Apply for InputFields {

impl Attributes {
#[cold]
fn apply_diff_index_maps<'a, A, B>(
fn apply_diff_index_maps(
el: &Element,
// this makes it possible to diff `&'a IndexMap<_, A>` and `IndexMap<_, &'a A>`.
mut new_iter: impl Iterator<Item = (&'static str, &'a str)>,
new: &IndexMap<&'static str, A>,
old: &IndexMap<&'static str, B>,
) where
A: AsRef<str>,
B: AsRef<str>,
{
new: &IndexMap<AttrValue, AttrValue>,
old: &IndexMap<AttrValue, AttrValue>,
) {
let mut old_iter = old.iter();
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 {
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);
}
}
Expand All @@ -109,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);
}
}
Expand All @@ -123,6 +120,7 @@ impl Attributes {
// removed attributes
(None, Some(attr)) => {
for (key, _) in iter::once(attr).chain(old_iter) {
let key = key;
if !new.contains_key(key) {
Self::remove_attribute(el, key);
}
Expand All @@ -138,7 +136,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 {
Expand All @@ -148,7 +146,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(),
}
}

Expand Down Expand Up @@ -233,8 +231,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 {
() => {
Expand Down Expand Up @@ -263,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, 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.
Expand Down
45 changes: 38 additions & 7 deletions packages/yew/src/virtual_dom/mod.rs
View file
Expand Up @@ -39,8 +39,9 @@ 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;
use std::rc::Rc;
use std::{fmt, hint::unreachable_unchecked};
Expand All @@ -57,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,
Expand All @@ -65,25 +67,36 @@ impl Deref for AttrValue {
}
}

impl Hash for AttrValue {
#[inline(always)]
fn hash<H: Hasher>(&self, state: &mut H) {
self.as_ref().hash(state);
}
}

impl From<&'static str> for AttrValue {
#[inline(always)]
fn from(s: &'static str) -> Self {
AttrValue::Static(s)
}
}

impl From<String> for AttrValue {
#[inline(always)]
fn from(s: String) -> Self {
AttrValue::Rc(Rc::from(s))
}
}

impl From<Rc<str>> for AttrValue {
#[inline(always)]
fn from(s: Rc<str>) -> Self {
AttrValue::Rc(s)
}
}

impl From<Cow<'static, str>> for AttrValue {
#[inline(always)]
fn from(s: Cow<'static, str>) -> Self {
match s {
Cow::Borrowed(s) => s.into(),
Expand All @@ -102,11 +115,18 @@ impl Clone for AttrValue {
}

impl AsRef<str> for AttrValue {
#[inline(always)]
fn as_ref(&self) -> &str {
&*self
}
}

impl Borrow<str> for AttrValue {
fn borrow(&self) -> &str {
&*self
}
}

impl fmt::Display for AttrValue {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
Expand All @@ -117,6 +137,7 @@ impl fmt::Display for AttrValue {
}

impl PartialEq for AttrValue {
#[inline(always)]
fn eq(&self, other: &Self) -> bool {
self.as_ref() == other.as_ref()
}
Expand Down Expand Up @@ -292,7 +313,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<AttrValue, AttrValue>),
}

impl Attributes {
Expand All @@ -303,21 +324,21 @@ 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<dyn Iterator<Item = (&'static str, &'a str)> + 'a> {
pub fn iter<'a>(&'a self) -> Box<dyn Iterator<Item = (&'a str, &'a str)> + '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(
keys.iter()
.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<AttrValue, AttrValue> {
macro_rules! unpack {
() => {
match self {
Expand All @@ -331,15 +352,15 @@ 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 } => {
*self = Self::IndexMap(
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!()
Expand All @@ -348,8 +369,18 @@ impl Attributes {
}
}

impl From<IndexMap<AttrValue, AttrValue>> for Attributes {
fn from(v: IndexMap<AttrValue, AttrValue>) -> Self {
Self::IndexMap(v)
}
}

impl From<IndexMap<&'static str, AttrValue>> 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)
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/yew/src/virtual_dom/vtag.rs
View file
Expand Up @@ -351,7 +351,7 @@ impl VTag {
pub fn add_attribute(&mut self, key: &'static str, value: impl Into<AttrValue>) {
self.attributes
.get_mut_index_map()
.insert(key, value.into());
.insert(AttrValue::Static(key), value.into());
}

/// Sets attributes to a virtual node.
Expand All @@ -366,7 +366,7 @@ impl VTag {
pub fn __macro_push_attr(&mut self, key: &'static str, value: impl IntoPropValue<AttrValue>) {
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).
Expand Down

1 comment on commit e9b64e0

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yew master branch benchmarks (Lower is better)

Benchmark suite Current: e9b64e0 Previous: 7377156 Ratio
yew-struct-keyed 01_run1k 168.974 225.8875 0.75
yew-struct-keyed 02_replace1k 196.1035 224.433 0.87
yew-struct-keyed 03_update10th1k_x16 395.089 358.3595 1.10
yew-struct-keyed 04_select1k 56.093500000000006 72.30799999999999 0.78
yew-struct-keyed 05_swap1k 80.769 99.957 0.81
yew-struct-keyed 06_remove-one-1k 27.409 32.537 0.84
yew-struct-keyed 07_create10k 3070.388 3262.2275 0.94
yew-struct-keyed 08_create1k-after1k_x2 424.5605 457.603 0.93
yew-struct-keyed 09_clear1k_x8 216.015 194.5915 1.11
yew-struct-keyed 21_ready-memory 1.457233428955078 1.457233428955078 1
yew-struct-keyed 22_run-memory 1.6923751831054688 1.6921844482421875 1.00
yew-struct-keyed 23_update5-memory 1.6990699768066406 1.6954841613769531 1.00
yew-struct-keyed 24_run5-memory 1.709758758544922 1.944538116455078 0.88
yew-struct-keyed 25_run-clear-memory 1.326934814453125 1.3269538879394531 1.00
yew-struct-keyed 31_startup-ci 1734.076 1741.4520000000002 1.00
yew-struct-keyed 32_startup-bt 34.26 42.236 0.81
yew-struct-keyed 33_startup-mainthreadcost 205.404 314.76400000000007 0.65
yew-struct-keyed 34_startup-totalbytes 328.744140625 328.744140625 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.