diff options
| -rw-r--r-- | components/style/bloom.rs | 13 | ||||
| -rw-r--r-- | components/style/gecko/selector_parser.rs | 3 | ||||
| -rw-r--r-- | components/style/selector_map.rs | 110 | ||||
| -rw-r--r-- | components/style/sharing/mod.rs | 5 | ||||
| -rw-r--r-- | components/style/stylist.rs | 10 |
5 files changed, 110 insertions, 31 deletions
diff --git a/components/style/bloom.rs b/components/style/bloom.rs index 1840c780506..87fd0cba5c2 100644 --- a/components/style/bloom.rs +++ b/components/style/bloom.rs @@ -126,14 +126,11 @@ where element.each_class(|class| f(class.get_hash())); - #[cfg(feature = "gecko")] - if static_prefs::pref!("layout.css.bloom-filter-attribute-names.enabled") { - element.each_attr_name(|name| { - if !is_attr_name_excluded_from_filter(name) { - f(name.get_hash()) - } - }); - } + element.each_attr_name(|name| { + if !is_attr_name_excluded_from_filter(name) { + f(name.get_hash()) + } + }); } impl<E: TElement> Drop for StyleBloom<E> { diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 5379454daa0..7028d554aa2 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -249,8 +249,7 @@ impl ::selectors::SelectorImpl for SelectorImpl { type NonTSPseudoClass = NonTSPseudoClass; fn should_collect_attr_hash(name: &AtomIdent) -> bool { - static_prefs::pref!("layout.css.bloom-filter-attribute-names.enabled") && - !crate::bloom::is_attr_name_excluded_from_filter(name) + !crate::bloom::is_attr_name_excluded_from_filter(name) } } diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 40806ed47af..48ab5cdbe66 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -104,10 +104,14 @@ pub struct SelectorMap<T: 'static> { pub class_hash: MaybeCaseInsensitiveHashMap<Atom, SmallVec<[T; 1]>>, /// A hash from local name to rules which contain that local name selector. pub local_name_hash: PrecomputedHashMap<LocalName, SmallVec<[T; 1]>>, + /// A hash from attributes to rules which contain that attribute selector. + pub attribute_hash: PrecomputedHashMap<LocalName, SmallVec<[T; 1]>>, /// A hash from namespace to rules which contain that namespace selector. pub namespace_hash: PrecomputedHashMap<Namespace, SmallVec<[T; 1]>>, /// All other rules. pub other: SmallVec<[T; 1]>, + /// Whether we should bucket by attribute names. + bucket_attributes: bool, /// The number of entries in this map. pub count: usize, } @@ -129,18 +133,29 @@ impl<T: 'static> SelectorMap<T> { root: SmallVec::new(), id_hash: MaybeCaseInsensitiveHashMap::new(), class_hash: MaybeCaseInsensitiveHashMap::new(), + attribute_hash: HashMap::default(), local_name_hash: HashMap::default(), namespace_hash: HashMap::default(), other: SmallVec::new(), + bucket_attributes: static_prefs::pref!("layout.css.bucket-attribute-names.enabled"), count: 0, } } + /// Trivially constructs an empty `SelectorMap`, with attribute bucketing + /// explicitly disabled. + pub fn new_without_attribute_bucketing() -> Self { + let mut ret = Self::new(); + ret.bucket_attributes = false; + ret + } + /// Clears the hashmap retaining storage. pub fn clear(&mut self) { self.root.clear(); self.id_hash.clear(); self.class_hash.clear(); + self.attribute_hash.clear(); self.local_name_hash.clear(); self.namespace_hash.clear(); self.other.clear(); @@ -218,6 +233,21 @@ impl SelectorMap<Rule> { } }); + if self.bucket_attributes { + rule_hash_target.each_attr_name(|name| { + if let Some(rules) = self.attribute_hash.get(name) { + SelectorMap::get_matching_rules( + element, + rules, + matching_rules_list, + context, + flags_setter, + cascade_level, + ) + } + }); + } + if let Some(rules) = self.local_name_hash.get(rule_hash_target.local_name()) { SelectorMap::get_matching_rules( element, @@ -302,6 +332,7 @@ impl<T: SelectorMapEntry> SelectorMap<T> { .class_hash .try_entry(class.clone(), quirks_mode)? .or_insert_with(SmallVec::new), + Bucket::Attribute { name, lower_name } | Bucket::LocalName { name, lower_name } => { // If the local name in the selector isn't lowercase, // insert it into the rule hash twice. This means that, @@ -316,13 +347,19 @@ impl<T: SelectorMapEntry> SelectorMap<T> { // selector, the rulehash lookup may produce superfluous // selectors, but the subsequent selector matching work // will filter them out. + let is_attribute = matches!($bucket, Bucket::Attribute { .. }); + let hash = if is_attribute { + &mut self.attribute_hash + } else { + &mut self.local_name_hash + }; if name != lower_name { - self.local_name_hash + hash .try_entry(lower_name.clone())? .or_insert_with(SmallVec::new) .try_push($entry.clone())?; } - self.local_name_hash + hash .try_entry(name.clone())? .or_insert_with(SmallVec::new) }, @@ -338,7 +375,7 @@ impl<T: SelectorMapEntry> SelectorMap<T> { let bucket = { let mut disjoint_buckets = SmallVec::new(); - let bucket = find_bucket(entry.selector(), &mut disjoint_buckets); + let bucket = find_bucket(entry.selector(), &mut disjoint_buckets, self.bucket_attributes); // See if inserting this selector in multiple entries in the // selector map would be worth it. Consider a case like: @@ -409,8 +446,29 @@ impl<T: SelectorMapEntry> SelectorMap<T> { let mut done = false; element.each_class(|class| { - if !done { - if let Some(v) = self.class_hash.get(class, quirks_mode) { + if done { + return; + } + if let Some(v) = self.class_hash.get(class, quirks_mode) { + for entry in v.iter() { + if !f(&entry) { + done = true; + return; + } + } + } + }); + + if done { + return false; + } + + if self.bucket_attributes { + element.each_attr_name(|name| { + if done { + return; + } + if let Some(v) = self.attribute_hash.get(name) { for entry in v.iter() { if !f(&entry) { done = true; @@ -418,10 +476,11 @@ impl<T: SelectorMapEntry> SelectorMap<T> { } } } + }); + + if done { + return false; } - }); - if done { - return false; } if let Some(v) = self.local_name_hash.get(element.local_name()) { @@ -507,6 +566,10 @@ enum Bucket<'a> { name: &'a LocalName, lower_name: &'a LocalName, }, + Attribute { + name: &'a LocalName, + lower_name: &'a LocalName, + }, Class(&'a Atom), ID(&'a Atom), Root, @@ -520,9 +583,10 @@ impl<'a> Bucket<'a> { Bucket::Universal => 0, Bucket::Namespace(..) => 1, Bucket::LocalName { .. } => 2, - Bucket::Class(..) => 3, - Bucket::ID(..) => 4, - Bucket::Root => 5, + Bucket::Attribute { .. } => 3, + Bucket::Class(..) => 4, + Bucket::ID(..) => 5, + Bucket::Root => 6, } } @@ -537,11 +601,24 @@ type DisjointBuckets<'a> = SmallVec<[Bucket<'a>; 5]>; fn specific_bucket_for<'a>( component: &'a Component<SelectorImpl>, disjoint_buckets: &mut DisjointBuckets<'a>, + bucket_attributes: bool, ) -> Bucket<'a> { match *component { Component::Root => Bucket::Root, Component::ID(ref id) => Bucket::ID(id), Component::Class(ref class) => Bucket::Class(class), + Component::AttributeInNoNamespace { ref local_name, .. } if bucket_attributes => Bucket::Attribute { + name: local_name, + lower_name: local_name, + }, + Component::AttributeInNoNamespaceExists { ref local_name, ref local_name_lower } if bucket_attributes => Bucket::Attribute { + name: local_name, + lower_name: local_name_lower, + }, + Component::AttributeOther(ref selector) if bucket_attributes => Bucket::Attribute { + name: &selector.local_name, + lower_name: &selector.local_name_lower, + }, Component::LocalName(ref selector) => Bucket::LocalName { name: &selector.name, lower_name: &selector.lower_name, @@ -567,14 +644,14 @@ fn specific_bucket_for<'a>( // // So inserting `span` in the rule hash makes sense since we want to // match the slotted <span>. - Component::Slotted(ref selector) => find_bucket(selector.iter(), disjoint_buckets), - Component::Host(Some(ref selector)) => find_bucket(selector.iter(), disjoint_buckets), + Component::Slotted(ref selector) => find_bucket(selector.iter(), disjoint_buckets, bucket_attributes), + Component::Host(Some(ref selector)) => find_bucket(selector.iter(), disjoint_buckets, bucket_attributes), Component::Is(ref list) | Component::Where(ref list) => { if list.len() == 1 { - find_bucket(list[0].iter(), disjoint_buckets) + find_bucket(list[0].iter(), disjoint_buckets, bucket_attributes) } else { for selector in &**list { - let bucket = find_bucket(selector.iter(), disjoint_buckets); + let bucket = find_bucket(selector.iter(), disjoint_buckets, bucket_attributes); disjoint_buckets.push(bucket); } Bucket::Universal @@ -593,12 +670,13 @@ fn specific_bucket_for<'a>( fn find_bucket<'a>( mut iter: SelectorIter<'a, SelectorImpl>, disjoint_buckets: &mut DisjointBuckets<'a>, + bucket_attributes: bool, ) -> Bucket<'a> { let mut current_bucket = Bucket::Universal; loop { for ss in &mut iter { - let new_bucket = specific_bucket_for(ss, disjoint_buckets); + let new_bucket = specific_bucket_for(ss, disjoint_buckets, bucket_attributes); if new_bucket.more_specific_than(¤t_bucket) { current_bucket = new_bucket; } diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index 49b34fbbbff..50eb51fba35 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -786,10 +786,7 @@ impl<E: TElement> StyleSharingCache<E> { } // It's possible that there are no styles for either id. - let may_match_different_id_rules = - checks::may_match_different_id_rules(shared, target.element, candidate.element); - - if may_match_different_id_rules { + if checks::may_match_different_id_rules(shared, target.element, candidate.element) { trace!("Miss: ID Attr"); return None; } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 643a1454e00..ef3d43ab737 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1961,7 +1961,15 @@ impl CascadeData { state_dependencies: ElementState::empty(), document_state_dependencies: DocumentState::empty(), mapped_ids: PrecomputedHashSet::default(), - selectors_for_cache_revalidation: SelectorMap::new(), + // NOTE: We disable attribute bucketing for revalidation because we + // rely on the buckets to match, but we don't want to just not share + // style across elements with different attributes. + // + // An alternative to this would be to perform a style sharing check + // like may_match_different_id_rules which would check that the + // attribute buckets match on all scopes. But that seems + // somewhat gnarly. + selectors_for_cache_revalidation: SelectorMap::new_without_attribute_bucketing(), animations: Default::default(), extra_data: ExtraStyleData::default(), effective_media_query_results: EffectiveMediaQueryResults::new(), |