style: avoid selector refcount churn during invalidation.

Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>

components/style/data.rs

@@ -17,6 +17,7 @@ use selector_parser::{EAGER_PSEUDO_COUNT, PseudoElement, RestyleDamage};
 use selectors::NthIndexCache;
 use servo_arc::Arc;
 use shared_lock::StylesheetGuards;
+use smallvec::SmallVec;
 use std::fmt;
 use std::mem;
 use std::ops::{Deref, DerefMut};
@@ -259,8 +260,14 @@ impl ElementData {
             return InvalidationResult::empty();
         }
 
+        let mut xbl_stylists = SmallVec::<[_; 3]>::new();
+        let cut_off_inheritance =
+            element.each_xbl_stylist(|s| xbl_stylists.push(s));
+
         let mut processor = StateAndAttrInvalidationProcessor::new(
             shared_context,
+            &xbl_stylists,
+            cut_off_inheritance,
             element,
             self,
             nth_index_cache,

components/style/dom.rs

@@ -686,9 +686,10 @@ pub trait TElement
     /// Implements Gecko's `nsBindingManager::WalkRules`.
     ///
     /// Returns whether to cut off the inheritance.
-    fn each_xbl_stylist<F>(&self, _: F) -> bool
+    fn each_xbl_stylist<'a, F>(&self, _: F) -> bool
     where
-        F: FnMut(&Stylist),
+        Self: 'a,
+        F: FnMut(AtomicRef<'a, Stylist>),
     {
         false
     }

components/style/gecko/wrapper.rs

@@ -17,7 +17,7 @@
 use CaseSensitivityExt;
 use app_units::Au;
 use applicable_declarations::ApplicableDeclarationBlock;
-use atomic_refcell::{AtomicRefCell, AtomicRefMut};
+use atomic_refcell::{AtomicRefCell, AtomicRef, AtomicRefMut};
 use context::{QuirksMode, SharedStyleContext, PostAnimationTasks, UpdateAnimationsTasks};
 use data::ElementData;
 use dom::{LayoutIterator, NodeInfo, TElement, TNode};
@@ -355,9 +355,9 @@ impl<'lb> GeckoXBLBinding<'lb> {
         }
     }
 
-    fn each_xbl_stylist<F>(self, f: &mut F)
+    fn each_xbl_stylist<F>(&self, f: &mut F)
     where
-        F: FnMut(&Stylist),
+        F: FnMut(AtomicRef<'lb, Stylist>),
     {
         if let Some(base) = self.base_binding() {
             base.each_xbl_stylist(f);
@@ -369,7 +369,7 @@ impl<'lb> GeckoXBLBinding<'lb> {
 
         if let Some(raw_data) = raw_data {
             let data = PerDocumentStyleData::from_ffi(&*raw_data).borrow();
-            f(&data.stylist);
+            f(AtomicRef::map(data, |d| &d.stylist));
         }
     }
 }
@@ -476,7 +476,7 @@ impl<'le> GeckoElement<'le> {
     }
 
     #[inline]
-    fn get_xbl_binding(&self) -> Option<GeckoXBLBinding> {
+    fn get_xbl_binding(&self) -> Option<GeckoXBLBinding<'le>> {
         if self.flags() & (structs::NODE_MAY_BE_IN_BINDING_MNGR as u32) == 0 {
             return None;
         }
@@ -485,7 +485,7 @@ impl<'le> GeckoElement<'le> {
     }
 
     #[inline]
-    fn get_xbl_binding_with_content(&self) -> Option<GeckoXBLBinding> {
+    fn get_xbl_binding_with_content(&self) -> Option<GeckoXBLBinding<'le>> {
         self.get_xbl_binding()
             .and_then(|b| b.get_binding_with_content())
     }
@@ -1246,9 +1246,10 @@ impl<'le> TElement for GeckoElement<'le> {
         self.may_have_animations() && unsafe { Gecko_ElementHasCSSTransitions(self.0) }
     }
 
-    fn each_xbl_stylist<F>(&self, mut f: F) -> bool
+    fn each_xbl_stylist<'a, F>(&self, mut f: F) -> bool
     where
-        F: FnMut(&Stylist),
+        'le: 'a,
+        F: FnMut(AtomicRef<'a, Stylist>),
     {
         // Walk the binding scope chain, starting with the binding attached to
         // our content, up till we run out of scopes or we get cut off.

components/style/invalidation/element/collector.rs

@@ -6,6 +6,7 @@
 //! changes.
 
 use Atom;
+use atomic_refcell::AtomicRef;
 use context::{QuirksMode, SharedStyleContext};
 use data::ElementData;
 use dom::TElement;
@@ -21,6 +22,7 @@ use selectors::attr::CaseSensitivity;
 use selectors::matching::{MatchingContext, MatchingMode, VisitedHandlingMode};
 use selectors::matching::matches_selector;
 use smallvec::SmallVec;
+use stylist::Stylist;
 
 #[derive(Debug, PartialEq)]
 enum VisitedDependent {
@@ -29,7 +31,7 @@ enum VisitedDependent {
 }
 
 /// The collector implementation.
-struct Collector<'a, 'b: 'a, E>
+struct Collector<'a, 'b: 'a, 'selectors: 'a, E>
 where
     E: TElement,
 {
@@ -44,8 +46,8 @@ where
     classes_removed: &'a SmallVec<[Atom; 8]>,
     classes_added: &'a SmallVec<[Atom; 8]>,
     state_changes: ElementState,
-    descendant_invalidations: &'a mut InvalidationVector,
+    descendant_invalidations: &'a mut InvalidationVector<'selectors>,
-    sibling_invalidations: &'a mut InvalidationVector,
+    sibling_invalidations: &'a mut InvalidationVector<'selectors>,
     invalidates_self: bool,
 }
 
@@ -53,6 +55,8 @@ where
 /// changes.
 pub struct StateAndAttrInvalidationProcessor<'a, 'b: 'a, E: TElement> {
     shared_context: &'a SharedStyleContext<'b>,
+    xbl_stylists: &'a [AtomicRef<'b, Stylist>],
+    cut_off_inheritance: bool,
     element: E,
     data: &'a mut ElementData,
     matching_context: MatchingContext<'a, E::Impl>,
@@ -62,6 +66,8 @@ impl<'a, 'b: 'a, E: TElement> StateAndAttrInvalidationProcessor<'a, 'b, E> {
     /// Creates a new StateAndAttrInvalidationProcessor.
     pub fn new(
         shared_context: &'a SharedStyleContext<'b>,
+        xbl_stylists: &'a [AtomicRef<'b, Stylist>],
+        cut_off_inheritance: bool,
         element: E,
         data: &'a mut ElementData,
         nth_index_cache: Option<&'a mut NthIndexCache>,
@@ -74,11 +80,18 @@ impl<'a, 'b: 'a, E: TElement> StateAndAttrInvalidationProcessor<'a, 'b, E> {
             shared_context.quirks_mode(),
         );
 
-        Self { shared_context, element, data, matching_context }
+        Self {
+            shared_context,
+            xbl_stylists,
+            cut_off_inheritance,
+            element,
+            data,
+            matching_context,
+        }
     }
 }
 
-impl<'a, 'b: 'a, E> InvalidationProcessor<'a, E> for StateAndAttrInvalidationProcessor<'a, 'b, E>
+impl<'a, 'b: 'a, E: 'a> InvalidationProcessor<'a, E> for StateAndAttrInvalidationProcessor<'a, 'b, E>
 where
     E: TElement,
 {
@@ -94,9 +107,9 @@ where
     fn collect_invalidations(
         &mut self,
         element: E,
-        _self_invalidations: &mut InvalidationVector,
+        _self_invalidations: &mut InvalidationVector<'a>,
-        descendant_invalidations: &mut InvalidationVector,
+        descendant_invalidations: &mut InvalidationVector<'a>,
-        sibling_invalidations: &mut InvalidationVector,
+        sibling_invalidations: &mut InvalidationVector<'a>,
     ) -> bool {
         debug_assert!(element.has_snapshot(), "Why bothering?");
 
@@ -175,24 +188,21 @@ where
                 invalidates_self: false,
             };
 
+            // TODO(emilio): We could use the cut_off_inheritance from XBL to
+            // skip user and author stuff in here if it's true.
             self.shared_context.stylist.each_invalidation_map(|invalidation_map| {
                 collector.collect_dependencies_in_invalidation_map(invalidation_map);
             });
 
-            // TODO(emilio): Consider storing dependencies from the UA sheet in
+            for stylist in self.xbl_stylists {
-            // a different map. If we do that, we can skip the stuff on the
+                // FIXME(emilio): Replace with assert / remove when we
-            // shared stylist iff cut_off_inheritance is true, and we can look
+                // figure out what to do with the quirks mode mismatches
-            // just at that map.
+                // (that is, when bug 1406875 is properly fixed).
-            let _cut_off_inheritance =
+                collector.quirks_mode = stylist.quirks_mode();
-                element.each_xbl_stylist(|stylist| {
+                stylist.each_invalidation_map(|invalidation_map| {
-                    // FIXME(emilio): Replace with assert / remove when we
+                    collector.collect_dependencies_in_invalidation_map(invalidation_map);
-                    // figure out what to do with the quirks mode mismatches
+                })
-                    // (that is, when bug 1406875 is properly fixed).
+            }
-                    collector.quirks_mode = stylist.quirks_mode();
-                    stylist.each_invalidation_map(|invalidation_map| {
-                        collector.collect_dependencies_in_invalidation_map(invalidation_map);
-                    });
-                });
 
             collector.invalidates_self
         };
@@ -259,13 +269,14 @@ where
     }
 }
 
-impl<'a, 'b, E> Collector<'a, 'b, E>
+impl<'a, 'b, 'selectors, E> Collector<'a, 'b, 'selectors, E>
 where
     E: TElement,
+    'selectors: 'a,
 {
     fn collect_dependencies_in_invalidation_map(
         &mut self,
-        map: &InvalidationMap,
+        map: &'selectors InvalidationMap,
     ) {
         let quirks_mode = self.quirks_mode;
         let removed_id = self.removed_id;
@@ -316,7 +327,7 @@ where
 
     fn collect_dependencies_in_map(
         &mut self,
-        map: &SelectorMap<Dependency>,
+        map: &'selectors SelectorMap<Dependency>,
     ) {
         map.lookup_with_additional(
             self.lookup_element,
@@ -332,7 +343,7 @@ where
 
     fn collect_state_dependencies(
         &mut self,
-        map: &SelectorMap<StateDependency>,
+        map: &'selectors SelectorMap<StateDependency>,
         state_changes: ElementState,
     ) {
         map.lookup_with_additional(
@@ -416,7 +427,7 @@ where
 
     fn scan_dependency(
         &mut self,
-        dependency: &Dependency,
+        dependency: &'selectors Dependency,
         is_visited_dependent: VisitedDependent,
     ) {
         debug!("TreeStyleInvalidator::scan_dependency({:?}, {:?}, {:?})",
@@ -469,7 +480,7 @@ where
         }
     }
 
-    fn note_dependency(&mut self, dependency: &Dependency) {
+    fn note_dependency(&mut self, dependency: &'selectors Dependency) {
         if dependency.affects_self() {
             self.invalidates_self = true;
         }
@@ -479,14 +490,14 @@ where
             debug_assert_ne!(dependency.selector_offset, dependency.selector.len());
             debug_assert!(!dependency.affects_later_siblings());
             self.descendant_invalidations.push(Invalidation::new(
-                dependency.selector.clone(),
+                &dependency.selector,
                 dependency.selector.len() - dependency.selector_offset + 1,
             ));
         } else if dependency.affects_later_siblings() {
             debug_assert_ne!(dependency.selector_offset, 0);
             debug_assert_ne!(dependency.selector_offset, dependency.selector.len());
             self.sibling_invalidations.push(Invalidation::new(
-                dependency.selector.clone(),
+                &dependency.selector,
                 dependency.selector.len() - dependency.selector_offset + 1,
             ));
         }

components/style/invalidation/element/invalidator.rs

@@ -33,9 +33,9 @@ where
     fn collect_invalidations(
         &mut self,
         element: E,
-        self_invalidations: &mut InvalidationVector,
+        self_invalidations: &mut InvalidationVector<'a>,
-        descendant_invalidations: &mut InvalidationVector,
+        descendant_invalidations: &mut InvalidationVector<'a>,
-        sibling_invalidations: &mut InvalidationVector,
+        sibling_invalidations: &mut InvalidationVector<'a>,
     ) -> bool;
 
     /// Returns whether the invalidation process should process the descendants
@@ -68,7 +68,7 @@ where
 }
 
 /// A vector of invalidations, optimized for small invalidation sets.
-pub type InvalidationVector = SmallVec<[Invalidation; 10]>;
+pub type InvalidationVector<'a> = SmallVec<[Invalidation<'a>; 10]>;
 
 /// The kind of invalidation we're processing.
 ///
@@ -83,8 +83,8 @@ enum InvalidationKind {
 /// An `Invalidation` is a complex selector that describes which elements,
 /// relative to a current element we are processing, must be restyled.
 #[derive(Clone)]
-pub struct Invalidation {
+pub struct Invalidation<'a> {
-    selector: Selector<SelectorImpl>,
+    selector: &'a Selector<SelectorImpl>,
     /// The offset of the selector pointing to a compound selector.
     ///
     /// This order is a "parse order" offset, that is, zero is the leftmost part
@@ -99,9 +99,9 @@ pub struct Invalidation {
     matched_by_any_previous: bool,
 }
 
-impl Invalidation {
+impl<'a> Invalidation<'a> {
     /// Create a new invalidation for a given selector and offset.
-    pub fn new(selector: Selector<SelectorImpl>, offset: usize) -> Self {
+    pub fn new(selector: &'a Selector<SelectorImpl>, offset: usize) -> Self {
         Self {
             selector,
             offset,
@@ -140,7 +140,7 @@ impl Invalidation {
     }
 }
 
-impl fmt::Debug for Invalidation {
+impl<'a> fmt::Debug for Invalidation<'a> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use cssparser::ToCss;
 
@@ -259,7 +259,7 @@ where
     /// was invalidated.
     fn invalidate_siblings(
         &mut self,
-        sibling_invalidations: &mut InvalidationVector,
+        sibling_invalidations: &mut InvalidationVector<'b>,
     ) -> bool {
         if sibling_invalidations.is_empty() {
             return false;
@@ -300,7 +300,7 @@ where
     fn invalidate_pseudo_element_or_nac(
         &mut self,
         child: E,
-        invalidations: &InvalidationVector
+        invalidations: &InvalidationVector<'b>,
     ) -> bool {
         let mut sibling_invalidations = InvalidationVector::new();
 
@@ -325,8 +325,8 @@ where
     fn invalidate_child(
         &mut self,
         child: E,
-        invalidations: &InvalidationVector,
+        invalidations: &InvalidationVector<'b>,
-        sibling_invalidations: &mut InvalidationVector,
+        sibling_invalidations: &mut InvalidationVector<'b>,
     ) -> bool {
         let mut invalidations_for_descendants = InvalidationVector::new();
 
@@ -368,7 +368,7 @@ where
 
     fn invalidate_nac(
         &mut self,
-        invalidations: &InvalidationVector,
+        invalidations: &InvalidationVector<'b>,
     ) -> bool {
         let mut any_nac_root = false;
 
@@ -386,7 +386,7 @@ where
     fn invalidate_dom_descendants_of(
         &mut self,
         parent: E::ConcreteNode,
-        invalidations: &InvalidationVector,
+        invalidations: &InvalidationVector<'b>,
     ) -> bool {
         let mut any_descendant = false;
 
@@ -420,7 +420,7 @@ where
     /// descendants, and invalidate style on them.
     fn invalidate_descendants(
         &mut self,
-        invalidations: &InvalidationVector,
+        invalidations: &InvalidationVector<'b>,
     ) -> bool {
         if invalidations.is_empty() {
             return false;
@@ -485,8 +485,8 @@ where
     /// Returns whether invalidated the current element's style.
     fn process_sibling_invalidations(
         &mut self,
-        descendant_invalidations: &mut InvalidationVector,
+        descendant_invalidations: &mut InvalidationVector<'b>,
-        sibling_invalidations: &mut InvalidationVector,
+        sibling_invalidations: &mut InvalidationVector<'b>,
     ) -> bool {
         let mut i = 0;
         let mut new_sibling_invalidations = InvalidationVector::new();
@@ -520,9 +520,9 @@ where
     /// Returns whether our style was invalidated as a result.
     fn process_descendant_invalidations(
         &mut self,
-        invalidations: &InvalidationVector,
+        invalidations: &InvalidationVector<'b>,
-        descendant_invalidations: &mut InvalidationVector,
+        descendant_invalidations: &mut InvalidationVector<'b>,
-        sibling_invalidations: &mut InvalidationVector,
+        sibling_invalidations: &mut InvalidationVector<'b>,
     ) -> bool {
         let mut invalidated = false;
 
@@ -553,9 +553,9 @@ where
     /// down in the tree.
     fn process_invalidation(
         &mut self,
-        invalidation: &Invalidation,
+        invalidation: &Invalidation<'b>,
-        descendant_invalidations: &mut InvalidationVector,
+        descendant_invalidations: &mut InvalidationVector<'b>,
-        sibling_invalidations: &mut InvalidationVector,
+        sibling_invalidations: &mut InvalidationVector<'b>,
         invalidation_kind: InvalidationKind,
     ) -> SingleInvalidationResult {
         debug!("TreeStyleInvalidator::process_invalidation({:?}, {:?}, {:?})",
@@ -627,7 +627,7 @@ where
 
 
                 let next_invalidation = Invalidation {
-                    selector: invalidation.selector.clone(),
+                    selector: invalidation.selector,
                     offset: next_combinator_offset + 1,
                     matched_by_any_previous: false,
                 };

components/style/selector_map.rs

@@ -306,9 +306,10 @@ impl<T: SelectorMapEntry> SelectorMap<T> {
     /// FIXME(bholley) This overlaps with SelectorMap<Rule>::get_all_matching_rules,
     /// but that function is extremely hot and I'd rather not rearrange it.
     #[inline]
-    pub fn lookup<E, F>(&self, element: E, quirks_mode: QuirksMode, f: &mut F) -> bool
+    pub fn lookup<'a, E, F>(&'a self, element: E, quirks_mode: QuirksMode, f: &mut F) -> bool
-        where E: TElement,
+    where
-              F: FnMut(&T) -> bool
+        E: TElement,
+        F: FnMut(&'a T) -> bool
     {
         // Id.
         if let Some(id) = element.get_id() {
@@ -366,15 +367,17 @@ impl<T: SelectorMapEntry> SelectorMap<T> {
     ///
     /// Returns false if the callback ever returns false.
     #[inline]
-    pub fn lookup_with_additional<E, F>(&self,
+    pub fn lookup_with_additional<'a, E, F>(
-                                        element: E,
+        &'a self,
-                                        quirks_mode: QuirksMode,
+        element: E,
-                                        additional_id: Option<&Atom>,
+        quirks_mode: QuirksMode,
-                                        additional_classes: &[Atom],
+        additional_id: Option<&Atom>,
-                                        f: &mut F)
+        additional_classes: &[Atom],
-                                        -> bool
+        f: &mut F,
-        where E: TElement,
+    ) -> bool
-              F: FnMut(&T) -> bool
+    where
+        E: TElement,
+        F: FnMut(&'a T) -> bool
     {
         // Do the normal lookup.
         if !self.lookup(element, quirks_mode, f) {

components/style/stylist.rs

@@ -484,8 +484,9 @@ impl Stylist {
     /// NOTE(heycam) This might be better as an `iter_invalidation_maps`, once
     /// we have `impl trait` and can return that easily without bothering to
     /// create a whole new iterator type.
-    pub fn each_invalidation_map<F>(&self, mut f: F)
+    pub fn each_invalidation_map<'a, F>(&'a self, mut f: F)
-        where F: FnMut(&InvalidationMap)
+    where
+        F: FnMut(&'a InvalidationMap)
     {
         for (data, _) in self.cascade_data.iter_origins() {
             f(&data.invalidation_map)