From dee4aea264f80d92a18050a3f6218c1fc19fe6a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 13 Jul 2017 03:57:44 +0200 Subject: [PATCH] style: Remove hashes from style rules and dependencies. Dependencies are very numerous, and now we shouldn't be getting so many of them. Style rules just don't need them, so it's a waste of memory. --- components/selectors/matching.rs | 16 ++++--- components/selectors/parser.rs | 24 +++++----- .../invalidation/element/invalidation_map.rs | 47 +++++-------------- .../style/invalidation/element/invalidator.rs | 8 ++-- components/style/invalidation/stylesheets.rs | 4 +- components/style/selector_map.rs | 2 +- components/style/stylist.rs | 31 ++++++------ ports/geckolib/glue.rs | 13 +++-- 8 files changed, 61 insertions(+), 84 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 9fc5b0815a1..f65703d0578 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -164,10 +164,10 @@ pub fn matches_selector_list(selector_list: &SelectorList, -> bool where E: Element { - selector_list.0.iter().any(|selector_and_hashes| { - matches_selector(&selector_and_hashes.selector, + selector_list.0.iter().any(|selector| { + matches_selector(selector, 0, - &selector_and_hashes.hashes, + None, element, context, &mut |_, _| {}) @@ -371,7 +371,7 @@ enum SelectorMatchingResult { #[inline(always)] pub fn matches_selector(selector: &Selector, offset: usize, - hashes: &AncestorHashes, + hashes: Option<&AncestorHashes>, element: &E, context: &mut MatchingContext, flags_setter: &mut F) @@ -380,9 +380,11 @@ pub fn matches_selector(selector: &Selector, F: FnMut(&E, ElementSelectorFlags), { // Use the bloom filter to fast-reject. - if let Some(filter) = context.bloom_filter { - if !may_match::(hashes, filter) { - return false; + if let Some(hashes) = hashes { + if let Some(filter) = context.bloom_filter { + if !may_match::(hashes, filter) { + return false; + } } } diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 77d31741de6..9acdc36ff0d 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -197,7 +197,7 @@ impl SelectorAndHashes { } #[derive(PartialEq, Eq, Clone, Debug)] -pub struct SelectorList(pub Vec>); +pub struct SelectorList(pub Vec>); impl SelectorList { /// Parse a comma-separated list of Selectors. @@ -207,13 +207,13 @@ impl SelectorList { pub fn parse<'i, 't, P, E>(parser: &P, input: &mut CssParser<'i, 't>) -> Result>> where P: Parser<'i, Impl=Impl, Error=E> { - input.parse_comma_separated(|input| parse_selector(parser, input).map(SelectorAndHashes::new)) + input.parse_comma_separated(|input| parse_selector(parser, input)) .map(SelectorList) } /// Creates a SelectorList from a Vec of selectors. Used in tests. pub fn from_vec(v: Vec>) -> Self { - SelectorList(v.into_iter().map(SelectorAndHashes::new).collect()) + SelectorList(v) } } @@ -761,10 +761,10 @@ impl ToCss for SelectorList { let mut iter = self.0.iter(); let first = iter.next() .expect("Empty SelectorList, should contain at least one selector"); - first.selector.to_css(dest)?; - for selector_and_hashes in iter { + first.to_css(dest)?; + for selector in iter { dest.write_str(", ")?; - selector_and_hashes.selector.to_css(dest)?; + selector.to_css(dest)?; } Ok(()) } @@ -1761,7 +1761,7 @@ pub mod tests { let result = SelectorList::parse(parser, &mut CssParser::new(&mut parser_input)); if let Ok(ref selectors) = result { assert_eq!(selectors.0.len(), 1); - assert_eq!(selectors.0[0].selector.to_css_string(), input); + assert_eq!(selectors.0[0].to_css_string(), input); } result } @@ -2066,7 +2066,7 @@ pub mod tests { #[test] fn test_pseudo_iter() { - let selector = &parse("q::before").unwrap().0[0].selector; + let selector = &parse("q::before").unwrap().0[0]; assert!(!selector.is_universal()); let mut iter = selector.iter(); assert_eq!(iter.next(), Some(&Component::PseudoElement(PseudoElement::Before))); @@ -2080,13 +2080,13 @@ pub mod tests { #[test] fn test_universal() { - let selector = &parse("*|*::before").unwrap().0[0].selector; + let selector = &parse("*|*::before").unwrap().0[0]; assert!(selector.is_universal()); } #[test] fn test_empty_pseudo_iter() { - let selector = &parse("::before").unwrap().0[0].selector; + let selector = &parse("::before").unwrap().0[0]; assert!(selector.is_universal()); let mut iter = selector.iter(); assert_eq!(iter.next(), Some(&Component::PseudoElement(PseudoElement::Before))); @@ -2112,11 +2112,11 @@ pub mod tests { #[test] fn visitor() { let mut test_visitor = TestVisitor { seen: vec![], }; - parse(":not(:hover) ~ label").unwrap().0[0].selector.visit(&mut test_visitor); + parse(":not(:hover) ~ label").unwrap().0[0].visit(&mut test_visitor); assert!(test_visitor.seen.contains(&":hover".into())); let mut test_visitor = TestVisitor { seen: vec![], }; - parse("::before:hover").unwrap().0[0].selector.visit(&mut test_visitor); + parse("::before:hover").unwrap().0[0].visit(&mut test_visitor); assert!(test_visitor.seen.contains(&":hover".into())); } } diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index 7ea92c25d85..e5c6c63bfce 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -10,8 +10,8 @@ use element_state::ElementState; use selector_map::{MaybeCaseInsensitiveHashMap, SelectorMap, SelectorMapEntry}; use selector_parser::SelectorImpl; use selectors::attr::NamespaceConstraint; -use selectors::parser::{AncestorHashes, Combinator, Component}; -use selectors::parser::{Selector, SelectorAndHashes, SelectorIter, SelectorMethods}; +use selectors::parser::{Combinator, Component}; +use selectors::parser::{Selector, SelectorIter, SelectorMethods}; use selectors::visitor::SelectorVisitor; use smallvec::SmallVec; @@ -59,10 +59,6 @@ pub struct Dependency { /// The dependency selector. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] pub selector: Selector, - /// The ancestor hashes associated with the above selector at the given - /// offset. - #[cfg_attr(feature = "servo", ignore_heap_size_of = "No heap data")] - pub hashes: AncestorHashes, /// The offset into the selector that we should match on. pub selector_offset: usize, } @@ -188,10 +184,10 @@ impl InvalidationMap { /// Adds a selector to this `InvalidationMap`. pub fn note_selector( &mut self, - selector_and_hashes: &SelectorAndHashes, + selector: &Selector, quirks_mode: QuirksMode) { - self.collect_invalidations_for(selector_and_hashes, quirks_mode) + self.collect_invalidations_for(selector, quirks_mode) } /// Clears this map, leaving it empty. @@ -206,13 +202,12 @@ impl InvalidationMap { fn collect_invalidations_for( &mut self, - selector_and_hashes: &SelectorAndHashes, + selector: &Selector, quirks_mode: QuirksMode) { - debug!("InvalidationMap::collect_invalidations_for({:?})", - selector_and_hashes.selector); + debug!("InvalidationMap::collect_invalidations_for({:?})", selector); - let mut iter = selector_and_hashes.selector.iter(); + let mut iter = selector.iter(); let mut combinator; let mut index = 0; @@ -241,22 +236,6 @@ impl InvalidationMap { index += 1; // Account for the simple selector. } - // Reuse the bloom hashes if this is the base selector. Otherwise, - // rebuild them. - let mut hashes = None; - - let mut get_hashes = || -> AncestorHashes { - if hashes.is_none() { - hashes = Some(if sequence_start == 0 { - selector_and_hashes.hashes.clone() - } else { - let seq_iter = selector_and_hashes.selector.iter_from(sequence_start); - AncestorHashes::from_iter(seq_iter) - }); - } - hashes.clone().unwrap() - }; - self.has_id_attribute_selectors |= compound_visitor.has_id_attribute_selectors; self.has_class_attribute_selectors |= compound_visitor.has_class_attribute_selectors; @@ -265,9 +244,8 @@ impl InvalidationMap { .entry(class, quirks_mode) .or_insert_with(SelectorMap::new) .insert(Dependency { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: sequence_start, - hashes: get_hashes(), }, quirks_mode); } @@ -276,9 +254,8 @@ impl InvalidationMap { .entry(id, quirks_mode) .or_insert_with(SelectorMap::new) .insert(Dependency { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: sequence_start, - hashes: get_hashes(), }, quirks_mode); } @@ -286,9 +263,8 @@ impl InvalidationMap { self.state_affecting_selectors .insert(StateDependency { dep: Dependency { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: sequence_start, - hashes: get_hashes(), }, state: compound_visitor.state, }, quirks_mode); @@ -297,9 +273,8 @@ impl InvalidationMap { if compound_visitor.other_attributes { self.other_attribute_affecting_selectors .insert(Dependency { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: sequence_start, - hashes: get_hashes(), }, quirks_mode); } diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index ec644f5e564..7c31f4a3c53 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -762,14 +762,14 @@ impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E> let matched_then = matches_selector(&dependency.selector, dependency.selector_offset, - &dependency.hashes, + None, &self.wrapper, &mut then_context, &mut |_, _| {}); let matches_now = matches_selector(&dependency.selector, dependency.selector_offset, - &dependency.hashes, + None, &self.element, &mut now_context, &mut |_, _| {}); @@ -802,7 +802,7 @@ impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E> let matched_then = matches_selector(&dependency.selector, dependency.selector_offset, - &dependency.hashes, + None, &self.wrapper, &mut then_context, &mut |_, _| {}); @@ -811,7 +811,7 @@ impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E> let matches_now = matches_selector(&dependency.selector, dependency.selector_offset, - &dependency.hashes, + None, &self.element, &mut now_context, &mut |_, _| {}); diff --git a/components/style/invalidation/stylesheets.rs b/components/style/invalidation/stylesheets.rs index 9540482786c..19c35405676 100644 --- a/components/style/invalidation/stylesheets.rs +++ b/components/style/invalidation/stylesheets.rs @@ -282,8 +282,8 @@ impl StylesheetInvalidationSet { match *rule { Style(ref lock) => { let style_rule = lock.read_with(guard); - for selector_and_hashes in &style_rule.selectors.0 { - self.collect_scopes(&selector_and_hashes.selector); + for selector in &style_rule.selectors.0 { + self.collect_scopes(selector); if self.fully_invalid { return; } diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 6ef72d15716..8b77dd01290 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -202,7 +202,7 @@ impl SelectorMap { for rule in rules { if matches_selector(&rule.selector, 0, - &rule.hashes, + Some(&rule.hashes), element, context, flags_setter) { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index b3ff2cc9e37..0b50b6d82f1 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -27,7 +27,7 @@ use selectors::attr::NamespaceConstraint; use selectors::bloom::BloomFilter; use selectors::matching::{ElementSelectorFlags, matches_selector, MatchingContext, MatchingMode}; use selectors::matching::VisitedHandlingMode; -use selectors::parser::{AncestorHashes, Combinator, Component, Selector, SelectorAndHashes}; +use selectors::parser::{AncestorHashes, Combinator, Component, Selector}; use selectors::parser::{SelectorIter, SelectorMethods}; use selectors::sink::Push; use selectors::visitor::SelectorVisitor; @@ -478,10 +478,10 @@ impl Stylist { CssRule::Style(ref locked) => { let style_rule = locked.read_with(&guard); self.num_declarations += style_rule.block.read_with(&guard).len(); - for selector_and_hashes in &style_rule.selectors.0 { + for selector in &style_rule.selectors.0 { self.num_selectors += 1; - let map = if let Some(pseudo) = selector_and_hashes.selector.pseudo_element() { + let map = if let Some(pseudo) = selector.pseudo_element() { self.pseudos_map .entry(pseudo.canonical()) .or_insert_with(PerPseudoElementSelectorMap::new) @@ -490,25 +490,26 @@ impl Stylist { self.element_map.borrow_for_origin(&origin) }; + let hashes = AncestorHashes::new(&selector); map.insert( - Rule::new(selector_and_hashes.selector.clone(), - selector_and_hashes.hashes.clone(), + Rule::new(selector.clone(), + hashes.clone(), locked.clone(), self.rules_source_order), self.quirks_mode); - self.invalidation_map.note_selector(selector_and_hashes, self.quirks_mode); - if needs_revalidation(&selector_and_hashes.selector) { + self.invalidation_map.note_selector(selector, self.quirks_mode); + if needs_revalidation(&selector) { self.selectors_for_cache_revalidation.insert( - RevalidationSelectorAndHashes::new(&selector_and_hashes), + RevalidationSelectorAndHashes::new(&selector, &hashes), self.quirks_mode); } - selector_and_hashes.selector.visit(&mut AttributeAndStateDependencyVisitor { + selector.visit(&mut AttributeAndStateDependencyVisitor { attribute_dependencies: &mut self.attribute_dependencies, style_attribute_dependency: &mut self.style_attribute_dependency, state_dependencies: &mut self.state_dependencies, }); - selector_and_hashes.selector.visit(&mut MappedIdVisitor { + selector.visit(&mut MappedIdVisitor { mapped_ids: &mut self.mapped_ids, }); } @@ -1302,7 +1303,7 @@ impl Stylist { *element, self.quirks_mode, &mut |selector_and_hashes| { results.push(matches_selector(&selector_and_hashes.selector, selector_and_hashes.selector_offset, - &selector_and_hashes.hashes, + Some(&selector_and_hashes.hashes), element, &mut matching_context, flags_setter)); @@ -1431,12 +1432,12 @@ struct RevalidationSelectorAndHashes { } impl RevalidationSelectorAndHashes { - fn new(selector_and_hashes: &SelectorAndHashes) -> Self { + fn new(selector: &Selector, hashes: &AncestorHashes) -> Self { // We basically want to check whether the first combinator is a // pseudo-element combinator. If it is, we want to use the offset one // past it. Otherwise, our offset is 0. let mut index = 0; - let mut iter = selector_and_hashes.selector.iter(); + let mut iter = selector.iter(); // First skip over the first ComplexSelector. We can't check what sort // of combinator we have until we do that. @@ -1450,9 +1451,9 @@ impl RevalidationSelectorAndHashes { }; RevalidationSelectorAndHashes { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: offset, - hashes: selector_and_hashes.hashes.clone(), + hashes: hashes.clone(), } } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index baee4e4e490..b18b03f25d3 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1263,7 +1263,7 @@ pub extern "C" fn Servo_StyleRule_GetSelectorTextAtIndex(rule: RawServoStyleRule if index >= rule.selectors.0.len() { return; } - rule.selectors.0[index].selector.to_css(unsafe { result.as_mut().unwrap() }).unwrap(); + rule.selectors.0[index].to_css(unsafe { result.as_mut().unwrap() }).unwrap(); }) } @@ -1287,7 +1287,7 @@ pub extern "C" fn Servo_StyleRule_GetSpecificityAtIndex( *specificity = 0; return; } - *specificity = rule.selectors.0[index].selector.specificity() as u64; + *specificity = rule.selectors.0[index].specificity() as u64; }) } @@ -1302,14 +1302,14 @@ pub extern "C" fn Servo_StyleRule_SelectorMatchesElement(rule: RawServoStyleRule return false; } - let selector_and_hashes = &rule.selectors.0[index]; + let selector = &rule.selectors.0[index]; let mut matching_mode = MatchingMode::Normal; match PseudoElement::from_pseudo_type(pseudo_type) { Some(pseudo) => { // We need to make sure that the requested pseudo element type // matches the selector pseudo element type before proceeding. - match selector_and_hashes.selector.pseudo_element() { + match selector.pseudo_element() { Some(selector_pseudo) if *selector_pseudo == pseudo => { matching_mode = MatchingMode::ForStatelessPseudoElement }, @@ -1319,7 +1319,7 @@ pub extern "C" fn Servo_StyleRule_SelectorMatchesElement(rule: RawServoStyleRule None => { // Do not attempt to match if a pseudo element is requested and // this is not a pseudo element selector, or vice versa. - if selector_and_hashes.selector.has_pseudo_element() { + if selector.has_pseudo_element() { return false; } }, @@ -1327,8 +1327,7 @@ pub extern "C" fn Servo_StyleRule_SelectorMatchesElement(rule: RawServoStyleRule let element = GeckoElement(element); let mut ctx = MatchingContext::new(matching_mode, None, element.owner_document_quirks_mode()); - matches_selector(&selector_and_hashes.selector, 0, &selector_and_hashes.hashes, - &element, &mut ctx, &mut |_, _| {}) + matches_selector(selector, 0, None, &element, &mut ctx, &mut |_, _| {}) }) }