Auto merge of #17172 - bzbarsky:cache-parent-level, r=emilio

Parent mismatch should not clear style sharing cache.

We can have cousins in the cache whose parent doesn't match ours, and other
cousins whose parent does.  When we encounter one of the former, that's not a
reason to stop lookin for the latter.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1369620

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix https://bugzilla.mozilla.org/show_bug.cgi?id=1369620"

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17172)
<!-- Reviewable:end -->

components/style/bloom.rs

@@ -122,6 +122,12 @@ impl<E: TElement> StyleBloom<E> {
         self.elements.is_empty()
     }
 
+    /// Returns the DOM depth of elements that can be correctly
+    /// matched against the bloom filter (that is, the number of
+    /// elements in our list).
+    pub fn matching_depth(&self) -> usize {
+        self.elements.len()
+    }
 
     /// Clears the bloom filter.
     pub fn clear(&mut self) {

components/style/matching.rs

@@ -883,12 +883,14 @@ pub trait MatchMethods : TElement {
                     .validation_data
                     .take();
 
+            let dom_depth = context.thread_local.bloom_filter.matching_depth();
             context.thread_local
                    .style_sharing_candidate_cache
                    .insert_if_possible(self,
                                        data.styles().primary.values(),
                                        primary_results.relations,
-                                       validation_data);
+                                       validation_data,
+                                       dom_depth);
         }
 
         child_cascade_requirement

components/style/sharing/mod.rs

@@ -326,15 +326,20 @@ impl<E: TElement> StyleSharingTarget<E> {
         data: &mut ElementData)
         -> StyleSharingResult
     {
+        let cache = &mut context.thread_local.style_sharing_candidate_cache;
         let shared_context = &context.shared;
         let selector_flags_map = &mut context.thread_local.selector_flags;
         let bloom_filter = &context.thread_local.bloom_filter;
 
+        if cache.dom_depth != bloom_filter.matching_depth() {
+            debug!("Can't share style, because DOM depth changed from {:?} to {:?}, element: {:?}",
+                   cache.dom_depth, bloom_filter.matching_depth(), self.element);
+            return StyleSharingResult::CannotShare;
+        }
         debug_assert_eq!(bloom_filter.current_parent(),
                          self.element.parent_element());
 
-        let result = context.thread_local
-            .style_sharing_candidate_cache
+        let result = cache
             .share_style_if_possible(shared_context,
                                      selector_flags_map,
                                      bloom_filter,
@@ -398,6 +403,10 @@ pub enum StyleSharingResult {
 /// storing nodes here temporarily is safe.
 pub struct StyleSharingCandidateCache<E: TElement> {
     cache: LRUCache<[StyleSharingCandidate<E>; STYLE_SHARING_CANDIDATE_CACHE_SIZE + 1]>,
+    /// The DOM depth we're currently at.  This is used as an optimization to
+    /// clear the cache when we change depths, since we know at that point
+    /// nothing in the cache will match.
+    dom_depth: usize,
 }
 
 impl<E: TElement> StyleSharingCandidateCache<E> {
@@ -405,6 +414,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
     pub fn new() -> Self {
         StyleSharingCandidateCache {
             cache: LRUCache::new(),
+            dom_depth: 0,
         }
     }
 
@@ -424,7 +434,8 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
                               element: &E,
                               style: &ComputedValues,
                               relations: StyleRelations,
-                              mut validation_data: ValidationData) {
+                              mut validation_data: ValidationData,
+                              dom_depth: usize) {
         use selectors::matching::AFFECTED_BY_PRESENTATIONAL_HINTS;
 
         let parent = match element.parent_element() {
@@ -467,6 +478,12 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
 
         debug!("Inserting into cache: {:?} with parent {:?}", element, parent);
 
+        if self.dom_depth != dom_depth {
+            debug!("Clearing cache because depth changed from {:?} to {:?}, element: {:?}",
+                   self.dom_depth, dom_depth, element);
+            self.clear();
+            self.dom_depth = dom_depth;
+        }
         self.cache.insert(StyleSharingCandidate {
             element: unsafe { SendElement::new(*element) },
             validation_data: validation_data,
@@ -509,7 +526,6 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
             return StyleSharingResult::CannotShare;
         }
 
-        let mut should_clear_cache = false;
         for (i, candidate) in self.iter_mut().enumerate() {
             let sharing_result =
                 Self::test_candidate(
@@ -553,11 +569,6 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
                     // Cache miss, let's see what kind of failure to decide
                     // whether we keep trying or not.
                     match miss {
-                        // Cache miss because of parent, clear the candidate cache.
-                        CacheMiss::Parent => {
-                            should_clear_cache = true;
-                            break;
-                        },
                         // Too expensive failure, give up, we don't want another
                         // one of these.
                         CacheMiss::PresHints |
@@ -571,10 +582,6 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
         debug!("{:?} Cannot share style: {} cache entries", target.element,
                self.cache.num_entries());
 
-        if should_clear_cache {
-            self.clear();
-        }
-
         StyleSharingResult::CannotShare
     }