Bug 1980234 - Cache DependencyInvalidationKind within Dependency for faster lookup of NormalInvalidationKind. r=dshin

Differential Revision: https://phabricator.services.mozilla.com/D259468

Author: dcrescalante

style/invalidation/element/invalidation_map.rs

@@ -71,9 +71,8 @@ pub struct Dependency {
     #[ignore_malloc_size_of = "Arc"]
     pub next: Option<ThinArc<(), Dependency>>,
 
-    /// What kind of relative selector invalidation this generates.
-    /// None if this dependency is not within a relative selector.
-    relative_kind: Option<RelativeDependencyInvalidationKind>,
+    /// What kind of selector invalidation this generates.
+    kind: DependencyInvalidationKind,
 }
 
 impl SelectorMapEntry for Dependency {
@@ -125,12 +124,64 @@ pub enum RelativeDependencyInvalidationKind {
 /// Invalidation kind merging normal and relative dependencies.
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, MallocSizeOf)]
 pub enum DependencyInvalidationKind {
+    /// This dependency is for full selector invalidation.
+    /// It is assuumed that there will be no next dependency to look for.
+    FullSelector,
     /// This dependency is a normal dependency.
     Normal(NormalDependencyInvalidationKind),
     /// This dependency is a relative dependency.
     Relative(RelativeDependencyInvalidationKind),
 }
 
+/// Return the type of normal invalidation given a selector & an offset.
+fn get_normal_invalidation_kind(selector: &Selector<SelectorImpl>, selector_offset: usize) -> DependencyInvalidationKind {
+    if selector_offset == 0 || selector.len() <= selector_offset{
+        return DependencyInvalidationKind::Normal(NormalDependencyInvalidationKind::Element);
+    }
+    let combinator = Some(selector
+        .combinator_at_match_order(selector_offset - 1)
+    );
+    DependencyInvalidationKind::Normal(
+        match combinator {
+            None => NormalDependencyInvalidationKind::Element,
+            Some(Combinator::Child) | Some(Combinator::Descendant) => {
+                NormalDependencyInvalidationKind::Descendants
+            },
+            Some(Combinator::LaterSibling) | Some(Combinator::NextSibling) => {
+                NormalDependencyInvalidationKind::Siblings
+            },
+            Some(Combinator::PseudoElement) => {
+                NormalDependencyInvalidationKind::ElementAndDescendants
+            },
+            Some(Combinator::SlotAssignment) => NormalDependencyInvalidationKind::SlottedElements,
+            Some(Combinator::Part) => NormalDependencyInvalidationKind::Parts,
+        }
+    )
+}
+
+/// Return the relative invalidation kind given a match hint
+fn get_relative_kind(match_hint: RelativeSelectorMatchHint) -> RelativeDependencyInvalidationKind{
+    match match_hint {
+        RelativeSelectorMatchHint::InChild => RelativeDependencyInvalidationKind::Parent,
+        RelativeSelectorMatchHint::InSubtree => {
+            RelativeDependencyInvalidationKind::Ancestors
+        },
+        RelativeSelectorMatchHint::InNextSibling => {
+            RelativeDependencyInvalidationKind::PrevSibling
+        },
+        RelativeSelectorMatchHint::InSibling => {
+            RelativeDependencyInvalidationKind::EarlierSibling
+        },
+        RelativeSelectorMatchHint::InNextSiblingSubtree => {
+            RelativeDependencyInvalidationKind::AncestorPrevSibling
+        },
+        RelativeSelectorMatchHint::InSiblingSubtree => {
+            RelativeDependencyInvalidationKind::AncestorEarlierSibling
+        },
+    }
+}
+
+
 impl Dependency {
     /// Creates a dummy dependency to invalidate the whole selector.
     ///
@@ -145,56 +196,22 @@ impl Dependency {
             selector_offset: selector.len() + 1,
             selector,
             next: None,
-            relative_kind: None,
+            kind: DependencyInvalidationKind::FullSelector,
         }
     }
 
