From 6515f78f6dee9b5825c7b5b9c0590ba3fb86a461 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Mon, 10 Apr 2017 12:22:33 +0800 Subject: [PATCH] Check for empty when determining what selectors to use for revalidation. --- components/style/matching.rs | 12 +++++------- components/style/restyle_hints.rs | 27 ++++++++++++++++++--------- components/style/stylist.rs | 28 +++++++++++++++------------- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index 2f2468413f5..820a8e2f061 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -154,9 +154,7 @@ fn element_matches_candidate(element: &E, miss!(PresHints) } - if !match_same_sibling_affecting_rules(element, - candidate_element, - shared_context) { + if !revalidate(element, candidate_element, shared_context) { miss!(SiblingRules) } @@ -212,10 +210,10 @@ fn match_same_style_affecting_attributes_rules(element: &E, } #[inline] -fn match_same_sibling_affecting_rules(element: &E, - candidate: &E, - ctx: &SharedStyleContext) -> bool { - ctx.stylist.match_same_sibling_affecting_rules(element, candidate) +fn revalidate(element: &E, + candidate: &E, + ctx: &SharedStyleContext) -> bool { + ctx.stylist.revalidate_entry_for_cache(element, candidate) } static STYLE_SHARING_CANDIDATE_CACHE_SIZE: usize = 8; diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index dd04079c783..9fe5e8a439e 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -400,8 +400,15 @@ fn is_attr_selector(sel: &SimpleSelector) -> bool { } } -fn is_sibling_affecting_selector(sel: &SimpleSelector) -> bool { +/// Whether a selector containing this simple selector needs to be explicitly +/// matched against both the style sharing cache entry and the candidate. +/// +/// +/// We use this for selectors that can have different matching behavior between +/// siblings that are otherwise identical as far as the cache is concerned. +fn needs_cache_revalidation(sel: &SimpleSelector) -> bool { match *sel { + SimpleSelector::Empty | SimpleSelector::FirstChild | SimpleSelector::LastChild | SimpleSelector::OnlyChild | @@ -412,6 +419,7 @@ fn is_sibling_affecting_selector(sel: &SimpleSelector) -> bool { SimpleSelector::FirstOfType | SimpleSelector::LastOfType | SimpleSelector::OnlyOfType => true, + SimpleSelector::NonTSPseudoClass(ref p) => p.state_flag().is_empty(), _ => false, } } @@ -481,7 +489,7 @@ struct Dependency { /// selector. pub struct SelectorDependencyVisitor<'a> { dependency_set: &'a mut DependencySet, - affects_siblings: bool, + needs_cache_revalidation: bool, affected_by_attribute: bool, } @@ -490,14 +498,15 @@ impl<'a> SelectorDependencyVisitor<'a> { pub fn new(dependency_set: &'a mut DependencySet) -> Self { SelectorDependencyVisitor { dependency_set: dependency_set, - affects_siblings: false, + needs_cache_revalidation: false, affected_by_attribute: false, } } - /// Returns whether this visitor has known of a sibling-dependent selector. - pub fn affects_siblings(&self) -> bool { - self.affects_siblings + /// Returns whether this visitor has encountered a simple selector that needs + /// cache revalidation. + pub fn needs_cache_revalidation(&self) -> bool { + self.needs_cache_revalidation } /// Returns whether this visitor has known of a attribute-dependent @@ -518,8 +527,8 @@ impl<'a> SelectorVisitor for SelectorDependencyVisitor<'a> { let mut sensitivities = Sensitivities::new(); for s in &selector.compound_selector { sensitivities.states.insert(selector_to_state(s)); - if !self.affects_siblings { - self.affects_siblings = is_sibling_affecting_selector(s); + if !self.needs_cache_revalidation { + self.needs_cache_revalidation = needs_cache_revalidation(s); } if !sensitivities.attrs { sensitivities.attrs = is_attr_selector(s); @@ -529,7 +538,7 @@ impl<'a> SelectorVisitor for SelectorDependencyVisitor<'a> { let hint = combinator_to_restyle_hint(combinator); self.affected_by_attribute |= sensitivities.attrs; - self.affects_siblings |= hint.intersects(RESTYLE_LATER_SIBLINGS); + self.needs_cache_revalidation |= hint.intersects(RESTYLE_LATER_SIBLINGS); if !sensitivities.is_empty() { self.dependency_set.add_dependency(Dependency { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 4731ec84cb9..64f9df5fc44 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -111,9 +111,10 @@ pub struct Stylist { /// Selector dependencies used to compute restyle hints. state_deps: DependencySet, - /// Selectors in the page affecting siblings + /// Selectors that require explicit cache revalidation (i.e. which depend + /// on state that is not otherwise visible to the cache, like child index). #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] - sibling_affecting_selectors: Vec>, + selectors_for_cache_revalidation: Vec>, /// Selectors in the page matching elements with non-common style-affecting /// attributes. @@ -170,7 +171,7 @@ impl Stylist { rule_tree: RuleTree::new(), state_deps: DependencySet::new(), - sibling_affecting_selectors: vec![], + selectors_for_cache_revalidation: vec![], style_affecting_attributes_selectors: vec![] }; @@ -225,7 +226,7 @@ impl Stylist { self.state_deps.clear(); self.animations.clear(); - self.sibling_affecting_selectors.clear(); + self.selectors_for_cache_revalidation.clear(); self.style_affecting_attributes_selectors.clear(); extra_data.clear_font_faces(); @@ -246,8 +247,8 @@ impl Stylist { } debug!("Stylist stats:"); - debug!(" - Got {} sibling-affecting selectors", - self.sibling_affecting_selectors.len()); + debug!(" - Got {} selectors for cache revalidation", + self.selectors_for_cache_revalidation.len()); debug!(" - Got {} non-common-style-attribute-affecting selectors", self.style_affecting_attributes_selectors.len()); debug!(" - Got {} deps for style-hint calculation", @@ -304,8 +305,8 @@ impl Stylist { SelectorDependencyVisitor::new(&mut self.state_deps); selector.visit(&mut visitor); - if visitor.affects_siblings() { - self.sibling_affecting_selectors.push(selector.clone()); + if visitor.needs_cache_revalidation() { + self.selectors_for_cache_revalidation.push(selector.clone()); } if visitor.affected_by_attribute() { @@ -813,12 +814,13 @@ impl Stylist { self.rule_tree.root() } - /// Returns whether two elements match the same sibling-affecting rules. + /// Returns whether two elements match the same set of revalidation- + /// requiring rules. /// /// This is also for the style sharing candidate cache. - pub fn match_same_sibling_affecting_rules(&self, - element: &E, - candidate: &E) -> bool + pub fn revalidate_entry_for_cache(&self, + element: &E, + candidate: &E) -> bool where E: TElement, { use selectors::matching::StyleRelations; @@ -828,7 +830,7 @@ impl Stylist { // // TODO(emilio): Use the bloom filter, since they contain the element's // ancestor chain and it's correct for the candidate too. - for ref selector in self.sibling_affecting_selectors.iter() { + for ref selector in self.selectors_for_cache_revalidation.iter() { let element_matches = matches_complex_selector(&selector.complex_selector, element, None, &mut StyleRelations::empty(),