Stop slicing selectors when noting dependencies, and match with an offset instead.

MozReview-Commit-ID: KLqmdRKygO0

components/selectors/matching.rs

@@ -160,6 +160,7 @@ pub fn matches_selector_list<E>(selector_list: &SelectorList<E::Impl>,
 {
     selector_list.0.iter().any(|selector_and_hashes| {
         matches_selector(&selector_and_hashes.selector,
+                         0,
                          &selector_and_hashes.hashes,
                          element,
                          context,
@@ -333,8 +334,16 @@ enum SelectorMatchingResult {
 }
 
 /// Matches a selector, fast-rejecting against a bloom filter.
+///
+/// We accept an offset to allow consumers to represent and match against partial
+/// selectors (indexed from the right). We use this API design, rather than
+/// having the callers pass a SelectorIter, because creating a SelectorIter
+/// requires dereferencing the selector to get the length, which adds an
+/// unncessary cache miss for cases when we can fast-reject with AncestorHashes
+/// (which the caller can store inline with the selector pointer).
 #[inline(always)]
 pub fn matches_selector<E, F>(selector: &Selector<E::Impl>,
+                              offset: usize,
                               hashes: &AncestorHashes,
                               element: &E,
                               context: &mut MatchingContext,
@@ -350,11 +359,12 @@ pub fn matches_selector<E, F>(selector: &Selector<E::Impl>,
         }
     }
 
-    matches_complex_selector(selector, element, context, flags_setter)
+    matches_complex_selector(selector, offset, element, context, flags_setter)
 }
 
 /// Matches a complex selector.
 pub fn matches_complex_selector<E, F>(complex_selector: &Selector<E::Impl>,
+                                      offset: usize,
                                       element: &E,
                                       context: &mut MatchingContext,
                                       flags_setter: &mut F)
@@ -362,11 +372,15 @@ pub fn matches_complex_selector<E, F>(complex_selector: &Selector<E::Impl>,
     where E: Element,
           F: FnMut(&E, ElementSelectorFlags),
 {
-    let mut iter = complex_selector.iter();
+    let mut iter = if offset == 0 {
+        complex_selector.iter()
+    } else {
+        complex_selector.iter_from(offset)
+    };
 
     if cfg!(debug_assertions) {
         if context.matching_mode == MatchingMode::ForStatelessPseudoElement {
-            assert!(complex_selector.iter().any(|c| {
+            assert!(iter.clone().any(|c| {
                 matches!(*c, Component::PseudoElement(..))
             }));
         }

components/selectors/parser.rs

@@ -177,9 +177,13 @@ pub struct AncestorHashes(pub [u32; NUM_ANCESTOR_HASHES]);
 
 impl AncestorHashes {
     pub fn new<Impl: SelectorImpl>(s: &Selector<Impl>) -> Self {
+        Self::from_iter(s.iter())
+    }
+
+    pub fn from_iter<Impl: SelectorImpl>(iter: SelectorIter<Impl>) -> Self {
         let mut hashes = [0; NUM_ANCESTOR_HASHES];
         // Compute ancestor hashes for the bloom filter.
-        let mut hash_iter = s.iter_ancestors()
+        let mut hash_iter = AncestorIter::new(iter)
                              .map(|x| x.ancestor_hash())
                              .filter(|x| x.is_some())
                              .map(|x| x.unwrap());
@@ -356,6 +360,16 @@ impl<Impl: SelectorImpl> Selector<Impl> {
         }
     }
 
+    pub fn iter_from(&self, offset: usize) -> SelectorIter<Impl> {
+        // Note: selectors are stored left-to-right but logical order is right-to-left.
+        let slice = self.0.as_ref();
+        let iter = slice[..(slice.len() - offset)].iter().rev();
+        SelectorIter {
+            iter: iter,
+            next_combinator: None,
+        }
+    }
+
     /// Returns an iterator over the entire sequence of simple selectors and combinators,
     /// from right to left.
     pub fn iter_raw(&self) -> Rev<slice::Iter<Component<Impl>>> {
@@ -368,28 +382,6 @@ impl<Impl: SelectorImpl> Selector<Impl> {
         self.0.iter()
     }
 
-    /// Returns an iterator over ancestor simple selectors. All combinators and
-    /// non-ancestor simple selectors will be skipped.
-    pub fn iter_ancestors(&self) -> AncestorIter<Impl> {
-        AncestorIter::new(self.iter())
-    }
-
-    /// Returns a Selector identical to |self| but with the rightmost |index| entries
-    /// removed.
-    pub fn slice_from(&self, index: usize) -> Self {
-        // Note that we convert the slice_from to slice_to because selectors are
-        // stored left-to-right but logical order is right-to-left.
-        Selector(self.0.clone().slice_to(self.0.len() - index), self.1)
-    }
-
-    /// Returns a Selector identical to |self| but with the leftmost |len() - index|
-    /// entries removed.
-    pub fn slice_to(&self, index: usize) -> Self {
-        // Note that we convert the slice_to to slice_from because selectors are
-        // stored left-to-right but logical order is right-to-left.
-        Selector(self.0.clone().slice_from(self.0.len() - index), self.1)
-    }
-
     /// Creates a Selector from a vec of Components. Used in tests.
     pub fn from_vec(vec: Vec<Component<Impl>>, specificity_and_flags: u32) -> Self {
         Selector(ArcSlice::new(vec.into_boxed_slice()), specificity_and_flags)

components/style/gecko/wrapper.rs

@@ -1405,7 +1405,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
             NonTSPseudoClass::MozPlaceholder => false,
             NonTSPseudoClass::MozAny(ref sels) => {
                 sels.iter().any(|s| {
-                    matches_complex_selector(s, self, context, flags_setter)
+                    matches_complex_selector(s, 0, self, context, flags_setter)
                 })
             }
             NonTSPseudoClass::MozSystemMetric(ref s) |

components/style/restyle_hints.rs

@@ -23,7 +23,7 @@ use selectors::attr::{AttrSelectorOperation, NamespaceConstraint};
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode};
 use selectors::matching::{RelevantLinkStatus, VisitedHandlingMode, matches_selector};
 use selectors::parser::{AncestorHashes, Combinator, Component};
-use selectors::parser::{Selector, SelectorAndHashes, SelectorMethods};
+use selectors::parser::{Selector, SelectorAndHashes, SelectorIter, SelectorMethods};
 use selectors::visitor::SelectorVisitor;
 use smallvec::SmallVec;
 use std::cell::Cell;
@@ -643,7 +643,7 @@ impl<'a, E> Element for ElementWrapper<'a, E>
             use selectors::matching::matches_complex_selector;
             if let NonTSPseudoClass::MozAny(ref selectors) = *pseudo_class {
                 return selectors.iter().any(|s| {
-                    matches_complex_selector(s, self, context, _setter)
+                    matches_complex_selector(s, 0, self, context, _setter)
                 })
             }
         }
@@ -867,8 +867,13 @@ impl Sensitivities {
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct Dependency {
+    /// The dependency selector.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
     selector: Selector<SelectorImpl>,
+    /// The offset into the selector that we should match on.
+    selector_offset: usize,
+    /// The ancestor hashes associated with the above selector at the given
+    /// offset.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "No heap data")]
     hashes: AncestorHashes,
     /// The hint associated with this dependency.
@@ -878,8 +883,8 @@ pub struct Dependency {
 }
 
 impl SelectorMapEntry for Dependency {
-    fn selector(&self) -> &Selector<SelectorImpl> {
+    fn selector(&self) -> SelectorIter<SelectorImpl> {
-        &self.selector
+        self.selector.iter_from(self.selector_offset)
     }
 
     fn hashes(&self) -> &AncestorHashes {
@@ -999,19 +1004,20 @@ impl DependencySet {
                     None => RestyleHint::for_self(),
                 };
 
-                let (dep_selector, hashes) = if sequence_start == 0 {
+                // Reuse the bloom hashes if this is the base selector. Otherwise,
-                    // Reuse the bloom hashes if this is the base selector.
+                // rebuild them.
-                    (selector_and_hashes.selector.clone(), selector_and_hashes.hashes.clone())
+                let hashes = if sequence_start == 0 {
+                    selector_and_hashes.hashes.clone()
                 } else {
-                    let selector = selector_and_hashes.selector.slice_from(sequence_start);
+                    let seq_iter = selector_and_hashes.selector.iter_from(sequence_start);
-                    let hashes = AncestorHashes::new(&selector);
+                    AncestorHashes::from_iter(seq_iter)
-                    (selector, hashes)
                 };
 
                 self.dependencies.insert(Dependency {
                     sensitivities: visitor.sensitivities,
                     hint: hint,
-                    selector: dep_selector,
+                    selector: selector_and_hashes.selector.clone(),
+                    selector_offset: sequence_start,
                     hashes: hashes,
                 });
             }
@@ -1141,6 +1147,7 @@ impl DependencySet {
                                                  VisitedHandlingMode::AllLinksUnvisited);
             let matched_then =
                 matches_selector(&dep.selector,
+                                 dep.selector_offset,
                                  &dep.hashes,
                                  &snapshot_el,
                                  &mut then_context,
@@ -1150,6 +1157,7 @@ impl DependencySet {
                                                  VisitedHandlingMode::AllLinksUnvisited);
             let matches_now =
                 matches_selector(&dep.selector,
+                                 dep.selector_offset,
                                  &dep.hashes,
                                  el,
                                  &mut now_context,
@@ -1177,6 +1185,7 @@ impl DependencySet {
                 then_context.visited_handling = VisitedHandlingMode::RelevantLinkVisited;
                 let matched_then =
                     matches_selector(&dep.selector,
+                                     dep.selector_offset,
                                      &dep.hashes,
                                      &snapshot_el,
                                      &mut then_context,
@@ -1184,6 +1193,7 @@ impl DependencySet {
                 now_context.visited_handling = VisitedHandlingMode::RelevantLinkVisited;
                 let matches_now =
                     matches_selector(&dep.selector,
+                                     dep.selector_offset,
                                      &dep.hashes,
                                      el,
                                      &mut now_context,

components/style/selector_map.rs

@@ -12,7 +12,7 @@ use pdqsort::sort_by;
 use rule_tree::CascadeLevel;
 use selector_parser::SelectorImpl;
 use selectors::matching::{matches_selector, MatchingContext, ElementSelectorFlags};
-use selectors::parser::{AncestorHashes, Component, Combinator, Selector, SelectorAndHashes};
+use selectors::parser::{AncestorHashes, Component, Combinator, SelectorAndHashes, SelectorIter};
 use selectors::parser::LocalName as LocalNameSelector;
 use smallvec::VecLike;
 use std::borrow::Borrow;
@@ -23,15 +23,15 @@ use stylist::{ApplicableDeclarationBlock, Rule};
 /// A trait to abstract over a given selector map entry.
 pub trait SelectorMapEntry : Sized + Clone {
     /// Gets the selector we should use to index in the selector map.
-    fn selector(&self) -> &Selector<SelectorImpl>;
+    fn selector(&self) -> SelectorIter<SelectorImpl>;
 
     /// Gets the ancestor hashes associated with the selector.
     fn hashes(&self) -> &AncestorHashes;
 }
 
 impl SelectorMapEntry for SelectorAndHashes<SelectorImpl> {
-    fn selector(&self) -> &Selector<SelectorImpl> {
+    fn selector(&self) -> SelectorIter<SelectorImpl> {
-        &self.selector
+        self.selector.iter()
     }
 
     fn hashes(&self) -> &AncestorHashes {
@@ -232,6 +232,7 @@ impl SelectorMap<Rule> {
     {
         for rule in rules {
             if matches_selector(&rule.selector,
+                                0,
                                 &rule.hashes,
                                 element,
                                 context,
@@ -398,12 +399,12 @@ impl<T: SelectorMapEntry> SelectorMap<T> {
 ///
 /// Effectively, pseudo-elements are ignored, given only state pseudo-classes
 /// may appear before them.
-fn find_from_right<F, R>(selector: &Selector<SelectorImpl>,
+#[inline(always)]
+fn find_from_right<F, R>(mut iter: SelectorIter<SelectorImpl>,
                          mut f: F)
                          -> Option<R>
     where F: FnMut(&Component<SelectorImpl>) -> Option<R>,
 {
-    let mut iter = selector.iter();
     for ss in &mut iter {
         if let Some(r) = f(ss) {
             return Some(r)
@@ -422,9 +423,10 @@ fn find_from_right<F, R>(selector: &Selector<SelectorImpl>,
 }
 
 /// Retrieve the first ID name in the selector, or None otherwise.
-pub fn get_id_name(selector: &Selector<SelectorImpl>)
+#[inline(always)]
+pub fn get_id_name(iter: SelectorIter<SelectorImpl>)
                    -> Option<Atom> {
-    find_from_right(selector, |ss| {
+    find_from_right(iter, |ss| {
         // TODO(pradeep): Implement case-sensitivity based on the
         // document type and quirks mode.
         if let Component::ID(ref id) = *ss {
@@ -435,9 +437,10 @@ pub fn get_id_name(selector: &Selector<SelectorImpl>)
 }
 
 /// Retrieve the FIRST class name in the selector, or None otherwise.
-pub fn get_class_name(selector: &Selector<SelectorImpl>)
+#[inline(always)]
+pub fn get_class_name(iter: SelectorIter<SelectorImpl>)
                       -> Option<Atom> {
-    find_from_right(selector, |ss| {
+    find_from_right(iter, |ss| {
         // TODO(pradeep): Implement case-sensitivity based on the
         // document type and quirks mode.
         if let Component::Class(ref class) = *ss {
@@ -448,9 +451,10 @@ pub fn get_class_name(selector: &Selector<SelectorImpl>)
 }
 
 /// Retrieve the name if it is a type selector, or None otherwise.
-pub fn get_local_name(selector: &Selector<SelectorImpl>)
+#[inline(always)]
+pub fn get_local_name(iter: SelectorIter<SelectorImpl>)
                       -> Option<LocalNameSelector<SelectorImpl>> {
-    find_from_right(selector, |ss| {
+    find_from_right(iter, |ss| {
         if let Component::LocalName(ref n) = *ss {
             return Some(LocalNameSelector {
                 name: n.name.clone(),

components/style/stylist.rs

@@ -1134,6 +1134,7 @@ impl Stylist {
         let mut results = BitVec::new();
         self.selectors_for_cache_revalidation.lookup(*element, &mut |selector_and_hashes| {
             results.push(matches_selector(&selector_and_hashes.selector,
+                                          0,
                                           &selector_and_hashes.hashes,
                                           element,
                                           &mut matching_context,
@@ -1424,8 +1425,8 @@ pub struct Rule {
 }
 
 impl SelectorMapEntry for Rule {
-    fn selector(&self) -> &Selector<SelectorImpl> {
+    fn selector(&self) -> SelectorIter<SelectorImpl> {
-        &self.selector
+        self.selector.iter()
     }
 
     fn hashes(&self) -> &AncestorHashes {