-    /// Returns the combinator to the right of the partial selector this
-    /// dependency represents.
-    ///
-    /// TODO(emilio): Consider storing inline if it helps cache locality?
-    fn combinator(&self) -> Option<Combinator> {
-        if self.selector_offset == 0 {
-            return None;
-        }
-
-        Some(
-            self.selector
-                .combinator_at_match_order(self.selector_offset - 1),
-        )
-    }
-
     /// The kind of normal invalidation that this would generate. The dependency
     /// in question must be a normal dependency.
     pub fn normal_invalidation_kind(&self) -> NormalDependencyInvalidationKind {
-        debug_assert!(
-            self.relative_kind.is_none(),
-            "Querying normal invalidation kind on relative dependency."
-        );
-        match self.combinator() {
-            None => NormalDependencyInvalidationKind::Element,
-            Some(Combinator::Child) | Some(Combinator::Descendant) => {
-                NormalDependencyInvalidationKind::Descendants
-            },
-            Some(Combinator::LaterSibling) | Some(Combinator::NextSibling) => {
-                NormalDependencyInvalidationKind::Siblings
-            },
-            // TODO(emilio): We could look at the selector itself to see if it's
-            // an eager pseudo, and return only Descendants here if not.
-            Some(Combinator::PseudoElement) => {
-                NormalDependencyInvalidationKind::ElementAndDescendants
-            },
-            Some(Combinator::SlotAssignment) => NormalDependencyInvalidationKind::SlottedElements,
-            Some(Combinator::Part) => NormalDependencyInvalidationKind::Parts,
+        if let DependencyInvalidationKind::Normal(kind) = self.kind {
+            return kind;
         }
+        unreachable!("Querying normal invalidation kind on non-normal dependency.");
     }
 
     /// The kind of invalidation that this would generate.
     pub fn invalidation_kind(&self) -> DependencyInvalidationKind {
-        if let Some(kind) = self.relative_kind {
-            return DependencyInvalidationKind::Relative(kind);
-        }
-        DependencyInvalidationKind::Normal(self.normal_invalidation_kind())
+        self.kind
     }
 
     /// Is the combinator to the right of this dependency's compound selector
@@ -218,10 +235,10 @@ impl Dependency {
     /// connected through next sibling, e.g. `.a:has(> .b)`.
     pub fn dependency_is_relative_with_single_next_sibling(&self) -> bool {
         match self.invalidation_kind() {
-            DependencyInvalidationKind::Normal(_) => false,
             DependencyInvalidationKind::Relative(kind) => {
                 kind == RelativeDependencyInvalidationKind::PrevSibling
             },
+            _ => false,
         }
     }
 }
@@ -763,7 +780,7 @@ fn next_dependency(
             selector: selector.clone(),
             selector_offset,
             next: dependencies_from(previous, next_outer_dependency, next_scope_dependencies),
-            relative_kind: None,
+            kind: get_normal_invalidation_kind(selector, selector_offset),
         };
 
         Some(
@@ -799,7 +816,7 @@ impl<'a, 'b, 'c> Collector for SelectorDependencyCollector<'a, 'b, 'c> {
             selector: self.selector.clone(),
             selector_offset: offset,
             next: next,
-            relative_kind: None,
+            kind: get_normal_invalidation_kind(self.selector, offset),
         }
     }
 
@@ -1304,7 +1321,7 @@ impl<'a, 'b> Collector for RelativeSelectorDependencyCollector<'a, 'b> {
         );
         debug_assert!(
             next.as_ref()
-                .is_some_and(|d| d.slice()[0].relative_kind.is_none()),
+                .is_some_and(|d| !matches!(d.slice()[0].kind, DependencyInvalidationKind::Relative(_))),
             "Duplicate relative dependency?"
         );
         debug_assert!(
@@ -1315,24 +1332,7 @@ impl<'a, 'b> Collector for RelativeSelectorDependencyCollector<'a, 'b> {
         Dependency {
             selector: self.selector.selector.clone(),
             selector_offset: self.compound_state.offset,
-            relative_kind: Some(match self.combinator_count.get_match_hint() {
-                RelativeSelectorMatchHint::InChild => RelativeDependencyInvalidationKind::Parent,
-                RelativeSelectorMatchHint::InSubtree => {
-                    RelativeDependencyInvalidationKind::Ancestors
-                },
-                RelativeSelectorMatchHint::InNextSibling => {
-                    RelativeDependencyInvalidationKind::PrevSibling
-                },
-                RelativeSelectorMatchHint::InSibling => {
-                    RelativeDependencyInvalidationKind::EarlierSibling
-                },
-                RelativeSelectorMatchHint::InNextSiblingSubtree => {
-                    RelativeDependencyInvalidationKind::AncestorPrevSibling
-                },
-                RelativeSelectorMatchHint::InSiblingSubtree => {
-                    RelativeDependencyInvalidationKind::AncestorEarlierSibling
-                },
-            }),
+            kind: DependencyInvalidationKind::Relative(get_relative_kind(self.combinator_count.get_match_hint())),
             next: next,
         }
     }

style/invalidation/element/invalidator.rs

@@ -924,6 +924,7 @@ where
                             let n = &deps.as_ref().slice()[0];
                             let invalidation_kind = n.invalidation_kind();
                             match invalidation_kind {
+                                DependencyInvalidationKind::FullSelector => unreachable!(),
                                 DependencyInvalidationKind::Normal(_) => n,
                                 DependencyInvalidationKind::Relative(kind) => {
                                     self.processor.found_relative_selector_invalidation(

style/invalidation/element/relative_selector.rs

@@ -466,6 +466,7 @@ where
         host: Option<OpaqueElement>,
     ) {
         match dependency.invalidation_kind() {
+            DependencyInvalidationKind::FullSelector => unreachable!(),
             DependencyInvalidationKind::Normal(..) => {
                 self.dependencies
                     .entry(element)
@@ -498,6 +499,7 @@ where
         let mut result = ToInvalidate::default();
         for invalidation in self.invalidations {
             match invalidation.dependency.invalidation_kind() {
+                DependencyInvalidationKind::FullSelector => unreachable!(),
                 DependencyInvalidationKind::Normal(_) => {
                     unreachable!("Inner selector in invalidation?")
                 },
@@ -1073,7 +1075,7 @@ where
             loop {
                 debug_assert!(
                     matches!(d.invalidation_kind(), DependencyInvalidationKind::Normal(_)),
-                    "Unexpected outer relative dependency"
+                    "Unexpected dependency kind"
                 );
                 if !dependency_may_be_relevant(d, &element, false) {
                     break false;
@@ -1199,6 +1201,7 @@ where
         sibling_invalidations: &mut InvalidationVector<'a>,
     ) {
         match dependency.invalidation_kind() {
+            DependencyInvalidationKind::FullSelector => unreachable!(),
             DependencyInvalidationKind::Normal(_) => (),
             DependencyInvalidationKind::Relative(kind) => {
                 self.found_relative_selector_invalidation(element, kind, dependency);

style/invalidation/element/state_and_attributes.rs

@@ -527,7 +527,7 @@ where
                 dependency.invalidation_kind(),
                 DependencyInvalidationKind::Normal(_)
             ),
-            "Found relative selector dependency"
+            "Found unexpected dependency invalidation kind"
         );
         debug!(
             "TreeStyleInvalidator::scan_dependency({:?}, {:?})",