Auto merge of servo#16943 - emilio:bloom-restyle-hints, r=bholley

bors-servo · web-flow · commit d98abe1397ae · 2017-05-20T17:17:05.000-05:00
style: Make a bloom filter arrive to restyle hint computation.  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16943)
diff --git a/components/style/bloom.rs b/components/style/bloom.rs
@@ -9,6 +9,7 @@
 
 use dom::{SendElement, TElement};
 use selectors::bloom::BloomFilter;
+use smallvec::SmallVec;
 
 /// A struct that allows us to fast-reject deep descendant selectors avoiding
 /// selector-matching.
@@ -223,7 +224,7 @@ impl<E: TElement> StyleBloom<E> {
 
         // Let's collect the parents we are going to need to insert once we've
         // found the common one.
-        let mut parents_to_insert = vec![];
+        let mut parents_to_insert = SmallVec::<[E; 8]>::new();
 
         // If the bloom filter still doesn't have enough elements, the common
         // parent is up in the dom.
@@ -245,10 +246,12 @@ impl<E: TElement> StyleBloom<E> {
         // Not-so-happy case: Parent's don't match, so we need to keep going up
         // until we find a common ancestor.
         //
-        // Gecko currently models native anonymous content that conceptually hangs
-        // off the document (such as scrollbars) as a separate subtree from the
-        // document root.  Thus it's possible with Gecko that we do not find any
-        // common ancestor.
+        // Gecko currently models native anonymous content that conceptually
+        // hangs off the document (such as scrollbars) as a separate subtree
+        // from the document root.
+        //
+        // Thus it's possible with Gecko that we do not find any common
+        // ancestor.
         while **self.elements.last().unwrap() != common_parent {
             parents_to_insert.push(common_parent);
             self.pop().unwrap();
diff --git a/components/style/data.rs b/components/style/data.rs
@@ -4,13 +4,11 @@
 
 //! Per-node data used in style calculation.
 
-#![deny(missing_docs)]
-
 use context::SharedStyleContext;
 use dom::TElement;
 use properties::ComputedValues;
 use properties::longhands::display::computed_value as display;
-use restyle_hints::{RestyleReplacements, RestyleHint};
+use restyle_hints::{HintComputationContext, RestyleReplacements, RestyleHint};
 use rule_tree::StrongRuleNode;
 use selector_parser::{EAGER_PSEUDO_COUNT, PseudoElement, RestyleDamage};
 use shared_lock::StylesheetGuards;
@@ -372,15 +370,16 @@ impl ElementData {
     /// explicit sibling restyle hints from the stored restyle hint.
     ///
     /// Returns true if later siblings must be restyled.
-    pub fn compute_final_hint<E: TElement>(
+    pub fn compute_final_hint<'a, E: TElement>(
         &mut self,
         element: E,
-        context: &SharedStyleContext)
+        shared_context: &SharedStyleContext,
+        hint_context: HintComputationContext<'a, E>)
         -> bool
     {
         debug!("compute_final_hint: {:?}, {:?}",
                element,
-               context.traversal_flags);
+               shared_context.traversal_flags);
 
         let mut hint = match self.get_restyle() {
             Some(r) => r.hint.0.clone(),
@@ -395,7 +394,11 @@ impl ElementData {
                 element.implemented_pseudo_element());
 
         if element.has_snapshot() && !element.handled_snapshot() {
-            hint.insert(context.stylist.compute_restyle_hint(&element, context.snapshot_map));
+            let snapshot_hint =
+                shared_context.stylist.compute_restyle_hint(&element,
+                                                            shared_context,
+                                                            hint_context);
+            hint.insert(snapshot_hint);
             unsafe { element.set_handled_snapshot() }
             debug_assert!(element.handled_snapshot());
         }
diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs
@@ -9,6 +9,7 @@
 use Atom;
 use LocalName;
 use Namespace;
+use context::{SharedStyleContext, ThreadLocalStyleContext};
 use dom::TElement;
 use element_state::*;
 #[cfg(feature = "gecko")]
@@ -825,6 +826,35 @@ pub struct DependencySet {
     dependencies: SelectorMap<Dependency>,
 }
 
+/// The data that we need to compute a given restyle hint.
+pub enum HintComputationContext<'a, E: 'a>
+    where E: TElement,
+{
+    /// The data we need to compute a restyle hint for the root of the
+    /// traversal.
+    ///
+    /// We don't bother with the bloom filter there for multiple reasons:
+    ///
+    ///  * The root of the traversal uses to be the root of the document, so we
+    ///    don't gain much using a bloom filter.
+    ///
+    ///  * The chances that a non-root-element root of the traversal has a
+    ///    snapshot is quite low.
+    Root,
+
+    /// The data we need to compute a restyle hint for a child.
+    ///
+    /// This needs a full-blown style context in order to get the selector
+    /// filters up-to-date, and the dom depth in order to insert into the filter
+    /// properly if needed.
+    Child {
+        /// The thread-local context, that holds the bloom filter alive.
+        local_context: &'a mut ThreadLocalStyleContext<E>,
+        /// The dom depth of this element.
+        dom_depth: usize,
+    }
+}
+
 impl DependencySet {
     /// Adds a selector to this `DependencySet`.
     pub fn note_selector(&mut self, selector: &Selector<SelectorImpl>) {
@@ -921,16 +951,17 @@ impl DependencySet {
 
     /// Compute a restyle hint given an element and a snapshot, per the rules
     /// explained in the rest of the documentation.
-    pub fn compute_hint<E>(
+    pub fn compute_hint<'a, E>(
         &self,
         el: &E,
-        snapshots: &SnapshotMap)
+        shared_context: &SharedStyleContext,
+        hint_context: HintComputationContext<'a, E>)
         -> RestyleHint
         where E: TElement,
     {
         debug_assert!(el.has_snapshot(), "Shouldn't be here!");
-
-        let snapshot_el = ElementWrapper::new(el.clone(), snapshots);
+        let snapshot_el =
+            ElementWrapper::new(*el, shared_context.snapshot_map);
         let snapshot =
             snapshot_el.snapshot().expect("has_snapshot lied so badly");
 
@@ -960,8 +991,30 @@ impl DependencySet {
             });
         }
 
-        // FIXME(emilio): A bloom filter here would be neat.
-        let mut matching_context =
+        let bloom_filter = match hint_context {
+            HintComputationContext::Root => None,
+            HintComputationContext::Child { mut local_context, dom_depth } => {
+                local_context
+                    .bloom_filter
+                    .insert_parents_recovering(*el, dom_depth);
+                local_context.bloom_filter.assert_complete(*el);
+                Some(local_context.bloom_filter.filter())
+            }
+        };
+
+        let mut element_matching_context =
+            MatchingContext::new(MatchingMode::Normal, bloom_filter);
+
+        // NOTE(emilio): We can't use the bloom filter for snapshots, given that
+        // arbitrary elements in the parent chain may have mutated their
+        // id's/classes, which means that they won't be in the filter, and as
+        // such we may fast-reject selectors incorrectly.
+        //
+        // We may be able to improve this if we record as we go down the tree
+        // whether any parent had a snapshot, and whether those snapshots were
+        // taken due to an element class/id change, but it's not clear we _need_
+        // it right now.
+        let mut snapshot_matching_context =
             MatchingContext::new(MatchingMode::Normal, None);
 
         let lookup_element = if el.implemented_pseudo_element().is_some() {
@@ -989,11 +1042,11 @@ impl DependencySet {
             // change its matching behavior here.
             let matched_then =
                 matches_selector(&dep.selector, &snapshot_el,
-                                 &mut matching_context,
+                                 &mut snapshot_matching_context,
                                  &mut |_, _| {});
             let matches_now =
                 matches_selector(&dep.selector, el,
-                                 &mut matching_context,
+                                 &mut element_matching_context,
                                  &mut |_, _| {});
             if matched_then != matches_now {
                 hint.insert_from(&dep.hint);
diff --git a/components/style/stylist.rs b/components/style/stylist.rs
@@ -4,11 +4,9 @@
 
 //! Selector matching.
 
-#![deny(missing_docs)]
-
 use {Atom, LocalName, Namespace};
 use bit_vec::BitVec;
-use context::QuirksMode;
+use context::{QuirksMode, SharedStyleContext};
 use data::ComputedStyle;
 use dom::{AnimationRules, TElement};
 use element_state::ElementState;
@@ -23,9 +21,9 @@ use properties::{self, CascadeFlags, ComputedValues};
 #[cfg(feature = "servo")]
 use properties::INHERIT_ALL;
 use properties::PropertyDeclarationBlock;
-use restyle_hints::{RestyleHint, DependencySet};
+use restyle_hints::{HintComputationContext, DependencySet, RestyleHint};
 use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource};
-use selector_parser::{SelectorImpl, PseudoElement, SnapshotMap};
+use selector_parser::{SelectorImpl, PseudoElement};
 use selectors::attr::NamespaceConstraint;
 use selectors::bloom::BloomFilter;
 use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS};
@@ -1029,13 +1027,14 @@ impl Stylist {
     /// Given an element, and a snapshot table that represents a previous state
     /// of the tree, compute the appropriate restyle hint, that is, the kind of
     /// restyle we need to do.
-    pub fn compute_restyle_hint<E>(&self,
-                                   element: &E,
-                                   snapshots: &SnapshotMap)
-                                   -> RestyleHint
+    pub fn compute_restyle_hint<'a, E>(&self,
+                                       element: &E,
+                                       shared_context: &SharedStyleContext,
+                                       context: HintComputationContext<'a, E>)
+                                       -> RestyleHint
         where E: TElement,
     {
-        self.dependencies.compute_hint(element, snapshots)
+        self.dependencies.compute_hint(element, shared_context, context)
     }
 
     /// Computes styles for a given declaration with parent_style.
diff --git a/components/style/traversal.rs b/components/style/traversal.rs
@@ -9,7 +9,7 @@ use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext};
 use data::{ElementData, ElementStyles, StoredRestyleHint};
 use dom::{DirtyDescendants, NodeInfo, OpaqueNode, TElement, TNode};
 use matching::{ChildCascadeRequirement, MatchMethods, StyleSharingBehavior};
-use restyle_hints::RestyleHint;
+use restyle_hints::{HintComputationContext, RestyleHint};
 use selector_parser::RestyleDamage;
 #[cfg(feature = "servo")] use servo_config::opts;
 use std::borrow::BorrowMut;
@@ -123,7 +123,8 @@ pub trait DomTraversal<E: TElement> : Sync {
     type ThreadLocalContext: Send + BorrowMut<ThreadLocalStyleContext<E>>;
 
     /// Process `node` on the way down, before its children have been processed.
-    fn process_preorder(&self, data: &PerLevelTraversalData,
+    fn process_preorder(&self,
+                        data: &PerLevelTraversalData,
                         thread_local: &mut Self::ThreadLocalContext,
                         node: E::ConcreteNode);
 
@@ -227,7 +228,10 @@ pub trait DomTraversal<E: TElement> : Sync {
         // Expanding snapshots here may create a LATER_SIBLINGS restyle hint, which
         // we propagate to the next sibling element.
         if let Some(mut data) = root.mutate_data() {
-            let later_siblings = data.compute_final_hint(root, shared_context);
+            let later_siblings =
+                data.compute_final_hint(root,
+                                        shared_context,
+                                        HintComputationContext::Root);
             if later_siblings {
                 if let Some(next) = root.next_sibling_element() {
                     if let Some(mut next_data) = next.mutate_data() {
@@ -669,11 +673,12 @@ pub fn recalc_style_at<E, D>(traversal: &D,
             r.damage_handled() | r.damage.handled_for_descendants()
         });
 
-        preprocess_children(traversal,
-                            element,
-                            propagated_hint,
-                            damage_handled,
-                            inherited_style_changed);
+        preprocess_children::<E, D>(context,
+                                    traversal_data,
+                                    element,
+                                    propagated_hint,
+                                    damage_handled,
+                                    inherited_style_changed);
     }
 
     // If we are in a restyle for reconstruction, drop the existing restyle
@@ -740,7 +745,8 @@ fn compute_style<E, D>(_traversal: &D,
         MatchAndCascade => {
             // Ensure the bloom filter is up to date.
             context.thread_local.bloom_filter
-                   .insert_parents_recovering(element, traversal_data.current_dom_depth);
+                   .insert_parents_recovering(element,
+                                              traversal_data.current_dom_depth);
 
             context.thread_local.bloom_filter.assert_complete(element);
             context.thread_local.statistics.elements_matched += 1;
@@ -770,15 +776,17 @@ fn compute_style<E, D>(_traversal: &D,
     }
 }
 
-fn preprocess_children<E, D>(traversal: &D,
+fn preprocess_children<E, D>(context: &mut StyleContext<E>,
+                             parent_traversal_data: &PerLevelTraversalData,
                              element: E,
                              mut propagated_hint: StoredRestyleHint,
                              damage_handled: RestyleDamage,
                              parent_inherited_style_changed: bool)
     where E: TElement,
-          D: DomTraversal<E>
+          D: DomTraversal<E>,
 {
     trace!("preprocess_children: {:?}", element);
+
     // Loop over all the children.
     for child in element.as_node().children() {
         // FIXME(bholley): Add TElement::element_children instead of this.
@@ -787,7 +795,8 @@ fn preprocess_children<E, D>(traversal: &D,
             None => continue,
         };
 
-        let mut child_data = unsafe { D::ensure_element_data(&child).borrow_mut() };
+        let mut child_data =
+            unsafe { D::ensure_element_data(&child).borrow_mut() };
 
         // If the child is unstyled, we don't need to set up any restyling.
         if !child_data.has_styles() {
@@ -798,7 +807,12 @@ fn preprocess_children<E, D>(traversal: &D,
         //
         // NB: This will be a no-op if there's no restyle data and no snapshot.
         let later_siblings =
-            child_data.compute_final_hint(child, traversal.shared_context());
+            child_data.compute_final_hint(child,
+                                          &context.shared,
+                                          HintComputationContext::Child {
+                                              local_context: &mut context.thread_local,
+                                              dom_depth: parent_traversal_data.current_dom_depth + 1,
+                                          });
 
         trace!(" > {:?} -> {:?} + {:?}, pseudo: {:?}, later_siblings: {:?}",
                child,
