From 98f95a32da4b2a58db2fec829c36b85d5e671645 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 1 Jun 2017 23:30:26 -0400 Subject: [PATCH] Fix the handling of the Bloom filter in the style sharing cache. --- components/style/bloom.rs | 10 ++++++- components/style/sharing/checks.rs | 4 +-- components/style/sharing/mod.rs | 45 ++++++++++++++++++++++++------ components/style/stylist.rs | 4 +-- components/style/traversal.rs | 25 ++++++++--------- 5 files changed, 61 insertions(+), 27 deletions(-) diff --git a/components/style/bloom.rs b/components/style/bloom.rs index f5df67e6a3f7..8ffe2d2a9689 100644 --- a/components/style/bloom.rs +++ b/components/style/bloom.rs @@ -158,6 +158,14 @@ impl StyleBloom { } } + /// Get the element that represents the chain of things inserted + /// into the filter right now. That chain is the given element + /// (if any) and its ancestors. + #[inline] + pub fn current_parent(&self) -> Option { + self.elements.last().map(|el| **el) + } + /// Insert the parents of an element in the bloom filter, trying to recover /// the filter if the last element inserted doesn't match. /// @@ -185,7 +193,7 @@ impl StyleBloom { } }; - if self.elements.last().map(|el| **el) == Some(parent_element) { + if self.current_parent() == Some(parent_element) { // Ta da, cache hit, we're all done. return; } diff --git a/components/style/sharing/checks.rs b/components/style/sharing/checks.rs index 40b5140809f4..8f18d44cb220 100644 --- a/components/style/sharing/checks.rs +++ b/components/style/sharing/checks.rs @@ -6,10 +6,10 @@ //! quickly whether it's worth to share style, and whether two different //! elements can indeed share the same style. +use bloom::StyleBloom; use context::{SelectorFlagsMap, SharedStyleContext}; use dom::TElement; use element_state::*; -use selectors::bloom::BloomFilter; use selectors::matching::StyleRelations; use sharing::{StyleSharingCandidate, StyleSharingTarget}; use stylearc::Arc; @@ -102,7 +102,7 @@ pub fn have_same_state_ignoring_visitedness(element: E, pub fn revalidate(target: &mut StyleSharingTarget, candidate: &mut StyleSharingCandidate, shared_context: &SharedStyleContext, - bloom: &BloomFilter, + bloom: &StyleBloom, selector_flags_map: &mut SelectorFlagsMap) -> bool where E: TElement, diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index c72344be2010..995790b5d53b 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -7,13 +7,13 @@ use Atom; use bit_vec::BitVec; +use bloom::StyleBloom; use cache::{LRUCache, LRUCacheMutIterator}; use context::{SelectorFlagsMap, SharedStyleContext, StyleContext}; use data::{ComputedStyle, ElementData, ElementStyles}; use dom::{TElement, SendElement}; use matching::{ChildCascadeRequirement, MatchMethods}; use properties::ComputedValues; -use selectors::bloom::BloomFilter; use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode, StyleRelations}; use smallvec::SmallVec; use std::mem; @@ -95,19 +95,40 @@ impl ValidationData { } /// Computes the revalidation results if needed, and returns it. + /// Inline so we know at compile time what bloom_known_valid is. + #[inline] fn revalidation_match_results( &mut self, element: E, stylist: &Stylist, - bloom: &BloomFilter, + bloom: &StyleBloom, + bloom_known_valid: bool, flags_setter: &mut F ) -> &BitVec where E: TElement, F: FnMut(&E, ElementSelectorFlags), { if self.revalidation_match_results.is_none() { + // The bloom filter may already be set up for our element. + // If it is, use it. If not, we must be in a candidate + // (i.e. something in the cache), and the element is one + // of our cousins, not a sibling. In that case, we'll + // just do revalidation selector matching without a bloom + // filter, to avoid thrashing the filter. + let bloom_to_use = if bloom_known_valid { + debug_assert_eq!(bloom.current_parent(), + element.parent_element()); + Some(bloom.filter()) + } else { + if bloom.current_parent() == element.parent_element() { + Some(bloom.filter()) + } else { + None + } + }; self.revalidation_match_results = - Some(stylist.match_revalidation_selectors(&element, bloom, + Some(stylist.match_revalidation_selectors(&element, + bloom_to_use, flags_setter)); } @@ -149,16 +170,18 @@ impl StyleSharingCandidate { self.validation_data.pres_hints(*self.element) } - /// Get the classlist of this candidate. + /// Compute the bit vector of revalidation selector match results + /// for this candidate. fn revalidation_match_results( &mut self, stylist: &Stylist, - bloom: &BloomFilter, + bloom: &StyleBloom, ) -> &BitVec { self.validation_data.revalidation_match_results( *self.element, stylist, bloom, + /* bloom_known_valid = */ false, &mut |_, _| {}) } } @@ -204,7 +227,7 @@ impl StyleSharingTarget { fn revalidation_match_results( &mut self, stylist: &Stylist, - bloom: &BloomFilter, + bloom: &StyleBloom, selector_flags_map: &mut SelectorFlagsMap ) -> &BitVec { // It's important to set the selector flags. Otherwise, if we succeed in @@ -231,6 +254,7 @@ impl StyleSharingTarget { self.element, stylist, bloom, + /* bloom_known_valid = */ true, &mut set_selector_flags) } @@ -243,7 +267,10 @@ impl StyleSharingTarget { { let shared_context = &context.shared; let selector_flags_map = &mut context.thread_local.selector_flags; - let bloom_filter = context.thread_local.bloom_filter.filter(); + let bloom_filter = &context.thread_local.bloom_filter; + + debug_assert_eq!(bloom_filter.current_parent(), + self.element.parent_element()); let result = context.thread_local .style_sharing_candidate_cache @@ -400,7 +427,7 @@ impl StyleSharingCandidateCache { &mut self, shared_context: &SharedStyleContext, selector_flags_map: &mut SelectorFlagsMap, - bloom_filter: &BloomFilter, + bloom_filter: &StyleBloom, target: &mut StyleSharingTarget, data: &mut ElementData ) -> StyleSharingResult { @@ -498,7 +525,7 @@ impl StyleSharingCandidateCache { fn test_candidate(target: &mut StyleSharingTarget, candidate: &mut StyleSharingCandidate, shared: &SharedStyleContext, - bloom: &BloomFilter, + bloom: &StyleBloom, selector_flags_map: &mut SelectorFlagsMap) -> Result { macro_rules! miss { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 3a5a2489522f..ff7d15411e97 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1092,7 +1092,7 @@ impl Stylist { /// revalidation selectors. pub fn match_revalidation_selectors(&self, element: &E, - bloom: &BloomFilter, + bloom: Option<&BloomFilter>, flags_setter: &mut F) -> BitVec where E: TElement, @@ -1101,7 +1101,7 @@ impl Stylist { // NB: `MatchingMode` doesn't really matter, given we don't share style // between pseudos. let mut matching_context = - MatchingContext::new(MatchingMode::Normal, Some(bloom)); + MatchingContext::new(MatchingMode::Normal, bloom); // Note that, by the time we're revalidating, we're guaranteed that the // candidate and the entry have the same id, classes, and local name. diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 820bc41996ec..536ddf69f87e 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -799,19 +799,6 @@ fn compute_style(_traversal: &D, context.thread_local.statistics.elements_styled += 1; let kind = data.restyle_kind(); - // First, try the style sharing cache. If we get a match we can skip the rest - // of the work. - if let MatchAndCascade = kind { - let target = StyleSharingTarget::new(element); - let sharing_result = target.share_style_if_possible(context, data); - - if let StyleWasShared(index, had_damage) = sharing_result { - context.thread_local.statistics.styles_shared += 1; - context.thread_local.style_sharing_candidate_cache.touch(index); - return had_damage; - } - } - match kind { MatchAndCascade => { // Ensure the bloom filter is up to date. @@ -820,6 +807,18 @@ fn compute_style(_traversal: &D, traversal_data.current_dom_depth); context.thread_local.bloom_filter.assert_complete(element); + + // Now that our bloom filter is set up, try the style sharing + // cache. If we get a match we can skip the rest of the work. + let target = StyleSharingTarget::new(element); + let sharing_result = target.share_style_if_possible(context, data); + + if let StyleWasShared(index, had_damage) = sharing_result { + context.thread_local.statistics.styles_shared += 1; + context.thread_local.style_sharing_candidate_cache.touch(index); + return had_damage; + } + context.thread_local.statistics.elements_matched += 1; // Perform the matching and cascading.