Skip to content

Commit

Permalink
Bug 1663618 - Speed up custom property diffing. r=boris
Browse files Browse the repository at this point in the history
When entering or leaving fullscreen in youtube, we spend most of the
restyle time diffing custom properties, under IndexMap::eq.

Turns out that IndexMap equality is not order-aware, and thus you
actually need to make a hashmap lookup for each entry in the map, which
is unnecessarily inefficient.

Instead, just compare the iterators.

See indexmap-rs/indexmap#153.

Differential Revision: https://phabricator.services.mozilla.com/D89434
  • Loading branch information
emilio committed Sep 8, 2020
1 parent dbfcd87 commit 6024357
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
2 changes: 1 addition & 1 deletion servo/components/style/gecko/restyle_damage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl GeckoRestyleDamage {
&mut reset_only,
)
};
if reset_only && old_style.custom_properties() != new_style.custom_properties() {
if reset_only && !old_style.custom_properties_equal(new_style) {
// The Gecko_CalcStyleDifference call only checks the non-custom
// property structs, so we check the custom properties here. Since
// they generate no damage themselves, we can skip this check if we
Expand Down
21 changes: 21 additions & 0 deletions servo/components/style/properties/properties.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2975,6 +2975,27 @@ impl ComputedValues {
self.custom_properties.as_ref()
}

/// Returns whether we have the same custom properties as another style.
///
/// This should effectively be just:
///
/// self.custom_properties() == other.custom_properties()
///
/// But that's not really the case because IndexMap equality doesn't
/// consider ordering, which we have to account for. Also, for the same
/// reason, IndexMap equality comparisons are slower than needed.
///
/// See https://github.com/bluss/indexmap/issues/153
pub fn custom_properties_equal(&self, other: &Self) -> bool {
match (self.custom_properties(), other.custom_properties()) {
(Some(l), Some(r)) => {
l.len() == r.len() && l.iter().zip(r.iter()).all(|((k1, v1), (k2, v2))| k1 == k2 && v1 == v2)
},
(None, None) => true,
_ => false,
}
}

% for prop in data.longhands:
/// Gets the computed value of a given property.
#[inline(always)]
Expand Down
2 changes: 1 addition & 1 deletion servo/components/style/servo/restyle_damage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ fn compute_damage(old: &ComputedValues, new: &ComputedValues) -> ServoRestyleDam

// Paint worklets may depend on custom properties,
// so if they have changed we should repaint.
if old.custom_properties() != new.custom_properties() {
if !old.custom_properties_equal(new) {
damage.insert(ServoRestyleDamage::REPAINT);
}

Expand Down

0 comments on commit 6024357

Please sign in to comment.