Auto merge of #17707 - emilio:invalidation-less-memory, r=heycam

stylo: Waste less memory in invalidation stuff and style rules.

This should help a lot with https://bugzilla.mozilla.org/show_bug.cgi?id=1380488.

<!-- 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/17707)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-07-13 04:15:15 -07:00 committed by GitHub
commit 9515abbca3
10 changed files with 71 additions and 140 deletions

View file

@ -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<SelectorImpl>,
/// 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,
}
@ -109,10 +105,6 @@ impl SelectorMapEntry for Dependency {
fn selector(&self) -> SelectorIter<SelectorImpl> {
self.selector.iter_from(self.selector_offset)
}
fn hashes(&self) -> &AncestorHashes {
&self.hashes
}
}
/// The same, but for state selectors, which can track more exactly what state
@ -128,11 +120,7 @@ pub struct StateDependency {
impl SelectorMapEntry for StateDependency {
fn selector(&self) -> SelectorIter<SelectorImpl> {
self.dep.selector.iter_from(self.dep.selector_offset)
}
fn hashes(&self) -> &AncestorHashes {
&self.dep.hashes
self.dep.selector()
}
}
@ -196,10 +184,10 @@ impl InvalidationMap {
/// Adds a selector to this `InvalidationMap`.
pub fn note_selector(
&mut self,
selector_and_hashes: &SelectorAndHashes<SelectorImpl>,
selector: &Selector<SelectorImpl>,
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.
@ -214,13 +202,12 @@ impl InvalidationMap {
fn collect_invalidations_for(
&mut self,
selector_and_hashes: &SelectorAndHashes<SelectorImpl>,
selector: &Selector<SelectorImpl>,
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;
@ -249,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;
@ -273,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);
}
@ -284,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);
}
@ -294,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);
@ -305,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);
}

View file

@ -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 |_, _| {});

View file

@ -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;
}

View file

@ -14,7 +14,7 @@ use pdqsort::sort_by;
use rule_tree::CascadeLevel;
use selector_parser::SelectorImpl;
use selectors::matching::{matches_selector, MatchingContext, ElementSelectorFlags};
use selectors::parser::{AncestorHashes, Component, Combinator, SelectorAndHashes, SelectorIter};
use selectors::parser::{Component, Combinator, SelectorIter};
use selectors::parser::LocalName as LocalNameSelector;
use smallvec::VecLike;
use std::collections::HashMap;
@ -26,19 +26,6 @@ use stylist::Rule;
pub trait SelectorMapEntry : Sized + Clone {
/// Gets the selector we should use to index in the selector map.
fn selector(&self) -> SelectorIter<SelectorImpl>;
/// Gets the ancestor hashes associated with the selector.
fn hashes(&self) -> &AncestorHashes;
}
impl SelectorMapEntry for SelectorAndHashes<SelectorImpl> {
fn selector(&self) -> SelectorIter<SelectorImpl> {
self.selector.iter()
}
fn hashes(&self) -> &AncestorHashes {
&self.hashes
}
}
/// Map element data to selector-providing objects for which the last simple
@ -65,7 +52,7 @@ impl SelectorMapEntry for SelectorAndHashes<SelectorImpl> {
/// TODO: Tune the initial capacity of the HashMap
#[derive(Debug)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct SelectorMap<T: SelectorMapEntry> {
pub struct SelectorMap<T> {
/// A hash from an ID to rules which contain that ID selector.
pub id_hash: MaybeCaseInsensitiveHashMap<Atom, Vec<T>>,
/// A hash from a class name to rules which contain that class selector.
@ -83,7 +70,7 @@ fn sort_by_key<T, F: Fn(&T) -> K, K: Ord>(v: &mut [T], f: F) {
sort_by(v, |a, b| f(a).cmp(&f(b)))
}
impl<T: SelectorMapEntry> SelectorMap<T> {
impl<T> SelectorMap<T> {
/// Trivially constructs an empty `SelectorMap`.
pub fn new() -> Self {
SelectorMap {
@ -209,7 +196,7 @@ impl SelectorMap<Rule> {
for rule in rules {
if matches_selector(&rule.selector,
0,
&rule.hashes,
Some(&rule.hashes),
element,
context,
flags_setter) {

View file

@ -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<SelectorImpl>) -> Self {
fn new(selector: &Selector<SelectorImpl>, 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(),
}
}
}
@ -1461,10 +1462,6 @@ impl SelectorMapEntry for RevalidationSelectorAndHashes {
fn selector(&self) -> SelectorIter<SelectorImpl> {
self.selector.iter_from(self.selector_offset)
}
fn hashes(&self) -> &AncestorHashes {
&self.hashes
}
}
/// Visitor to determine whether a selector requires cache revalidation.
@ -1609,10 +1606,6 @@ impl SelectorMapEntry for Rule {
fn selector(&self) -> SelectorIter<SelectorImpl> {
self.selector.iter()
}
fn hashes(&self) -> &AncestorHashes {
&self.hashes
}
}
impl Rule {