From 7b2d94daeed06b5ad483a8e87cefc47fc045a2d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 30 May 2017 07:28:06 -0500 Subject: [PATCH] servo: Merge #17063 - style: Allow sharing styles across elements with presentational hints (from emilio:pres-hints-sharing); r=bholley Source-Repo: https://github.com/servo/servo Source-Revision: 38a6a3bff6f6e0e35eb592891d5e70e7cb897b69 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 5c6316d592dfa5ef57e7fc03f4c5211cbbc1b1d7 --- servo/components/layout/animation.rs | 8 +- servo/components/layout_thread/lib.rs | 18 +- servo/components/style/context.rs | 38 +-- servo/components/style/matching.rs | 40 +-- servo/components/style/rule_tree/mod.rs | 6 + servo/components/style/sharing/checks.rs | 72 ++---- servo/components/style/sharing/mod.rs | 308 ++++++++++++++++++----- servo/components/style/stylist.rs | 2 +- servo/components/style/traversal.rs | 8 +- 9 files changed, 318 insertions(+), 182 deletions(-) diff --git a/servo/components/layout/animation.rs b/servo/components/layout/animation.rs index a054927190ca..5d7a1a87accc 100644 --- a/servo/components/layout/animation.rs +++ b/servo/components/layout/animation.rs @@ -6,13 +6,13 @@ use context::LayoutContext; use flow::{self, Flow}; +use fnv::FnvHashMap; use gfx::display_list::OpaqueNode; use ipc_channel::ipc::IpcSender; use msg::constellation_msg::PipelineId; use opaque_node::OpaqueNodeMethods; use script_traits::{AnimationState, ConstellationControlMsg, LayoutMsg as ConstellationMsg}; use script_traits::UntrustedNodeAddress; -use std::collections::HashMap; use std::sync::mpsc::Receiver; use style::animation::{Animation, update_style_for_animation}; use style::font_metrics::ServoMetricsProvider; @@ -24,8 +24,8 @@ use style::timer::Timer; /// `expired_animations`. pub fn update_animation_state(constellation_chan: &IpcSender, script_chan: &IpcSender, - running_animations: &mut HashMap>, - expired_animations: &mut HashMap>, + running_animations: &mut FnvHashMap>, + expired_animations: &mut FnvHashMap>, mut newly_transitioning_nodes: Option<&mut Vec>, new_animations_receiver: &Receiver, pipeline_id: PipelineId, @@ -149,7 +149,7 @@ pub fn update_animation_state(constellation_chan: &IpcSender, // this should be made generic. pub fn recalc_style_for_animations(context: &LayoutContext, flow: &mut Flow, - animations: &HashMap>) { let mut damage = RestyleDamage::empty(); flow.mutate_fragments(&mut |fragment| { diff --git a/servo/components/layout_thread/lib.rs b/servo/components/layout_thread/lib.rs index 32cf5dc23a33..9ddff21070e9 100644 --- a/servo/components/layout_thread/lib.rs +++ b/servo/components/layout_thread/lib.rs @@ -44,7 +44,7 @@ use euclid::point::Point2D; use euclid::rect::Rect; use euclid::scale_factor::ScaleFactor; use euclid::size::Size2D; -use fnv::FnvHasher; +use fnv::FnvHashMap; use gfx::display_list::{OpaqueNode, WebRenderImageInfo}; use gfx::font; use gfx::font_cache_thread::FontCacheThread; @@ -99,7 +99,6 @@ use servo_url::ServoUrl; use std::borrow::ToOwned; use std::cell::{Cell, RefCell}; use std::collections::HashMap; -use std::hash::BuildHasherDefault; use std::marker::PhantomData; use std::mem as std_mem; use std::ops::{Deref, DerefMut}; @@ -203,10 +202,10 @@ pub struct LayoutThread { document_shared_lock: Option, /// The list of currently-running animations. - running_animations: StyleArc>>>, + running_animations: StyleArc>>>, /// The list of animations that have expired since the last style recalculation. - expired_animations: StyleArc>>>, + expired_animations: StyleArc>>>, /// A counter for epoch messages epoch: Cell, @@ -224,9 +223,8 @@ pub struct LayoutThread { /// The CSS error reporter for all CSS loaded in this layout thread error_reporter: CSSErrorReporter, - webrender_image_cache: Arc>>>, + webrender_image_cache: Arc>>, /// Webrender interface. webrender_api: webrender_traits::RenderApi, @@ -492,8 +490,8 @@ impl LayoutThread { outstanding_web_fonts: outstanding_web_fonts_counter, root_flow: RefCell::new(None), document_shared_lock: None, - running_animations: StyleArc::new(RwLock::new(HashMap::new())), - expired_animations: StyleArc::new(RwLock::new(HashMap::new())), + running_animations: StyleArc::new(RwLock::new(FnvHashMap::default())), + expired_animations: StyleArc::new(RwLock::new(FnvHashMap::default())), epoch: Cell::new(Epoch(0)), viewport_size: Size2D::new(Au(0), Au(0)), webrender_api: webrender_api_sender.create_api(), @@ -521,7 +519,7 @@ impl LayoutThread { script_chan: Arc::new(Mutex::new(script_chan)), }, webrender_image_cache: - Arc::new(RwLock::new(HashMap::with_hasher(Default::default()))), + Arc::new(RwLock::new(FnvHashMap::default())), timer: if PREFS.get("layout.animations.test.enabled") .as_boolean().unwrap_or(false) { diff --git a/servo/components/style/context.rs b/servo/components/style/context.rs index 179e82500686..ef19d45b6d9f 100644 --- a/servo/components/style/context.rs +++ b/servo/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; @@ -21,11 +20,8 @@ use font_metrics::FontMetricsProvider; #[cfg(feature = "gecko")] use properties::ComputedValues; use selector_parser::SnapshotMap; use selectors::matching::ElementSelectorFlags; -#[cfg(feature = "servo")] use servo_config::opts; use shared_lock::StylesheetGuards; -use sharing::StyleSharingCandidateCache; -#[cfg(feature = "servo")] use std::collections::HashMap; -#[cfg(feature = "gecko")] use std::env; +use sharing::{ValidationData, StyleSharingCandidateCache}; use std::fmt; use std::ops::Add; #[cfg(feature = "servo")] use std::sync::Mutex; @@ -79,6 +75,7 @@ pub struct StyleSystemOptions { #[cfg(feature = "gecko")] fn get_env(name: &str) -> bool { + use std::env; match env::var(name) { Ok(s) => !s.is_empty(), Err(_) => false, @@ -88,6 +85,8 @@ fn get_env(name: &str) -> bool { impl Default for StyleSystemOptions { #[cfg(feature = "servo")] fn default() -> Self { + use servo_config::opts; + StyleSystemOptions { disable_style_sharing_cache: opts::get().disable_share_style_cache, dump_style_statistics: opts::get().style_sharing_stats, @@ -135,11 +134,11 @@ pub struct SharedStyleContext<'a> { /// The animations that are currently running. #[cfg(feature = "servo")] - pub running_animations: Arc>>>, + pub running_animations: Arc>>>, /// The list of animations that have expired since the last style recalculation. #[cfg(feature = "servo")] - pub expired_animations: Arc>>>, + pub expired_animations: Arc>>>, /// Data needed to create the thread-local style context from the shared one. #[cfg(feature = "servo")] @@ -154,26 +153,27 @@ impl<'a> SharedStyleContext<'a> { } } -/// Information about the current element being processed. We group this together -/// into a single struct within ThreadLocalStyleContext so that we can instantiate -/// and destroy it easily at the beginning and end of element processing. +/// Information about the current element being processed. We group this +/// together into a single struct within ThreadLocalStyleContext so that we can +/// instantiate and destroy it easily at the beginning and end of element +/// processing. pub struct CurrentElementInfo { - /// The element being processed. Currently we use an OpaqueNode since we only - /// use this for identity checks, but we could use SendElement if there were - /// a good reason to. + /// The element being processed. Currently we use an OpaqueNode since we + /// only use this for identity checks, but we could use SendElement if there + /// were a good reason to. 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 validation_data: ValidationData, /// A Vec of possibly expired animations. Used only by Servo. #[allow(dead_code)] pub possibly_expired_animations: Vec, } -/// Statistics gathered during the traversal. We gather statistics on each thread -/// and then combine them after the threads join via the Add implementation below. +/// Statistics gathered during the traversal. We gather statistics on each +/// thread and then combine them after the threads join via the Add +/// implementation below. #[derive(Default)] pub struct TraversalStatistics { /// The total number of elements traversed. @@ -463,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, + validation_data: ValidationData::new(), possibly_expired_animations: Vec::new(), }); } diff --git a/servo/components/style/matching.rs b/servo/components/style/matching.rs index 25349b5457ff..c811fef3afaa 100644 --- a/servo/components/style/matching.rs +++ b/servo/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}; @@ -855,17 +855,19 @@ pub trait MatchMethods : TElement { // // If we do have the results, grab them here to satisfy the borrow // checker. - let revalidation_match_results = context.thread_local - .current_element_info - .as_mut().unwrap() - .revalidation_match_results - .take(); + let validation_data = + context.thread_local + .current_element_info + .as_mut().unwrap() + .validation_data + .take(); + context.thread_local .style_sharing_candidate_cache .insert_if_possible(self, data.styles().primary.values(), primary_results.relations, - revalidation_match_results); + validation_data); } child_cascade_requirement @@ -1341,30 +1343,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/servo/components/style/rule_tree/mod.rs b/servo/components/style/rule_tree/mod.rs index 22a862cac5f8..2557b4c77e5f 100644 --- a/servo/components/style/rule_tree/mod.rs +++ b/servo/components/style/rule_tree/mod.rs @@ -61,6 +61,12 @@ pub enum StyleSource { Declarations(Arc>), } +impl PartialEq for StyleSource { + fn eq(&self, other: &Self) -> bool { + self.ptr_equals(other) + } +} + impl StyleSource { #[inline] fn ptr_equals(&self, other: &Self) -> bool { diff --git a/servo/components/style/sharing/checks.rs b/servo/components/style/sharing/checks.rs index 4ac537e00096..d773afc2bd43 100644 --- a/servo/components/style/sharing/checks.rs +++ b/servo/components/style/sharing/checks.rs @@ -6,14 +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 sink::ForgetfulSink; +use selectors::matching::StyleRelations; +use sharing::{StyleSharingCandidate, StyleSharingTarget}; use stylearc::Arc; /// Determines, based on the results of selector matching, whether it's worth to @@ -24,8 +22,7 @@ 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_PRESENTATIONAL_HINTS) + AFFECTED_BY_STYLE_ATTRIBUTE) } /// Whether, given two elements, they have pointer-equal computed values. @@ -52,33 +49,24 @@ pub fn same_computed_values(first: Option, second: Option) -> bool /// 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. -pub fn has_presentational_hints(element: E) -> bool +pub fn have_same_presentational_hints( + target: &mut StyleSharingTarget, + candidate: &mut StyleSharingCandidate +) -> bool where E: TElement, { - let mut hints = ForgetfulSink::new(); - element.synthesize_presentational_hints_for_legacy_attributes(&mut hints); - !hints.is_empty() + target.pres_hints() == candidate.pres_hints() } /// 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())); - - if candidate.class_attributes.is_none() { - let mut attrs = vec![]; - candidate.element.each_class(|c| attrs.push(c.clone())); - candidate.class_attributes = Some(attrs) - } - - element_class_attributes == *candidate.class_attributes.as_ref().unwrap() + target.class_list() == candidate.class_list() } /// Compare element and candidate state, but ignore visitedness. Styles don't @@ -102,50 +90,20 @@ 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); - if candidate.revalidation_match_results.is_none() { - let results = - stylist.match_revalidation_selectors(&*candidate.element, bloom, - &mut |_, _| {}); - candidate.revalidation_match_results = Some(results); - } - - let for_element = info.revalidation_match_results.as_ref().unwrap(); - let for_candidate = candidate.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 // matched the same rulehash buckets, and as such, that the bits we're diff --git a/servo/components/style/sharing/mod.rs b/servo/components/style/sharing/mod.rs index 67fde2e047b9..7eaf0322d6bb 100644 --- a/servo/components/style/sharing/mod.rs +++ b/servo/components/style/sharing/mod.rs @@ -8,14 +8,16 @@ 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}; use properties::ComputedValues; use selectors::bloom::BloomFilter; -use selectors::matching::StyleRelations; -use sink::ForgetfulSink; +use selectors::matching::{ElementSelectorFlags, StyleRelations}; +use smallvec::SmallVec; +use std::ops::Deref; +use stylist::{ApplicableDeclarationBlock, Stylist}; mod checks; @@ -32,23 +34,127 @@ pub enum StyleSharingBehavior { Disallow, } +/// Some data we want to avoid recomputing all the time while trying to share +/// style. +#[derive(Debug)] +pub struct ValidationData { + /// The class list of this element. + /// + /// TODO(emilio): See if it's worth to sort them, or doing something else in + /// a similar fashion as what Boris is doing for the ID attribute. + class_list: Option>, + + /// The list of presentational attributes of the element. + pres_hints: Option>, + + /// The cached result of matching this entry against the revalidation + /// selectors. + revalidation_match_results: Option, +} + +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(), + } + } + + /// Get or compute the list of presentational attributes associated with + /// this element. + pub fn pres_hints(&mut self, element: E) -> &[ApplicableDeclarationBlock] + where E: TElement, + { + if self.pres_hints.is_none() { + let mut pres_hints = SmallVec::new(); + element.synthesize_presentational_hints_for_legacy_attributes(&mut pres_hints); + self.pres_hints = Some(pres_hints); + } + &*self.pres_hints.as_ref().unwrap() + } + + /// Get or compute the class-list associated with this element. + pub fn class_list(&mut self, element: E) -> &[Atom] + where E: TElement, + { + if self.class_list.is_none() { + let mut class_list = SmallVec::new(); + element.each_class(|c| class_list.push(c.clone())); + self.class_list = Some(class_list); + } + &*self.class_list.as_ref().unwrap() + } + + /// Computes the revalidation results if needed, and returns it. + fn revalidation_match_results( + &mut self, + element: E, + stylist: &Stylist, + bloom: &BloomFilter, + flags_setter: &mut F + ) -> &BitVec + where E: TElement, + F: FnMut(&E, ElementSelectorFlags), + { + if self.revalidation_match_results.is_none() { + self.revalidation_match_results = + Some(stylist.match_revalidation_selectors(&element, bloom, + flags_setter)); + } + + self.revalidation_match_results.as_ref().unwrap() + } +} + /// Information regarding a style sharing candidate, that is, an entry in the /// style sharing cache. /// /// Note that this information is stored in TLS and cleared after the traversal, /// and once here, the style information of the element is immutable, so it's /// safe to access. -/// -/// TODO: We can stick a lot more info here. #[derive(Debug)] pub struct StyleSharingCandidate { /// The element. We use SendElement here so that the cache may live in /// ScopedTLS. element: SendElement, - /// The cached class names. - class_attributes: Option>, - /// The cached result of matching this entry against the revalidation selectors. - revalidation_match_results: Option, + validation_data: ValidationData, +} + +impl StyleSharingCandidate { + /// Get the classlist of this candidate. + fn class_list(&mut self) -> &[Atom] { + self.validation_data.class_list(*self.element) + } + + /// Get the pres hints of this candidate. + fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] { + self.validation_data.pres_hints(*self.element) + } + + /// Get the classlist of this candidate. + fn revalidation_match_results( + &mut self, + stylist: &Stylist, + bloom: &BloomFilter, + ) -> &BitVec { + self.validation_data.revalidation_match_results( + *self.element, + stylist, + bloom, + &mut |_, _| {}) + } } impl PartialEq> for StyleSharingCandidate { @@ -57,6 +163,102 @@ impl PartialEq> for StyleSharingCandidate< } } +/// An element we want to test against the style sharing cache. +pub struct StyleSharingTarget { + element: E, + validation_data: ValidationData, +} + +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, + validation_data: ValidationData::new(), + } + } + + fn class_list(&mut self) -> &[Atom] { + self.validation_data.class_list(self.element) + } + + /// Get the pres hints of this candidate. + fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] { + self.validation_data.pres_hints(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.validation_data.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); + + mem::swap(&mut self.validation_data, + &mut context + .thread_local + .current_element_info.as_mut().unwrap() + .validation_data); + + result + } +} + /// A cache miss result. #[derive(Clone, Debug)] pub enum CacheMiss { @@ -133,7 +335,9 @@ impl StyleSharingCandidateCache { element: &E, style: &ComputedValues, relations: StyleRelations, - revalidation_match_results: Option) { + mut validation_data: ValidationData) { + use selectors::matching::AFFECTED_BY_PRESENTATIONAL_HINTS; + let parent = match element.parent_element() { Some(element) => element, None => { @@ -154,14 +358,6 @@ impl StyleSharingCandidateCache { return; } - // Make sure we noted any presentational hints in the StyleRelations. - if cfg!(debug_assertions) { - let mut hints = ForgetfulSink::new(); - element.synthesize_presentational_hints_for_legacy_attributes(&mut hints); - debug_assert!(hints.is_empty(), - "Style relations should not be shareable!"); - } - let box_style = style.get_box(); if box_style.specifies_transitions() { debug!("Failing to insert to the cache: transitions"); @@ -173,12 +369,18 @@ impl StyleSharingCandidateCache { return; } + // Take advantage of the information we've learned during + // selector-matching. + if !relations.intersects(AFFECTED_BY_PRESENTATIONAL_HINTS) { + debug_assert!(validation_data.pres_hints.as_ref().map_or(true, |v| v.is_empty())); + validation_data.pres_hints = Some(SmallVec::new()); + } + debug!("Inserting into cache: {:?} with parent {:?}", element, parent); self.cache.insert(StyleSharingCandidate { element: unsafe { SendElement::new(*element) }, - class_attributes: None, - revalidation_match_results: revalidation_match_results, + validation_data: validation_data, }); } @@ -193,43 +395,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 } @@ -237,11 +435,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 ); @@ -254,7 +451,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), @@ -293,7 +490,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 { @@ -303,11 +500,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 { @@ -319,66 +515,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::have_same_presentational_hints(target, candidate) { 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/servo/components/style/stylist.rs b/servo/components/style/stylist.rs index ed29425f5237..def4de9eab10 100644 --- a/servo/components/style/stylist.rs +++ b/servo/components/style/stylist.rs @@ -1397,7 +1397,7 @@ impl Rule { /// This represents the declarations in a given declaration block for a given /// importance. #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct ApplicableDeclarationBlock { /// The style source, either a style rule, or a property declaration block. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] diff --git a/servo/components/style/traversal.rs b/servo/components/style/traversal.rs index 30f9da0567a1..a21b83be2e16 100644 --- a/servo/components/style/traversal.rs +++ b/servo/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; @@ -800,9 +800,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);