From 2b7fb519ba200ebc471aa5ab3dad3c6c48304386 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Thu, 23 Apr 2020 19:20:17 +0000
Subject: [PATCH] style: Optimize invalidation by scanning the rightmost
 compound inside :where() and :is() with the outer visitor.

See the comment about why this is valuable. For a selector like:

    .foo:is(.bar) > .baz

Before this patch we'd generate an Dependency for .bar like this:

    Dependency {
        selector: .bar,
        offset: 0,
        parent: Some(Dependency {
            selector: .foo:is(.bar) > .baz,
            offset: 1, // Pointing to the `>` combinator.
            parent: None,
        }),
    }

After this patch we'd generate just:

    Dependency {
        selector: .foo:is(.bar) > .baz,
        offset: 1, // Pointing to the `>` combinator.
        parent: None,
    }

This is not only less memory but also less work. The reason for that is that,
before this patch, when .bar changes, we'd look the dependency, and see there's
a parent, and then scan that, so we'd match `.bar` two times, one for the
initial dependency, and one for .foo:is(.bar).

Instead, with this we'd only check `.foo:is(.bar)` once.

Differential Revision: https://phabricator.services.mozilla.com/D71423
---
 .../invalidation/element/invalidation_map.rs  | 55 ++++++++++++++-----
 1 file changed, 41 insertions(+), 14 deletions(-)

diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs
index 031407adce1..b760d865aa2 100644
--- a/components/style/invalidation/element/invalidation_map.rs
+++ b/components/style/invalidation/element/invalidation_map.rs
@@ -347,17 +347,20 @@ struct SelectorDependencyCollector<'a> {
 
 impl<'a> SelectorDependencyCollector<'a> {
     fn visit_whole_selector(&mut self) -> bool {
-        let mut iter = self.selector.iter();
-        let mut combinator;
-        let mut index = 0;
+        let iter = self.selector.iter();
+        self.visit_whole_selector_from(iter, 0)
+    }
 
+    fn visit_whole_selector_from(&mut self, mut iter: SelectorIter<SelectorImpl>, mut index: usize) -> bool {
         loop {
             // Reset the compound state.
             self.compound_state = PerCompoundState::new(index);
 
             // Visit all the simple selectors in this sequence.
             for ss in &mut iter {
-                ss.visit(self);
+                if !ss.visit(self) {
+                    return false;
+                }
                 index += 1; // Account for the simple selector.
             }
 
@@ -376,15 +379,12 @@ impl<'a> SelectorDependencyCollector<'a> {
                 }
             }
 
-            combinator = iter.next_sequence();
+            let combinator = iter.next_sequence();
             if combinator.is_none() {
-                break;
+                return true;
             }
-
-            index += 1; // Account for the combinator.
+            index += 1; // account for the combinator
         }
-
-        true
     }
 
     fn add_attr_dependency(&mut self) -> bool {
@@ -409,6 +409,11 @@ impl<'a> SelectorDependencyCollector<'a> {
         // TODO(emilio): Maybe we should refcount the parent dependencies, or
         // cache them or something.
         for &(ref selector, ref selector_offset) in self.parent_selectors.iter() {
+            debug_assert_ne!(
+                self.compound_state.offset,
+                0,
+                "Shouldn't bother creating nested dependencies for the rightmost compound",
+            );
             let new_parent = Dependency {
                 selector: selector.clone(),
                 selector_offset: *selector_offset,
@@ -429,22 +434,44 @@ impl<'a> SelectorVisitor for SelectorDependencyCollector<'a> {
     type Impl = SelectorImpl;
 
     fn visit_selector_list(&mut self, list: &[Selector<SelectorImpl>]) -> bool {
-        self.parent_selectors.push((self.selector.clone(), self.compound_state.offset));
         for selector in list {
+            // Here we cheat a bit: We can visit the rightmost compound with
+            // the "outer" visitor, and it'd be fine. This reduces the amount of
+            // state and attribute invalidations, and we need to check the outer
+            // selector to the left anyway to avoid over-invalidation, so it
+            // avoids matching it twice uselessly.
+            let mut iter = selector.iter();
+            let mut index = 0;
+
+            for ss in &mut iter {
+                if !ss.visit(self) {
+                    return false;
+                }
+                index += 1;
+            }
+
+            let combinator = iter.next_sequence();
+            if combinator.is_none() {
+                continue;
+            }
+
+            index += 1; // account for the combinator.
+
+            self.parent_selectors.push((self.selector.clone(), self.compound_state.offset));
             let mut nested = SelectorDependencyCollector {
                 map: &mut *self.map,
                 document_state: &mut *self.document_state,
                 selector,
                 parent_selectors: &mut *self.parent_selectors,
                 quirks_mode: self.quirks_mode,
-                compound_state: PerCompoundState::new(0),
+                compound_state: PerCompoundState::new(index),
                 alloc_error: &mut *self.alloc_error,
             };
-            if !nested.visit_whole_selector() {
+            if !nested.visit_whole_selector_from(iter, index) {
                 return false;
             }
+            self.parent_selectors.pop();
         }
-        self.parent_selectors.pop();
         true
     }