From 6024357ea60d017b4b6ef0f4c03c232ef16550de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 8 Sep 2020 10:33:03 +0000
Subject: [PATCH] Bug 1663618 - Speed up custom property diffing. r=boris

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 https://github.com/bluss/indexmap/issues/153.

Differential Revision: https://phabricator.services.mozilla.com/D89434
---
 .../components/style/gecko/restyle_damage.rs  |  2 +-
 .../style/properties/properties.mako.rs       | 21 +++++++++++++++++++
 .../components/style/servo/restyle_damage.rs  |  2 +-
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/servo/components/style/gecko/restyle_damage.rs b/servo/components/style/gecko/restyle_damage.rs
index a97098363cbb..4749daea183c 100644
--- a/servo/components/style/gecko/restyle_damage.rs
+++ b/servo/components/style/gecko/restyle_damage.rs
@@ -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
diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs
index e0e0fe4cb03b..9f204a3c36d4 100644
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -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)]
diff --git a/servo/components/style/servo/restyle_damage.rs b/servo/components/style/servo/restyle_damage.rs
index cee0e486f3fa..fe17fa6198b1 100644
--- a/servo/components/style/servo/restyle_damage.rs
+++ b/servo/components/style/servo/restyle_damage.rs
@@ -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);
     }