Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-06-29 19:43:39 +01:00
Code Issues Projects Releases Packages Wiki Activity

style: Shrink CascadeData by turning the bloom filters into hash sets.

Browse source 
We're wasting 1kb there, which is kind of stupid.

The only advantage of using a bloom filter is that memory usage doesn't increase
even if there's a gazillion attribute selectors and such. But:

 * For IDs we already have a bunch of data structures for invalidation and such
   which key on the id, so the bloom filter would be a very minor thing.

 * For attribute selectors we don't have such a data structure, but if people
   used a gazillion attribute selectors we should!

Differential Revision: https://phabricator.services.mozilla.com/D4668
This commit is contained in:
Emilio Cobos Álvarez 2018-09-02 23:03:47 +00:00
parent 2af9264cfe
commit 40d2cbe8ed
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
1 changed files with 18 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

63
components/style/stylist.rs
View file

@ -24,11 +24,11 @@ use properties::{self, CascadeMode, ComputedValues};
use properties::{AnimationRules, PropertyDeclarationBlock}; use properties::{AnimationRules, PropertyDeclarationBlock};
use rule_cache::{RuleCache, RuleCacheConditions}; use rule_cache::{RuleCache, RuleCacheConditions};
use rule_tree::{CascadeLevel, RuleTree, ShadowCascadeOrder, StrongRuleNode, StyleSource}; use rule_tree::{CascadeLevel, RuleTree, ShadowCascadeOrder, StrongRuleNode, StyleSource};
use selector_map::{PrecomputedHashMap, SelectorMap, SelectorMapEntry}; use selector_map::{PrecomputedHashMap, PrecomputedHashSet, SelectorMap, SelectorMapEntry};
use selector_parser::{PerPseudoElementMap, PseudoElement, SelectorImpl, SnapshotMap}; use selector_parser::{PerPseudoElementMap, PseudoElement, SelectorImpl, SnapshotMap};
use selectors::NthIndexCache; use selectors::NthIndexCache;
use selectors::attr::{CaseSensitivity, NamespaceConstraint}; use selectors::attr::{CaseSensitivity, NamespaceConstraint};
use selectors::bloom::{BloomFilter, NonCountingBloomFilter}; use selectors::bloom::BloomFilter;
use selectors::matching::{matches_selector, ElementSelectorFlags, MatchingContext, MatchingMode}; use selectors::matching::{matches_selector, ElementSelectorFlags, MatchingContext, MatchingMode};
use selectors::matching::VisitedHandlingMode; use selectors::matching::VisitedHandlingMode;
use selectors::parser::{AncestorHashes, Combinator, Component, Selector}; use selectors::parser::{AncestorHashes, Combinator, Component, Selector};
@ -1396,8 +1396,7 @@ impl Stylist {
CaseSensitivity::CaseSensitive => {}, CaseSensitivity::CaseSensitive => {},
} }
let hash = id.get_hash(); self.any_applicable_rule_data(element, |data| data.mapped_ids.contains(id))
self.any_applicable_rule_data(element, |data| data.mapped_ids.might_contain_hash(hash))
} }
/// Returns the registered `@keyframes` animation for the specified name. /// Returns the registered `@keyframes` animation for the specified name.
@ -1753,11 +1752,9 @@ struct StylistSelectorVisitor<'a> {
passed_rightmost_selector: bool, passed_rightmost_selector: bool,
/// The filter with all the id's getting referenced from rightmost /// The filter with all the id's getting referenced from rightmost
/// selectors. /// selectors.
mapped_ids: &'a mut NonCountingBloomFilter, mapped_ids: &'a mut PrecomputedHashSet<Atom>,
/// The filter with the local names of attributes there are selectors for. /// The filter with the local names of attributes there are selectors for.
attribute_dependencies: &'a mut NonCountingBloomFilter, attribute_dependencies: &'a mut PrecomputedHashSet<LocalName>,
/// Whether there's any attribute selector for the [style] attribute.
style_attribute_dependency: &'a mut bool,
/// All the states selectors in the page reference. /// All the states selectors in the page reference.
state_dependencies: &'a mut ElementState, state_dependencies: &'a mut ElementState,
/// All the document states selectors in the page reference. /// All the document states selectors in the page reference.
@ -1822,13 +1819,8 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
name: &LocalName, name: &LocalName,
lower_name: &LocalName, lower_name: &LocalName,
) -> bool { ) -> bool {
if *lower_name == local_name!("style") { self.attribute_dependencies.insert(name.clone());
*self.style_attribute_dependency = true; self.attribute_dependencies.insert(lower_name.clone());
} else {
self.attribute_dependencies.insert_hash(name.get_hash());
self.attribute_dependencies
.insert_hash(lower_name.get_hash());
}
true true
} }
@ -1854,7 +1846,7 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
// //
// NOTE(emilio): See the comment regarding on when this may // NOTE(emilio): See the comment regarding on when this may
// break in visit_complex_selector. // break in visit_complex_selector.
self.mapped_ids.insert_hash(id.get_hash()); self.mapped_ids.insert(id.clone());
}, },
_ => {}, _ => {},
} }
@ -1954,18 +1946,9 @@ pub struct CascadeData {
/// The attribute local names that appear in attribute selectors. Used /// The attribute local names that appear in attribute selectors. Used
/// to avoid taking element snapshots when an irrelevant attribute changes. /// to avoid taking element snapshots when an irrelevant attribute changes.
/// (We don't bother storing the namespace, since namespaced attributes /// (We don't bother storing the namespace, since namespaced attributes are
/// are rare.) /// rare.)
#[ignore_malloc_size_of = "just an array"] attribute_dependencies: PrecomputedHashSet<Atom>,
attribute_dependencies: NonCountingBloomFilter,
/// Whether `"style"` appears in an attribute selector. This is not common,
/// and by tracking this explicitly, we can avoid taking an element snapshot
/// in the common case of style=""` changing due to modifying
/// `element.style`. (We could track this in `attribute_dependencies`, like
/// all other attributes, but we should probably not risk incorrectly
/// returning `true` for `"style"` just due to a hash collision.)
style_attribute_dependency: bool,
/// The element state bits that are relied on by selectors. Like /// The element state bits that are relied on by selectors. Like
/// `attribute_dependencies`, this is used to avoid taking element snapshots /// `attribute_dependencies`, this is used to avoid taking element snapshots
@ -1981,8 +1964,7 @@ pub struct CascadeData {
/// hence in our selector maps). Used to determine when sharing styles is /// hence in our selector maps). Used to determine when sharing styles is
/// safe: we disallow style sharing for elements whose id matches this /// safe: we disallow style sharing for elements whose id matches this
/// filter, and hence might be in one of our selector maps. /// filter, and hence might be in one of our selector maps.
#[ignore_malloc_size_of = "just an array"] mapped_ids: PrecomputedHashSet<LocalName>,
mapped_ids: NonCountingBloomFilter,
/// Selectors that require explicit cache revalidation (i.e. which depend /// Selectors that require explicit cache revalidation (i.e. which depend
/// on state that is not otherwise visible to the cache, like attributes or /// on state that is not otherwise visible to the cache, like attributes or
@ -2019,11 +2001,10 @@ impl CascadeData {
host_rules: None, host_rules: None,
slotted_rules: None, slotted_rules: None,
invalidation_map: InvalidationMap::new(), invalidation_map: InvalidationMap::new(),
attribute_dependencies: NonCountingBloomFilter::new(), attribute_dependencies: PrecomputedHashSet::default(),
style_attribute_dependency: false,
state_dependencies: ElementState::empty(), state_dependencies: ElementState::empty(),
document_state_dependencies: DocumentState::empty(), document_state_dependencies: DocumentState::empty(),
mapped_ids: NonCountingBloomFilter::new(), mapped_ids: PrecomputedHashSet::default(),
selectors_for_cache_revalidation: SelectorMap::new(), selectors_for_cache_revalidation: SelectorMap::new(),
animations: Default::default(), animations: Default::default(),
extra_data: ExtraStyleData::default(), extra_data: ExtraStyleData::default(),
@ -2088,13 +2069,9 @@ impl CascadeData {
/// selector of some rule. /// selector of some rule.
#[inline] #[inline]
pub fn might_have_attribute_dependency(&self, local_name: &LocalName) -> bool { pub fn might_have_attribute_dependency(&self, local_name: &LocalName) -> bool {
if *local_name == local_name!("style") { self.attribute_dependencies.contains(local_name)
return self.style_attribute_dependency;
}
self.attribute_dependencies
.might_contain_hash(local_name.get_hash())
} }
#[inline] #[inline]
fn normal_rules(&self, pseudo: Option<&PseudoElement>) -> Option<&SelectorMap<Rule>> { fn normal_rules(&self, pseudo: Option<&PseudoElement>) -> Option<&SelectorMap<Rule>> {
self.normal_rules.rules(pseudo) self.normal_rules.rules(pseudo)
@ -2221,7 +2198,6 @@ impl CascadeData {
needs_revalidation: false, needs_revalidation: false,
passed_rightmost_selector: false, passed_rightmost_selector: false,
attribute_dependencies: &mut self.attribute_dependencies, attribute_dependencies: &mut self.attribute_dependencies,
style_attribute_dependency: &mut self.style_attribute_dependency,
state_dependencies: &mut self.state_dependencies, state_dependencies: &mut self.state_dependencies,
document_state_dependencies: &mut self.document_state_dependencies, document_state_dependencies: &mut self.document_state_dependencies,
mapped_ids: &mut self.mapped_ids, mapped_ids: &mut self.mapped_ids,
@ -2431,7 +2407,6 @@ impl CascadeData {
self.clear_cascade_data(); self.clear_cascade_data();
self.invalidation_map.clear(); self.invalidation_map.clear();
self.attribute_dependencies.clear(); self.attribute_dependencies.clear();
self.style_attribute_dependency = false;
self.state_dependencies = ElementState::empty(); self.state_dependencies = ElementState::empty();
self.document_state_dependencies = DocumentState::empty(); self.document_state_dependencies = DocumentState::empty();
self.mapped_ids.clear(); self.mapped_ids.clear();
@ -2527,16 +2502,14 @@ impl Rule {
/// A function to be able to test the revalidation stuff. /// A function to be able to test the revalidation stuff.
pub fn needs_revalidation_for_testing(s: &Selector<SelectorImpl>) -> bool { pub fn needs_revalidation_for_testing(s: &Selector<SelectorImpl>) -> bool {
let mut attribute_dependencies = NonCountingBloomFilter::new(); let mut attribute_dependencies = Default::default();
let mut mapped_ids = NonCountingBloomFilter::new(); let mut mapped_ids = Default::default();
let mut style_attribute_dependency = false;
let mut state_dependencies = ElementState::empty(); let mut state_dependencies = ElementState::empty();
let mut document_state_dependencies = DocumentState::empty(); let mut document_state_dependencies = DocumentState::empty();
let mut visitor = StylistSelectorVisitor { let mut visitor = StylistSelectorVisitor {
needs_revalidation: false, needs_revalidation: false,
passed_rightmost_selector: false, passed_rightmost_selector: false,
attribute_dependencies: &mut attribute_dependencies, attribute_dependencies: &mut attribute_dependencies,
style_attribute_dependency: &mut style_attribute_dependency,
state_dependencies: &mut state_dependencies, state_dependencies: &mut state_dependencies,
document_state_dependencies: &mut document_state_dependencies, document_state_dependencies: &mut document_state_dependencies,
mapped_ids: &mut mapped_ids, mapped_ids: &mut mapped_ids,