diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 521bcb3b815..07050d8c91b 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -23,14 +23,11 @@ bitflags! { pub flags StyleRelations: usize { /// Whether this element is affected by an ID selector. const AFFECTED_BY_ID_SELECTOR = 1 << 0, - /// Whether this element has a style attribute. Computed - /// externally. - const AFFECTED_BY_STYLE_ATTRIBUTE = 1 << 1, /// Whether this element is affected by presentational hints. This is /// computed externally (that is, in Servo). - const AFFECTED_BY_PRESENTATIONAL_HINTS = 1 << 2, + const AFFECTED_BY_PRESENTATIONAL_HINTS = 1 << 1, /// Whether this element has pseudo-element styles. Computed externally. - const AFFECTED_BY_PSEUDO_ELEMENTS = 1 << 3, + const AFFECTED_BY_PSEUDO_ELEMENTS = 1 << 2, } } diff --git a/components/style/context.rs b/components/style/context.rs index ef19d45b6d9..cbcac7e9982 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -463,7 +463,7 @@ impl ThreadLocalStyleContext { self.current_element_info = Some(CurrentElementInfo { element: element.as_node().opaque(), is_initial_style: !data.has_styles(), - validation_data: ValidationData::new(), + validation_data: ValidationData::default(), possibly_expired_animations: Vec::new(), }); } diff --git a/components/style/sharing/checks.rs b/components/style/sharing/checks.rs index d773afc2bd4..40b5140809f 100644 --- a/components/style/sharing/checks.rs +++ b/components/style/sharing/checks.rs @@ -21,8 +21,7 @@ use stylearc::Arc; pub fn relations_are_shareable(relations: &StyleRelations) -> bool { use selectors::matching::*; !relations.intersects(AFFECTED_BY_ID_SELECTOR | - AFFECTED_BY_PSEUDO_ELEMENTS | - AFFECTED_BY_STYLE_ATTRIBUTE) + AFFECTED_BY_PSEUDO_ELEMENTS) } /// Whether, given two elements, they have pointer-equal computed values. @@ -44,11 +43,21 @@ pub fn same_computed_values(first: Option, second: Option) -> bool eq } -/// Whether a given element has presentational hints. -/// -/// We consider not worth to share style with an element that has presentational -/// hints, both because implementing the code that compares that the hints are -/// equal is somewhat annoying, and also because it'd be expensive enough. +/// Whether two elements have the same same style attribute (by pointer identity). +pub fn have_same_style_attribute( + target: &mut StyleSharingTarget, + candidate: &mut StyleSharingCandidate +) -> bool + where E: TElement, +{ + match (target.style_attribute(), candidate.style_attribute()) { + (None, None) => true, + (Some(_), None) | (None, Some(_)) => false, + (Some(a), Some(b)) => Arc::ptr_eq(a, b) + } +} + +/// Whether two elements have the same same presentational attributes. pub fn have_same_presentational_hints( target: &mut StyleSharingTarget, candidate: &mut StyleSharingCandidate diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index 7eaf0322d6b..4140d44a187 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -16,6 +16,7 @@ use properties::ComputedValues; use selectors::bloom::BloomFilter; use selectors::matching::{ElementSelectorFlags, StyleRelations}; use smallvec::SmallVec; +use std::mem; use std::ops::Deref; use stylist::{ApplicableDeclarationBlock, Stylist}; @@ -36,7 +37,7 @@ pub enum StyleSharingBehavior { /// Some data we want to avoid recomputing all the time while trying to share /// style. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct ValidationData { /// The class list of this element. /// @@ -53,23 +54,9 @@ pub struct ValidationData { } impl ValidationData { - /// Trivially construct an empty `ValidationData` with nothing on - /// it. - pub fn new() -> Self { - Self { - class_list: None, - pres_hints: None, - revalidation_match_results: None, - } - } - /// Move the cached data to a new instance, and return it. pub fn take(&mut self) -> Self { - Self { - class_list: self.class_list.take(), - pres_hints: self.pres_hints.take(), - revalidation_match_results: self.revalidation_match_results.take(), - } + mem::replace(self, Self::default()) } /// Get or compute the list of presentational attributes associated with @@ -90,8 +77,15 @@ impl ValidationData { where E: TElement, { if self.class_list.is_none() { - let mut class_list = SmallVec::new(); + let mut class_list = SmallVec::<[Atom; 5]>::new(); element.each_class(|c| class_list.push(c.clone())); + // Assuming there are a reasonable number of classes (we use the + // inline capacity as "reasonable number"), sort them to so that + // we don't mistakenly reject sharing candidates when one element + // has "foo bar" and the other has "bar foo". + if !class_list.spilled() { + class_list.sort_by(|a, b| a.get_hash().cmp(&b.get_hash())); + } self.class_list = Some(class_list); } &*self.class_list.as_ref().unwrap() @@ -132,6 +126,15 @@ pub struct StyleSharingCandidate { validation_data: ValidationData, } +impl Deref for StyleSharingCandidate { + type Target = E; + + fn deref(&self) -> &Self::Target { + &self.element + } +} + + impl StyleSharingCandidate { /// Get the classlist of this candidate. fn class_list(&mut self) -> &[Atom] { @@ -182,7 +185,7 @@ impl StyleSharingTarget { pub fn new(element: E) -> Self { Self { element: element, - validation_data: ValidationData::new(), + validation_data: ValidationData::default(), } } @@ -235,8 +238,6 @@ impl StyleSharingTarget { data: &mut ElementData) -> StyleSharingResult { - use std::mem; - let shared_context = &context.shared; let selector_flags_map = &mut context.thread_local.selector_flags; let bloom_filter = context.thread_local.bloom_filter.filter(); @@ -249,12 +250,9 @@ impl StyleSharingTarget { &mut self, data); - mem::swap(&mut self.validation_data, - &mut context - .thread_local - .current_element_info.as_mut().unwrap() - .validation_data); + context.thread_local.current_element_info.as_mut().unwrap().validation_data = + self.validation_data.take(); result } } @@ -420,12 +418,6 @@ impl StyleSharingCandidateCache { return StyleSharingResult::CannotShare; } - if target.style_attribute().is_some() { - debug!("{:?} Cannot share style: element has style attribute", - target.element); - return StyleSharingResult::CannotShare - } - if target.get_id().is_some() { debug!("{:?} Cannot share style: element has id", target.element); return StyleSharingResult::CannotShare @@ -553,7 +545,7 @@ impl StyleSharingCandidateCache { miss!(IdAttr) } - if target.style_attribute().is_some() { + if !checks::have_same_style_attribute(target, candidate) { miss!(StyleAttr) } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index def4de9eab1..e14ed0e27c5 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -26,8 +26,8 @@ use selector_map::{SelectorMap, SelectorMapEntry}; use selector_parser::{SelectorImpl, PseudoElement}; use selectors::attr::NamespaceConstraint; use selectors::bloom::BloomFilter; -use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS}; use selectors::matching::{ElementSelectorFlags, matches_selector, MatchingContext, MatchingMode}; +use selectors::matching::AFFECTED_BY_PRESENTATIONAL_HINTS; use selectors::parser::{Combinator, Component, Selector, SelectorInner, SelectorIter, SelectorMethods}; use selectors::visitor::SelectorVisitor; use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; @@ -966,7 +966,9 @@ impl Stylist { assert_eq!(declaration.level, CascadeLevel::PresHints); } } - // Never share style for elements with preshints + // Note the existence of presentational attributes so that the + // style sharing cache can avoid re-querying them if they don't + // exist. context.relations |= AFFECTED_BY_PRESENTATIONAL_HINTS; } debug!("preshints: {:?}", context.relations); @@ -1005,7 +1007,6 @@ impl Stylist { // Step 4: Normal style attributes. if let Some(sa) = style_attribute { - context.relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, ApplicableDeclarationBlock::from_declarations(sa.clone(),