From 5cf915118ee12a05fd61619347d319e2d63a61fd Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 8 May 2017 16:06:15 +0800 Subject: [PATCH 1/2] style: Record names that appear in attribute selectors. --- components/style/stylist.rs | 68 ++++++++++++++++++++++++++++++++++++- ports/geckolib/glue.rs | 7 ++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index c298216d8fa..5f5f377ca68 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -27,7 +27,7 @@ use selectors::Element; use selectors::bloom::BloomFilter; use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS}; use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_selector}; -use selectors::parser::{Combinator, Component, Selector, SelectorInner, SelectorIter}; +use selectors::parser::{AttrSelector, Combinator, Component, Selector, SelectorInner, SelectorIter}; use selectors::parser::{SelectorMethods, LocalName as LocalNameSelector}; use selectors::visitor::SelectorVisitor; use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; @@ -118,6 +118,23 @@ pub struct Stylist { /// Selector dependencies used to compute restyle hints. dependencies: DependencySet, + /// The attribute local names that appear in attribute selectors. Used + /// to avoid taking element snapshots when an irrelevant attribute changes. + /// (We don't bother storing the namespace, since namespaced attributes + /// are rare.) + /// + /// FIXME(heycam): This doesn't really need to be a counting Bloom filter. + #[cfg_attr(feature = "servo", ignore_heap_size_of = "just an array")] + attribute_dependencies: BloomFilter, + + /// 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, + /// Selectors that require explicit cache revalidation (i.e. which depend /// on state that is not otherwise visible to the cache, like attributes or /// tree-structural state like child index and pseudos). @@ -184,6 +201,8 @@ impl Stylist { rules_source_order: 0, rule_tree: RuleTree::new(), dependencies: DependencySet::new(), + attribute_dependencies: BloomFilter::new(), + style_attribute_dependency: false, selectors_for_cache_revalidation: SelectorMap::new(), num_selectors: 0, num_declarations: 0, @@ -254,6 +273,8 @@ impl Stylist { self.rules_source_order = 0; // We want to keep rule_tree around across stylist rebuilds. self.dependencies.clear(); + self.attribute_dependencies.clear(); + self.style_attribute_dependency = false; self.selectors_for_cache_revalidation = SelectorMap::new(); self.num_selectors = 0; self.num_declarations = 0; @@ -375,6 +396,7 @@ impl Stylist { self.add_rule_to_map(selector, locked, stylesheet); self.dependencies.note_selector(selector); self.note_for_revalidation(selector); + self.note_attribute_dependencies(selector); } self.rules_source_order += 1; } @@ -434,6 +456,28 @@ impl Stylist { } } + /// Returns whether the given attribute might appear in an attribute + /// selector of some rule in the stylist. + pub fn might_have_attribute_dependency(&self, + local_name: &::LocalName) + -> bool { + #[cfg(feature = "servo")] + let style_lower_name = local_name!("style"); + #[cfg(feature = "gecko")] + let style_lower_name = atom!("style"); + + if *local_name == style_lower_name { + self.style_attribute_dependency + } else { + self.attribute_dependencies.might_contain(local_name) + } + } + + #[inline] + fn note_attribute_dependencies(&mut self, selector: &Selector) { + selector.visit(&mut AttributeDependencyVisitor(self)); + } + /// Computes the style for a given "precomputed" pseudo-element, taking the /// universal rules and applying them. /// @@ -962,6 +1006,28 @@ impl Drop for Stylist { } } +/// Visitor to collect names that appear in attribute selectors. +struct AttributeDependencyVisitor<'a>(&'a mut Stylist); + +impl<'a> SelectorVisitor for AttributeDependencyVisitor<'a> { + type Impl = SelectorImpl; + + fn visit_attribute_selector(&mut self, selector: &AttrSelector) -> bool { + #[cfg(feature = "servo")] + let style_lower_name = local_name!("style"); + #[cfg(feature = "gecko")] + let style_lower_name = atom!("style"); + + if selector.lower_name == style_lower_name { + self.0.style_attribute_dependency = true; + } else { + self.0.attribute_dependencies.insert(&selector.name); + self.0.attribute_dependencies.insert(&selector.lower_name); + } + true + } +} + /// Visitor determine whether a selector requires cache revalidation. /// /// Note that we just check simple selectors and eagerly return when the first diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 9d37f8f01f8..7516eb80b9e 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2331,3 +2331,10 @@ pub extern "C" fn Servo_StyleSet_ResolveForDeclarations(raw_data: RawServoStyleS parent_style, declarations.clone()).into_strong() } + +#[no_mangle] +pub extern "C" fn Servo_StyleSet_MightHaveAttributeDependency(raw_data: RawServoStyleSetBorrowed, + local_name: *mut nsIAtom) -> bool { + let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); + unsafe { Atom::with(local_name, &mut |atom| data.stylist.might_have_attribute_dependency(atom)) } +} From b405a1cb678ca903e691c2abb6b5f400dd3801f7 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Tue, 9 May 2017 18:07:40 +0800 Subject: [PATCH 2/2] style: Record ElementState bits that selectors depend on. --- components/style/stylist.rs | 34 ++++++++++++++++++++++++++++------ ports/geckolib/glue.rs | 8 ++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 5f5f377ca68..5df061e48f6 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -11,6 +11,7 @@ use bit_vec::BitVec; use context::QuirksMode; use data::ComputedStyle; use dom::{AnimationRules, TElement}; +use element_state::ElementState; use error_reporting::RustLogReporter; use font_metrics::FontMetricsProvider; use keyframes::KeyframesAnimation; @@ -135,6 +136,11 @@ pub struct Stylist { /// 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 + /// `attribute_dependencies`, this is used to avoid taking element snapshots + /// when an irrelevant element state bit changes. + state_dependencies: ElementState, + /// Selectors that require explicit cache revalidation (i.e. which depend /// on state that is not otherwise visible to the cache, like attributes or /// tree-structural state like child index and pseudos). @@ -203,6 +209,7 @@ impl Stylist { dependencies: DependencySet::new(), attribute_dependencies: BloomFilter::new(), style_attribute_dependency: false, + state_dependencies: ElementState::empty(), selectors_for_cache_revalidation: SelectorMap::new(), num_selectors: 0, num_declarations: 0, @@ -275,6 +282,7 @@ impl Stylist { self.dependencies.clear(); self.attribute_dependencies.clear(); self.style_attribute_dependency = false; + self.state_dependencies = ElementState::empty(); self.selectors_for_cache_revalidation = SelectorMap::new(); self.num_selectors = 0; self.num_declarations = 0; @@ -396,7 +404,7 @@ impl Stylist { self.add_rule_to_map(selector, locked, stylesheet); self.dependencies.note_selector(selector); self.note_for_revalidation(selector); - self.note_attribute_dependencies(selector); + self.note_attribute_and_state_dependencies(selector); } self.rules_source_order += 1; } @@ -473,9 +481,15 @@ impl Stylist { } } + /// Returns whether the given ElementState bit is relied upon by a selector + /// of some rule in the stylist. + pub fn has_state_dependency(&self, state: ElementState) -> bool { + self.state_dependencies.intersects(state) + } + #[inline] - fn note_attribute_dependencies(&mut self, selector: &Selector) { - selector.visit(&mut AttributeDependencyVisitor(self)); + fn note_attribute_and_state_dependencies(&mut self, selector: &Selector) { + selector.visit(&mut AttributeAndStateDependencyVisitor(self)); } /// Computes the style for a given "precomputed" pseudo-element, taking the @@ -1006,10 +1020,11 @@ impl Drop for Stylist { } } -/// Visitor to collect names that appear in attribute selectors. -struct AttributeDependencyVisitor<'a>(&'a mut Stylist); +/// Visitor to collect names that appear in attribute selectors and any +/// dependencies on ElementState bits. +struct AttributeAndStateDependencyVisitor<'a>(&'a mut Stylist); -impl<'a> SelectorVisitor for AttributeDependencyVisitor<'a> { +impl<'a> SelectorVisitor for AttributeAndStateDependencyVisitor<'a> { type Impl = SelectorImpl; fn visit_attribute_selector(&mut self, selector: &AttrSelector) -> bool { @@ -1026,6 +1041,13 @@ impl<'a> SelectorVisitor for AttributeDependencyVisitor<'a> { } true } + + fn visit_simple_selector(&mut self, s: &Component) -> bool { + if let Component::NonTSPseudoClass(ref p) = *s { + self.0.state_dependencies.insert(p.state_flag()); + } + true + } } /// Visitor determine whether a selector requires cache revalidation. diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 7516eb80b9e..5cb6cd92d12 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -17,6 +17,7 @@ use style::context::{ThreadLocalStyleContext, ThreadLocalStyleContextCreationInf use style::data::{ElementData, ElementStyles, RestyleData}; use style::dom::{AnimationOnlyDirtyDescendants, DirtyDescendants}; use style::dom::{ShowSubtreeData, TElement, TNode}; +use style::element_state::ElementState; use style::error_reporting::RustLogReporter; use style::font_metrics::get_metrics_provider_for_product; use style::gecko::data::{PerDocumentStyleData, PerDocumentStyleDataImpl}; @@ -2338,3 +2339,10 @@ pub extern "C" fn Servo_StyleSet_MightHaveAttributeDependency(raw_data: RawServo let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); unsafe { Atom::with(local_name, &mut |atom| data.stylist.might_have_attribute_dependency(atom)) } } + +#[no_mangle] +pub extern "C" fn Servo_StyleSet_HasStateDependency(raw_data: RawServoStyleSetBorrowed, + state: u64) -> bool { + let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); + data.stylist.has_state_dependency(ElementState::from_bits_truncate(state)) +}