Hoist bloom filter into scoped TLS, and remove a bunch of complexity and unsafety from the style system

components/layout/traversal.rs

@@ -9,7 +9,6 @@ use context::{LayoutContext, ScopedThreadLocalLayoutContext, SharedLayoutContext
 use display_list_builder::DisplayListBuildState;
 use flow::{self, PreorderFlowTraversal};
 use flow::{CAN_BE_FRAGMENTED, Flow, ImmutableFlowUtils, PostorderFlowTraversal};
-use gfx::display_list::OpaqueNode;
 use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutNode};
 use servo_config::opts;
 use style::atomic_refcell::AtomicRefCell;
@@ -18,13 +17,12 @@ use style::data::ElementData;
 use style::dom::{TElement, TNode};
 use style::selector_parser::RestyleDamage;
 use style::servo::restyle_damage::{BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT};
-use style::traversal::{DomTraversal, recalc_style_at, remove_from_bloom_filter};
+use style::traversal::{DomTraversal, recalc_style_at};
 use style::traversal::PerLevelTraversalData;
 use wrapper::{GetRawData, LayoutNodeHelpers, LayoutNodeLayoutData};
 
 pub struct RecalcStyleAndConstructFlows {
     shared: SharedLayoutContext,
-    root: OpaqueNode,
 }
 
 impl RecalcStyleAndConstructFlows {
@@ -35,10 +33,9 @@ impl RecalcStyleAndConstructFlows {
 
 impl RecalcStyleAndConstructFlows {
     /// Creates a traversal context, taking ownership of the shared layout context.
-    pub fn new(shared: SharedLayoutContext, root: OpaqueNode) -> Self {
+    pub fn new(shared: SharedLayoutContext) -> Self {
         RecalcStyleAndConstructFlows {
             shared: shared,
-            root: root,
         }
     }
 
@@ -75,7 +72,7 @@ impl<N> DomTraversal<N> for RecalcStyleAndConstructFlows
 
     fn process_postorder(&self, thread_local: &mut Self::ThreadLocalContext, node: N) {
         let context = LayoutContext::new(&self.shared);
-        construct_flows_at(&context, thread_local, self.root, node);
+        construct_flows_at(&context, thread_local, node);
     }
 
     fn text_node_needs_traversal(node: N) -> bool {
@@ -115,8 +112,8 @@ pub trait PostorderNodeMutTraversal<ConcreteThreadSafeLayoutNode: ThreadSafeLayo
 #[inline]
 #[allow(unsafe_code)]
 fn construct_flows_at<'a, N>(context: &LayoutContext<'a>,
-                             _thread_local: &ScopedThreadLocalLayoutContext<N::ConcreteElement>,
-                             root: OpaqueNode, node: N)
+                             _thread_local: &mut ScopedThreadLocalLayoutContext<N::ConcreteElement>,
+                             node: N)
     where N: LayoutNode,
 {
     debug!("construct_flows_at: {:?}", node);
@@ -142,8 +139,6 @@ fn construct_flows_at<'a, N>(context: &LayoutContext<'a>,
     if let Some(el) = node.as_element() {
         el.mutate_data().unwrap().persist();
         unsafe { el.unset_dirty_descendants(); }
-
-        remove_from_bloom_filter(&context.shared.style_context, root, el);
     }
 }
 

components/layout_thread/lib.rs

@@ -520,7 +520,6 @@ impl LayoutThread {
                 viewport_size: self.viewport_size.clone(),
                 screen_size_changed: screen_size_changed,
                 stylist: rw_data.stylist.clone(),
-                generation: self.generation,
                 goal: goal,
                 running_animations: self.running_animations.clone(),
                 expired_animations: self.expired_animations.clone(),
@@ -1152,7 +1151,7 @@ impl LayoutThread {
                                                                          data.reflow_info.goal);
 
         // NB: Type inference falls apart here for some reason, so we need to be very verbose. :-(
-        let traversal = RecalcStyleAndConstructFlows::new(shared_layout_context, element.as_node().opaque());
+        let traversal = RecalcStyleAndConstructFlows::new(shared_layout_context);
         let dom_depth = Some(0); // This is always the root node.
         let token = {
             let stylist = &<RecalcStyleAndConstructFlows as

components/script/test.rs

@@ -19,7 +19,7 @@ pub mod size_of {
     use dom::htmlspanelement::HTMLSpanElement;
     use dom::node::Node;
     use dom::text::Text;
-    use layout_wrapper::ServoThreadSafeLayoutNode;
+    use layout_wrapper::{ServoLayoutElement, ServoLayoutNode, ServoThreadSafeLayoutNode};
     use std::mem::size_of;
 
     pub fn CharacterData() -> usize {
@@ -50,6 +50,14 @@ pub mod size_of {
         size_of::<Node>()
     }
 
+    pub fn SendElement() -> usize {
+        size_of::<::style::dom::SendElement<ServoLayoutElement>>()
+    }
+
+    pub fn SendNode() -> usize {
+        size_of::<::style::dom::SendNode<ServoLayoutNode>>()
+    }
+
     pub fn ServoThreadSafeLayoutNode() -> usize {
         size_of::<ServoThreadSafeLayoutNode>()
     }

components/style/bloom.rs

@@ -5,76 +5,51 @@
 //! The style bloom filter is used as an optimization when matching deep
 //! descendant selectors.
 
-use dom::{TNode, TElement, UnsafeNode};
+use dom::{SendElement, TElement};
 use matching::MatchMethods;
 use selectors::bloom::BloomFilter;
 
-pub struct StyleBloom {
+pub struct StyleBloom<E: TElement> {
     /// The bloom filter per se.
     filter: Box<BloomFilter>,
 
-    /// The stack of elements that this bloom filter contains. These unsafe
-    /// nodes are guaranteed to be elements.
-    ///
-    /// Note that the use we do for them is safe, since the data we access from
-    /// them is completely read-only during restyling.
-    elements: Vec<UnsafeNode>,
-
-    /// A monotonic counter incremented which each reflow in order to invalidate
-    /// the bloom filter if appropriate.
-    generation: u32,
+    /// The stack of elements that this bloom filter contains.
+    elements: Vec<SendElement<E>>,
 }
 
-impl StyleBloom {
-    pub fn new(generation: u32) -> Self {
+impl<E: TElement> StyleBloom<E> {
+    pub fn new() -> Self {
         StyleBloom {
             filter: Box::new(BloomFilter::new()),
             elements: vec![],
-            generation: generation,
         }
     }
 
     pub fn filter(&self) -> &BloomFilter {
         &*self.filter
     }
 
-    pub fn generation(&self) -> u32 {
-        self.generation
-    }
-
-    pub fn maybe_pop<E>(&mut self, element: E)
-        where E: TElement + MatchMethods
-    {
-        if self.elements.last() == Some(&element.as_node().to_unsafe()) {
-            self.pop::<E>().unwrap();
+    pub fn maybe_pop(&mut self, element: E) {
+        if self.elements.last().map(|el| **el) == Some(element) {
+            self.pop().unwrap();
         }
     }
 
     /// Push an element to the bloom filter, knowing that it's a child of the
     /// last element parent.
-    pub fn push<E>(&mut self, element: E)
-        where E: TElement + MatchMethods,
-    {
+    pub fn push(&mut self, element: E) {
         if cfg!(debug_assertions) {
             if self.elements.is_empty() {
                 assert!(element.parent_element().is_none());
             }
         }
         element.insert_into_bloom_filter(&mut *self.filter);
-        self.elements.push(element.as_node().to_unsafe());
+        self.elements.push(unsafe { SendElement::new(element) });
     }
 
     /// Pop the last element in the bloom filter and return it.
-    fn pop<E>(&mut self) -> Option<E>
-        where E: TElement + MatchMethods,
-    {
-        let popped =
-            self.elements.pop().map(|unsafe_node| {
-                let parent = unsafe {
-                    E::ConcreteNode::from_unsafe(&unsafe_node)
-                };
-                parent.as_element().unwrap()
-            });
+    fn pop(&mut self) -> Option<E> {
+        let popped = self.elements.pop().map(|el| *el);
         if let Some(popped) = popped {
             popped.remove_from_bloom_filter(&mut self.filter);
         }
@@ -87,14 +62,12 @@ impl StyleBloom {
         self.elements.clear();
     }
 
-    fn rebuild<E>(&mut self, mut element: E) -> usize
-        where E: TElement + MatchMethods,
-    {
+    fn rebuild(&mut self, mut element: E) -> usize {
         self.clear();
 
         while let Some(parent) = element.parent_element() {
             parent.insert_into_bloom_filter(&mut *self.filter);
-            self.elements.push(parent.as_node().to_unsafe());
+            self.elements.push(unsafe { SendElement::new(parent) });
             element = parent;
         }
 
@@ -105,14 +78,11 @@ impl StyleBloom {
 
     /// In debug builds, asserts that all the parents of `element` are in the
     /// bloom filter.
-    pub fn assert_complete<E>(&self, mut element: E)
-        where E: TElement,
-    {
+    pub fn assert_complete(&self, mut element: E) {
         if cfg!(debug_assertions) {
             let mut checked = 0;
             while let Some(parent) = element.parent_element() {
-                assert_eq!(parent.as_node().to_unsafe(),
-                           self.elements[self.elements.len() - 1 - checked]);
+                assert_eq!(parent, *self.elements[self.elements.len() - 1 - checked]);
                 element = parent;
                 checked += 1;
             }
@@ -127,16 +97,13 @@ impl StyleBloom {
     /// provided always rebuilds the filter from scratch.
     ///
     /// Returns the new bloom filter depth.
-    pub fn insert_parents_recovering<E>(&mut self,
-                                        element: E,
-                                        element_depth: Option<usize>,
-                                        generation: u32)
-                                        -> usize
-        where E: TElement,
+    pub fn insert_parents_recovering(&mut self,
+                                     element: E,
+                                     element_depth: Option<usize>)
+                                     -> usize
     {
         // Easy case, we're in a different restyle, or we're empty.
-        if self.generation != generation || self.elements.is_empty() {
-            self.generation = generation;
+        if self.elements.is_empty() {
             return self.rebuild(element);
         }
 
@@ -149,8 +116,7 @@ impl StyleBloom {
             }
         };
 
-        let unsafe_parent = parent_element.as_node().to_unsafe();
-        if self.elements.last() == Some(&unsafe_parent) {
+        if self.elements.last().map(|el| **el) == Some(parent_element) {
             // Ta da, cache hit, we're all done.
             return self.elements.len();
         }
@@ -182,7 +148,7 @@ impl StyleBloom {
         // If the filter represents an element too deep in the dom, we need to
         // pop ancestors.
         while current_depth > element_depth - 1 {
-            self.pop::<E>().expect("Emilio is bad at math");
+            self.pop().expect("Emilio is bad at math");
             current_depth -= 1;
         }
 
@@ -219,9 +185,9 @@ impl StyleBloom {
         // 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.as_node().to_unsafe() {
+        while **self.elements.last().unwrap() != common_parent {
             parents_to_insert.push(common_parent);
-            self.pop::<E>().unwrap();
+            self.pop().unwrap();
             common_parent = match common_parent.parent_element() {
                 Some(parent) => parent,
                 None => {

components/style/context.rs

@@ -6,6 +6,7 @@
 
 use animation::Animation;
 use app_units::Au;
+use bloom::StyleBloom;
 use dom::{OpaqueNode, TElement};
 use error_reporting::ParseErrorReporter;
 use euclid::Size2D;
@@ -48,10 +49,6 @@ pub struct SharedStyleContext {
     /// The CSS selector stylist.
     pub stylist: Arc<Stylist>,
 
-    /// Starts at zero, and increased by one every time a layout completes.
-    /// This can be used to easily check for invalid stale data.
-    pub generation: u32,
-
     /// Why is this reflow occurring
     pub goal: ReflowGoal,
 
@@ -77,6 +74,7 @@ pub struct SharedStyleContext {
 
 pub struct ThreadLocalStyleContext<E: TElement> {
     pub style_sharing_candidate_cache: StyleSharingCandidateCache<E>,
+    pub bloom_filter: StyleBloom<E>,
     /// A channel on which new animations that have been triggered by style
     /// recalculation can be sent.
     pub new_animations_sender: Sender<Animation>,
@@ -86,6 +84,7 @@ impl<E: TElement> ThreadLocalStyleContext<E> {
     pub fn new(shared: &SharedStyleContext) -> Self {
         ThreadLocalStyleContext {
             style_sharing_candidate_cache: StyleSharingCandidateCache::new(),
+            bloom_filter: StyleBloom::new(),
             new_animations_sender: shared.local_context_creation_data.lock().unwrap().new_animations_sender.clone(),
         }
     }

components/style/dom.rs

@@ -16,6 +16,7 @@ use selector_parser::{ElementExt, PreExistingComputedValues, PseudoElement};
 use sink::Push;
 use std::fmt;
 use std::fmt::Debug;
+use std::ops::Deref;
 use std::sync::Arc;
 use stylist::ApplicableDeclarationBlock;
 
@@ -262,3 +263,38 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre
     /// native anonymous content can opt out of this style fixup.)
     fn skip_root_and_item_based_display_fixup(&self) -> bool;
 }
+
+/// TNode and TElement aren't Send because we want to be careful and explicit
+/// about our parallel traversal. However, there are certain situations
+/// (including but not limited to the traversal) where we need to send DOM
+/// objects to other threads.
+
+#[derive(Clone, Debug, PartialEq)]
+pub struct SendNode<N: TNode>(N);
+unsafe impl<N: TNode> Send for SendNode<N> {}
+impl<N: TNode> SendNode<N> {
+    pub unsafe fn new(node: N) -> Self {
+        SendNode(node)
+    }
+}
+impl<N: TNode> Deref for SendNode<N> {
+    type Target = N;
+    fn deref(&self) -> &N {
+        &self.0
+    }
+}
+
+#[derive(Debug, PartialEq)]
+pub struct SendElement<E: TElement>(E);
+unsafe impl<E: TElement> Send for SendElement<E> {}
+impl<E: TElement> SendElement<E> {
+    pub unsafe fn new(el: E) -> Self {
+        SendElement(el)
+    }
+}
+impl<E: TElement> Deref for SendElement<E> {
+    type Target = E;
+    fn deref(&self) -> &E {
+        &self.0
+    }
+}

components/style/gecko/data.rs

@@ -25,9 +25,6 @@ pub struct PerDocumentStyleDataImpl {
     /// Rule processor.
     pub stylist: Arc<Stylist>,
 
-    /// Last restyle generation.
-    pub last_restyle_generation: u32,
-
     /// List of stylesheets, mirrored from Gecko.
     pub stylesheets: Vec<Arc<Stylesheet>>,
 
@@ -67,7 +64,6 @@ impl PerDocumentStyleData {
 
         PerDocumentStyleData(AtomicRefCell::new(PerDocumentStyleDataImpl {
             stylist: Arc::new(Stylist::new(device)),
-            last_restyle_generation: 0,
             stylesheets: vec![],
             stylesheets_changed: true,
             new_animations_sender: new_anims_sender,
@@ -105,12 +101,6 @@ impl PerDocumentStyleDataImpl {
             self.stylesheets_changed = false;
         }
     }
-
-    pub fn next_generation(&mut self) -> u32 {
-        self.last_restyle_generation =
-            self.last_restyle_generation.wrapping_add(1);
-        self.last_restyle_generation
-    }
 }
 
 unsafe impl HasFFI for PerDocumentStyleData {