From d51d95d0c7714d443536ee098e5e9022f4fbe810 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 11 Apr 2017 14:06:11 +0800 Subject: [PATCH] Merge style_affecting_attributes_selectors with selectors_for_cache_revalidation. --- components/style/matching.rs | 28 +++------------- components/style/restyle_hints.rs | 10 +----- components/style/stylist.rs | 53 ++----------------------------- 3 files changed, 8 insertions(+), 83 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index 820a8e2f061..571a8135276 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -95,12 +95,9 @@ pub enum CacheMiss { Class, /// The presentation hints didn't match. PresHints, - /// The element and the candidate didn't match the same set of - /// sibling-affecting rules. - SiblingRules, - /// The element and the candidate didn't match the same set of style - /// affecting attribute selectors. - AttrRules, + /// The element and the candidate didn't match the same set of revalidation + /// selectors. + Revalidation, } fn element_matches_candidate(element: &E, @@ -155,13 +152,7 @@ fn element_matches_candidate(element: &E, } if !revalidate(element, candidate_element, shared_context) { - miss!(SiblingRules) - } - - if !match_same_style_affecting_attributes_rules(element, - candidate_element, - shared_context) { - miss!(AttrRules) + miss!(Revalidation) } let data = candidate_element.borrow_data().unwrap(); @@ -201,14 +192,6 @@ fn have_same_class(element: &E, element_class_attributes == *candidate.class_attributes.as_ref().unwrap() } -// TODO: These re-match the candidate every time, which is suboptimal. -#[inline] -fn match_same_style_affecting_attributes_rules(element: &E, - candidate: &E, - ctx: &SharedStyleContext) -> bool { - ctx.stylist.match_same_style_affecting_attributes_rules(element, candidate) -} - #[inline] fn revalidate(element: &E, candidate: &E, @@ -1031,8 +1014,7 @@ pub trait MatchMethods : TElement { // Too expensive failure, give up, we don't want another // one of these. CacheMiss::PresHints | - CacheMiss::SiblingRules | - CacheMiss::AttrRules => break, + CacheMiss::Revalidation => break, _ => {} } } diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index 9fe5e8a439e..594246f4838 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -490,7 +490,6 @@ struct Dependency { pub struct SelectorDependencyVisitor<'a> { dependency_set: &'a mut DependencySet, needs_cache_revalidation: bool, - affected_by_attribute: bool, } impl<'a> SelectorDependencyVisitor<'a> { @@ -499,7 +498,6 @@ impl<'a> SelectorDependencyVisitor<'a> { SelectorDependencyVisitor { dependency_set: dependency_set, needs_cache_revalidation: false, - affected_by_attribute: false, } } @@ -508,12 +506,6 @@ impl<'a> SelectorDependencyVisitor<'a> { pub fn needs_cache_revalidation(&self) -> bool { self.needs_cache_revalidation } - - /// Returns whether this visitor has known of a attribute-dependent - /// selector. - pub fn affected_by_attribute(&self) -> bool { - self.affected_by_attribute - } } impl<'a> SelectorVisitor for SelectorDependencyVisitor<'a> { @@ -537,7 +529,7 @@ impl<'a> SelectorVisitor for SelectorDependencyVisitor<'a> { let hint = combinator_to_restyle_hint(combinator); - self.affected_by_attribute |= sensitivities.attrs; + self.needs_cache_revalidation |= sensitivities.attrs; self.needs_cache_revalidation |= hint.intersects(RESTYLE_LATER_SIBLINGS); if !sensitivities.is_empty() { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 64f9df5fc44..f108d75fd76 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -112,14 +112,10 @@ pub struct Stylist { state_deps: DependencySet, /// Selectors that require explicit cache revalidation (i.e. which depend - /// on state that is not otherwise visible to the cache, like child index). + /// on state that is not otherwise visible to the cache, like attributes or + /// tree-structural state like child index and pseudos). #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] selectors_for_cache_revalidation: Vec>, - - /// Selectors in the page matching elements with non-common style-affecting - /// attributes. - #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] - style_affecting_attributes_selectors: Vec>, } /// This struct holds data which user of Stylist may want to extract @@ -172,7 +168,6 @@ impl Stylist { state_deps: DependencySet::new(), selectors_for_cache_revalidation: vec![], - style_affecting_attributes_selectors: vec![] }; SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| { @@ -227,7 +222,6 @@ impl Stylist { self.animations.clear(); self.selectors_for_cache_revalidation.clear(); - self.style_affecting_attributes_selectors.clear(); extra_data.clear_font_faces(); @@ -249,8 +243,6 @@ impl Stylist { debug!("Stylist stats:"); 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", self.state_deps.len()); @@ -308,10 +300,6 @@ impl Stylist { if visitor.needs_cache_revalidation() { self.selectors_for_cache_revalidation.push(selector.clone()); } - - if visitor.affected_by_attribute() { - self.style_affecting_attributes_selectors.push(selector.clone()); - } } } CssRule::Import(ref import) => { @@ -771,43 +759,6 @@ impl Stylist { &self.animations } - /// Whether two elements match the same not-common style-affecting attribute - /// rules. - /// - /// This is used to test elements and candidates in the style-sharing - /// candidate cache. - pub fn match_same_style_affecting_attributes_rules(&self, - element: &E, - candidate: &E) -> bool - where E: TElement, - { - use selectors::matching::StyleRelations; - use selectors::matching::matches_complex_selector; - // TODO(emilio): we can probably do better, the candidate should already - // know what rules it matches. Also, we should only match until we find - // a descendant combinator, the rest should be ok, since the parent is - // the same. - // - // 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.style_affecting_attributes_selectors.iter() { - let element_matches = - matches_complex_selector(&selector.complex_selector, element, - None, &mut StyleRelations::empty(), - &mut |_, _| {}); - let candidate_matches = - matches_complex_selector(&selector.complex_selector, candidate, - None, &mut StyleRelations::empty(), - &mut |_, _| {}); - - if element_matches != candidate_matches { - return false; - } - } - - true - } - /// Returns the rule root node. #[inline] pub fn rule_tree_root(&self) -> StrongRuleNode {