diff --git a/components/style/context.rs b/components/style/context.rs index 98122aa197d..ee50e953980 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -7,7 +7,6 @@ #[cfg(feature = "servo")] use animation::Animation; use animation::PropertyAnimation; use app_units::Au; -use bit_vec::BitVec; use bloom::StyleBloom; use cache::LRUCache; use data::ElementData; @@ -23,7 +22,7 @@ use selector_parser::SnapshotMap; use selectors::matching::ElementSelectorFlags; #[cfg(feature = "servo")] use servo_config::opts; use shared_lock::StylesheetGuards; -use sharing::StyleSharingCandidateCache; +use sharing::{CachedStyleSharingData, StyleSharingCandidateCache}; #[cfg(feature = "servo")] use std::collections::HashMap; #[cfg(feature = "gecko")] use std::env; use std::fmt; @@ -165,9 +164,8 @@ pub struct CurrentElementInfo { element: OpaqueNode, /// Whether the element is being styled for the first time. is_initial_style: bool, - /// Lazy cache of the result of matching the current element against the - /// revalidation selectors. - pub revalidation_match_results: Option, + /// Lazy cache of the different data used for style sharing. + pub cached_style_sharing_data: CachedStyleSharingData, /// A Vec of possibly expired animations. Used only by Servo. #[allow(dead_code)] pub possibly_expired_animations: Vec, @@ -465,7 +463,7 @@ impl ThreadLocalStyleContext { self.current_element_info = Some(CurrentElementInfo { element: element.as_node().opaque(), is_initial_style: !data.has_styles(), - revalidation_match_results: None, + cached_style_sharing_data: CachedStyleSharingData::new(), possibly_expired_animations: Vec::new(), }); } diff --git a/components/style/matching.rs b/components/style/matching.rs index 25349b5457f..c741a949df2 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -23,7 +23,7 @@ use rule_tree::{CascadeLevel, StrongRuleNode}; use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl}; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode, StyleRelations}; use selectors::matching::{VisitedHandlingMode, AFFECTED_BY_PSEUDO_ELEMENTS}; -use sharing::{StyleSharingBehavior, StyleSharingResult}; +use sharing::StyleSharingBehavior; use stylearc::Arc; use stylist::{ApplicableDeclarationList, RuleInclusion}; @@ -858,7 +858,7 @@ pub trait MatchMethods : TElement { let revalidation_match_results = context.thread_local .current_element_info .as_mut().unwrap() - .revalidation_match_results + .cached_style_sharing_data .take(); context.thread_local .style_sharing_candidate_cache @@ -1341,30 +1341,6 @@ pub trait MatchMethods : TElement { false } - /// Attempts to share a style with another node. This method is unsafe - /// because it depends on the `style_sharing_candidate_cache` having only - /// live nodes in it, and we have no way to guarantee that at the type - /// system level yet. - unsafe fn share_style_if_possible(&self, - context: &mut StyleContext, - data: &mut ElementData) - -> StyleSharingResult { - let shared_context = &context.shared; - let current_element_info = - context.thread_local.current_element_info.as_mut().unwrap(); - let selector_flags_map = &mut context.thread_local.selector_flags; - let bloom_filter = context.thread_local.bloom_filter.filter(); - - context.thread_local - .style_sharing_candidate_cache - .share_style_if_possible(shared_context, - current_element_info, - selector_flags_map, - bloom_filter, - *self, - data) - } - /// Given the old and new style of this element, and whether it's a /// pseudo-element, compute the restyle damage used to determine which /// kind of layout or painting operations we'll need. diff --git a/components/style/sharing/checks.rs b/components/style/sharing/checks.rs index 84ec5c5f0a5..6fad78fcb7e 100644 --- a/components/style/sharing/checks.rs +++ b/components/style/sharing/checks.rs @@ -6,13 +6,12 @@ //! quickly whether it's worth to share style, and whether two different //! elements can indeed share the same style. -use context::{CurrentElementInfo, SelectorFlagsMap, SharedStyleContext}; +use context::{SelectorFlagsMap, SharedStyleContext}; use dom::TElement; use element_state::*; -use matching::MatchMethods; use selectors::bloom::BloomFilter; -use selectors::matching::{ElementSelectorFlags, StyleRelations}; -use sharing::StyleSharingCandidate; +use selectors::matching::StyleRelations; +use sharing::{StyleSharingCandidate, StyleSharingTarget}; use sink::ForgetfulSink; use stylearc::Arc; @@ -63,16 +62,12 @@ pub fn has_presentational_hints(element: E) -> bool /// Whether a given element has the same class attribute than a given candidate. /// /// We don't try to share style across elements with different class attributes. -pub fn have_same_class(element: E, +pub fn have_same_class(target: &mut StyleSharingTarget, candidate: &mut StyleSharingCandidate) -> bool where E: TElement, { - // XXX Efficiency here, I'm only validating ideas. - let mut element_class_attributes = vec![]; - element.each_class(|c| element_class_attributes.push(c.clone())); - - element_class_attributes == candidate.class_list() + target.class_list() == candidate.class_list() } /// Compare element and candidate state, but ignore visitedness. Styles don't @@ -96,42 +91,19 @@ pub fn have_same_state_ignoring_visitedness(element: E, /// :first-child, etc, or on attributes that we don't check off-hand (pretty /// much every attribute selector except `id` and `class`. #[inline] -pub fn revalidate(element: E, +pub fn revalidate(target: &mut StyleSharingTarget, candidate: &mut StyleSharingCandidate, shared_context: &SharedStyleContext, bloom: &BloomFilter, - info: &mut CurrentElementInfo, selector_flags_map: &mut SelectorFlagsMap) -> bool where E: TElement, { let stylist = &shared_context.stylist; - if info.revalidation_match_results.is_none() { - // It's important to set the selector flags. Otherwise, if we succeed in - // sharing the style, we may not set the slow selector flags for the - // right elements (which may not necessarily be |element|), causing - // missed restyles after future DOM mutations. - // - // Gecko's test_bug534804.html exercises this. A minimal testcase is: - // - // - // - // - // - // - // The style sharing cache will get a hit for the second span. When the - // child span is subsequently removed from the DOM, missing selector - // flags would cause us to miss the restyle on the second span. - let mut set_selector_flags = |el: &E, flags: ElementSelectorFlags| { - element.apply_selector_flags(selector_flags_map, el, flags); - }; - info.revalidation_match_results = - Some(stylist.match_revalidation_selectors(&element, bloom, - &mut set_selector_flags)); - } + let for_element = + target.revalidation_match_results(stylist, bloom, selector_flags_map); - let for_element = info.revalidation_match_results.as_ref().unwrap(); let for_candidate = candidate.revalidation_match_results(stylist, bloom); // This assert "ensures", to some extent, that the two candidates have diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index 145ede1a23b..c850ee24605 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -8,7 +8,7 @@ use Atom; use bit_vec::BitVec; use cache::{LRUCache, LRUCacheMutIterator}; -use context::{CurrentElementInfo, SelectorFlagsMap, SharedStyleContext}; +use context::{SelectorFlagsMap, SharedStyleContext, StyleContext}; use data::{ComputedStyle, ElementData, ElementStyles}; use dom::{TElement, SendElement}; use matching::{ChildCascadeRequirement, MatchMethods}; @@ -17,6 +17,7 @@ use selectors::bloom::BloomFilter; use selectors::matching::{ElementSelectorFlags, StyleRelations}; use sink::ForgetfulSink; use smallvec::SmallVec; +use std::ops::Deref; use stylist::Stylist; mod checks; @@ -50,6 +51,23 @@ pub struct CachedStyleSharingData { } impl CachedStyleSharingData { + /// Trivially construct an empty `CachedStyleSharingData` with nothing on + /// it. + pub fn new() -> Self { + Self { + class_list: 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(), + revalidation_match_results: self.revalidation_match_results.take(), + } + } + /// Get or compute the class-list associated with this element. pub fn class_list(&mut self, element: E) -> &[Atom] where E: TElement, @@ -122,6 +140,97 @@ impl PartialEq> for StyleSharingCandidate< } } +/// An element we want to test against the style sharing cache. +pub struct StyleSharingTarget { + element: E, + cache: CachedStyleSharingData, +} + +impl Deref for StyleSharingTarget { + type Target = E; + + fn deref(&self) -> &Self::Target { + &self.element + } +} + +impl StyleSharingTarget { + /// Trivially construct a new StyleSharingTarget to test against the cache. + pub fn new(element: E) -> Self { + Self { + element: element, + cache: CachedStyleSharingData::new(), + } + } + + fn class_list(&mut self) -> &[Atom] { + self.cache.class_list(self.element) + } + + fn revalidation_match_results( + &mut self, + stylist: &Stylist, + bloom: &BloomFilter, + selector_flags_map: &mut SelectorFlagsMap + ) -> &BitVec { + // It's important to set the selector flags. Otherwise, if we succeed in + // sharing the style, we may not set the slow selector flags for the + // right elements (which may not necessarily be |element|), causing + // missed restyles after future DOM mutations. + // + // Gecko's test_bug534804.html exercises this. A minimal testcase is: + // + // + // + // + // + // + // The style sharing cache will get a hit for the second span. When the + // child span is subsequently removed from the DOM, missing selector + // flags would cause us to miss the restyle on the second span. + let element = self.element; + let mut set_selector_flags = |el: &E, flags: ElementSelectorFlags| { + element.apply_selector_flags(selector_flags_map, el, flags); + }; + + self.cache.revalidation_match_results(self.element, + stylist, + bloom, + &mut set_selector_flags) + } + + /// Attempts to share a style with another node. + pub fn share_style_if_possible( + mut self, + context: &mut StyleContext, + 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(); + + let result = context.thread_local + .style_sharing_candidate_cache + .share_style_if_possible(shared_context, + selector_flags_map, + bloom_filter, + &mut self, + data); + + // FIXME(emilio): Do this in a cleaner way. + mem::swap(&mut self.cache, + &mut context + .thread_local + .current_element_info.as_mut().unwrap() + .cached_style_sharing_data); + + result + } +} + /// A cache miss result. #[derive(Clone, Debug)] pub enum CacheMiss { @@ -198,7 +307,7 @@ impl StyleSharingCandidateCache { element: &E, style: &ComputedValues, relations: StyleRelations, - revalidation_match_results: Option) { + cache: CachedStyleSharingData) { let parent = match element.parent_element() { Some(element) => element, None => { @@ -242,10 +351,7 @@ impl StyleSharingCandidateCache { self.cache.insert(StyleSharingCandidate { element: unsafe { SendElement::new(*element) }, - cache: CachedStyleSharingData { - class_list: None, - revalidation_match_results: revalidation_match_results, - } + cache: cache, }); } @@ -260,43 +366,39 @@ impl StyleSharingCandidateCache { } /// Attempts to share a style with another node. - /// - /// This method is unsafe because it depends on the - /// `style_sharing_candidate_cache` having only live nodes in it, and we - /// have no way to guarantee that at the type system level yet. - pub unsafe fn share_style_if_possible( + fn share_style_if_possible( &mut self, shared_context: &SharedStyleContext, - current_element_info: &mut CurrentElementInfo, selector_flags_map: &mut SelectorFlagsMap, bloom_filter: &BloomFilter, - element: E, + target: &mut StyleSharingTarget, data: &mut ElementData ) -> StyleSharingResult { if shared_context.options.disable_style_sharing_cache { debug!("{:?} Cannot share style: style sharing cache disabled", - element); + target.element); return StyleSharingResult::CannotShare } - if element.parent_element().is_none() { - debug!("{:?} Cannot share style: element has no parent", element); + if target.parent_element().is_none() { + debug!("{:?} Cannot share style: element has no parent", + target.element); return StyleSharingResult::CannotShare } - if element.is_native_anonymous() { - debug!("{:?} Cannot share style: NAC", element); + if target.is_native_anonymous() { + debug!("{:?} Cannot share style: NAC", target.element); return StyleSharingResult::CannotShare; } - if element.style_attribute().is_some() { + if target.style_attribute().is_some() { debug!("{:?} Cannot share style: element has style attribute", - element); + target.element); return StyleSharingResult::CannotShare } - if element.get_id().is_some() { - debug!("{:?} Cannot share style: element has id", element); + if target.get_id().is_some() { + debug!("{:?} Cannot share style: element has id", target.element); return StyleSharingResult::CannotShare } @@ -304,11 +406,10 @@ impl StyleSharingCandidateCache { for (i, candidate) in self.iter_mut().enumerate() { let sharing_result = Self::test_candidate( - element, + target, candidate, &shared_context, bloom_filter, - current_element_info, selector_flags_map ); @@ -321,7 +422,7 @@ impl StyleSharingCandidateCache { let old_values = data.get_styles_mut() .and_then(|s| s.primary.values.take()); let child_cascade_requirement = - element.accumulate_damage( + target.accumulate_damage( &shared_context, data.get_restyle_mut(), old_values.as_ref().map(|v| &**v), @@ -360,7 +461,7 @@ impl StyleSharingCandidateCache { } } - debug!("{:?} Cannot share style: {} cache entries", element, + debug!("{:?} Cannot share style: {} cache entries", target.element, self.cache.num_entries()); if should_clear_cache { @@ -370,11 +471,10 @@ impl StyleSharingCandidateCache { StyleSharingResult::CannotShare } - fn test_candidate(element: E, + fn test_candidate(target: &mut StyleSharingTarget, candidate: &mut StyleSharingCandidate, shared: &SharedStyleContext, bloom: &BloomFilter, - info: &mut CurrentElementInfo, selector_flags_map: &mut SelectorFlagsMap) -> Result { macro_rules! miss { @@ -386,66 +486,66 @@ impl StyleSharingCandidateCache { // Check that we have the same parent, or at least the same pointer // identity for parent computed style. The latter check allows us to // share style between cousins if the parents shared style. - let parent = element.parent_element(); + let parent = target.parent_element(); let candidate_parent = candidate.element.parent_element(); if parent != candidate_parent && !checks::same_computed_values(parent, candidate_parent) { miss!(Parent) } - if element.is_native_anonymous() { + if target.is_native_anonymous() { debug_assert!(!candidate.element.is_native_anonymous(), "Why inserting NAC into the cache?"); miss!(NativeAnonymousContent) } - if *element.get_local_name() != *candidate.element.get_local_name() { + if *target.get_local_name() != *candidate.element.get_local_name() { miss!(LocalName) } - if *element.get_namespace() != *candidate.element.get_namespace() { + if *target.get_namespace() != *candidate.element.get_namespace() { miss!(Namespace) } - if element.is_link() != candidate.element.is_link() { + if target.is_link() != candidate.element.is_link() { miss!(Link) } - if element.matches_user_and_author_rules() != + if target.matches_user_and_author_rules() != candidate.element.matches_user_and_author_rules() { miss!(UserAndAuthorRules) } - if !checks::have_same_state_ignoring_visitedness(element, candidate) { + if !checks::have_same_state_ignoring_visitedness(target.element, candidate) { miss!(State) } - if element.get_id() != candidate.element.get_id() { + if target.get_id() != candidate.element.get_id() { miss!(IdAttr) } - if element.style_attribute().is_some() { + if target.style_attribute().is_some() { miss!(StyleAttr) } - if !checks::have_same_class(element, candidate) { + if !checks::have_same_class(target, candidate) { miss!(Class) } - if checks::has_presentational_hints(element) { + if checks::has_presentational_hints(target.element) { miss!(PresHints) } - if !checks::revalidate(element, candidate, shared, bloom, info, + if !checks::revalidate(target, candidate, shared, bloom, selector_flags_map) { miss!(Revalidation) } let data = candidate.element.borrow_data().unwrap(); - debug_assert!(element.has_current_styles(&data)); + debug_assert!(target.has_current_styles(&data)); debug!("Sharing style between {:?} and {:?}", - element, candidate.element); + target.element, candidate.element); Ok(data.styles().primary.clone()) } } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 60df72ffeba..ecefa15278a 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -11,7 +11,7 @@ use dom::{DirtyDescendants, NodeInfo, OpaqueNode, TElement, TNode}; use matching::{ChildCascadeRequirement, MatchMethods}; use restyle_hints::{HintComputationContext, RestyleHint}; use selector_parser::RestyleDamage; -use sharing::StyleSharingBehavior; +use sharing::{StyleSharingBehavior, StyleSharingTarget}; #[cfg(feature = "servo")] use servo_config::opts; use smallvec::SmallVec; use std::borrow::BorrowMut; @@ -795,9 +795,9 @@ fn compute_style(_traversal: &D, // First, try the style sharing cache. If we get a match we can skip the rest // of the work. if let MatchAndCascade = kind { - let sharing_result = unsafe { - element.share_style_if_possible(context, data) - }; + let target = StyleSharingTarget::new(element); + let sharing_result = target.share_style_if_possible(context, data); + if let StyleWasShared(index, had_damage) = sharing_result { context.thread_local.statistics.styles_shared += 1; context.thread_local.style_sharing_candidate_cache.touch(index);