servo: Merge #14662 - Hoist bloom filter into scoped TLS, and remove a bunch of complexity and unsafety from the style system (from bholley:bloom_tls); r=emilio

With this PR, the only remaining usage of UnsafeNode is the transition stuff, which is servo-only and probably going to be rewritten over the course of stylo. The parallel traversal is now fully typechecked and safe. \o/

r? @emilio

Source-Repo: https://github.com/servo/servo
Source-Revision: b35ab98132539d4e6f1fcd4b3307eae40e387861

servo/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);
     }
 }
 

servo/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

servo/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>()
     }

servo/components/style/bloom.rs

@@ -5,32 +5,23 @@
 //! 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,
         }
     }
 
@@ -38,43 +29,27 @@ impl StyleBloom {
         &*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 => {

servo/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(),
         }
     }

servo/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
+    }
+}

servo/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 {

servo/components/style/lib.rs

@@ -124,7 +124,6 @@ pub mod sink;
 pub mod str;
 pub mod stylesheets;
 pub mod thread_state;
-mod tid;
 pub mod timer;
 pub mod traversal;
 #[macro_use]

servo/components/style/matching.rs

@@ -13,7 +13,7 @@ use cache::LRUCache;
 use cascade_info::CascadeInfo;
 use context::{SharedStyleContext, StyleContext};
 use data::{ComputedStyle, ElementData, ElementStyles, PseudoStyles};
-use dom::{TElement, TNode};
+use dom::{SendElement, TElement, TNode};
 use properties::{CascadeFlags, ComputedValues, SHAREABLE, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade};
 use properties::longhands::display::computed_value as display;
 use rule_tree::StrongRuleNode;
@@ -65,19 +65,13 @@ impl MatchResults {
     }
 }
 
-// TElement isn't Send because we want to be careful and explicit about our
-// parallel traversal, but we need the candidates to be Send so that we can stick
-// them in ScopedTLS.
-#[derive(Debug, PartialEq)]
-struct SendElement<E: TElement>(pub E);
-unsafe impl<E: TElement> Send for SendElement<E> {}
-
 /// Information regarding a candidate.
 ///
 /// TODO: We can stick a lot more info here.
 #[derive(Debug)]
 struct StyleSharingCandidate<E: TElement> {
-    /// The element.
+    /// The element. We use SendElement here so that the cache may live in
+    /// ScopedTLS.
     element: SendElement<E>,
     /// The cached common style affecting attribute info.
     common_style_affecting_attributes: Option<CommonStyleAffectingAttributes>,
@@ -353,7 +347,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
                element, parent);
 
         self.cache.insert(StyleSharingCandidate {
-            element: SendElement(*element),
+            element: unsafe { SendElement::new(*element) },
             common_style_affecting_attributes: None,
             class_attributes: None,
         }, ());
@@ -501,9 +495,8 @@ trait PrivateMatchMethods: TElement {
                                               shared_context: &SharedStyleContext,
                                               candidate: &mut StyleSharingCandidate<Self>)
                                               -> Result<ComputedStyle, CacheMiss> {
-        let candidate_element = candidate.element.0;
-        element_matches_candidate(self, candidate, &candidate_element,
-                                  shared_context)
+        let candidate_element = *candidate.element;
+        element_matches_candidate(self, candidate, &candidate_element, shared_context)
     }
 }
 

servo/components/style/parallel.rs

@@ -4,9 +4,23 @@
 
 //! Implements parallel traversal over the DOM tree.
 //!
-//! This code is highly unsafe. Keep this file small and easy to audit.
+//! This traversal is based on Rayon, and therefore its safety is largely
+//! verified by the type system.
+//!
+//! The primary trickiness and fine print for the above relates to the
+//! thread safety of the DOM nodes themselves. Accessing a DOM element
+//! concurrently on multiple threads is actually mostly "safe", since all
+//! the mutable state is protected by an AtomicRefCell, and so we'll
+//! generally panic if something goes wrong. Still, we try to to enforce our
+//! thread invariants at compile time whenever possible. As such, TNode and
+//! TElement are not Send, so ordinary style system code cannot accidentally
+//! share them with other threads. In the parallel traversal, we explicitly
+//! invoke |unsafe { SendNode::new(n) }| to put nodes in containers that may
+//! be sent to other threads. This occurs in only a handful of places and is
+//! easy to grep for. At the time of this writing, there is no other unsafe
+//! code in the parallel traversal.
 
-use dom::{OpaqueNode, TElement, TNode, UnsafeNode};
+use dom::{OpaqueNode, SendNode, TElement, TNode};
 use rayon;
 use scoped_tls::ScopedTLS;
 use servo_config::opts;
@@ -16,6 +30,7 @@ use traversal::{STYLE_SHARING_CACHE_HITS, STYLE_SHARING_CACHE_MISSES};
 
 pub const CHUNK_SIZE: usize = 64;
 
+#[allow(unsafe_code)]
 pub fn traverse_dom<N, D>(traversal: &D,
                           root: N::ConcreteElement,
                           known_root_dom_depth: Option<usize>,
@@ -37,12 +52,12 @@ pub fn traverse_dom<N, D>(traversal: &D,
         let mut children = vec![];
         for kid in root.as_node().children() {
             if kid.as_element().map_or(false, |el| el.get_data().is_none()) {
-                children.push(kid.to_unsafe());
+                children.push(unsafe { SendNode::new(kid) });
             }
         }
         (children, known_root_dom_depth.map(|x| x + 1))
     } else {
-        (vec![root.as_node().to_unsafe()], known_root_dom_depth)
+        (vec![unsafe { SendNode::new(root.as_node()) }], known_root_dom_depth)
     };
 
     let traversal_data = PerLevelTraversalData {
@@ -70,13 +85,13 @@ pub fn traverse_dom<N, D>(traversal: &D,
 /// A parallel top-down DOM traversal.
 #[inline(always)]
 #[allow(unsafe_code)]
-fn top_down_dom<'a, 'scope, N, D>(unsafe_nodes: &'a [UnsafeNode],
+fn top_down_dom<'a, 'scope, N, D>(nodes: &'a [SendNode<N>],
                                   root: OpaqueNode,
                                   mut traversal_data: PerLevelTraversalData,
                                   scope: &'a rayon::Scope<'scope>,
                                   traversal: &'scope D,
                                   tls: &'scope ScopedTLS<'scope, D::ThreadLocalContext>)
-    where N: TNode,
+    where N: TNode + 'scope,
           D: DomTraversal<N>,
 {
     let mut discovered_child_nodes = vec![];
@@ -85,17 +100,15 @@ fn top_down_dom<'a, 'scope, N, D>(unsafe_nodes: &'a [UnsafeNode],
         // potentially traversing a child on this thread.
         let mut tlc = tls.ensure(|| traversal.create_thread_local_context());
 
-        for unsafe_node in unsafe_nodes {
-            // Get a real layout node.
-            let node = unsafe { N::from_unsafe(&unsafe_node) };
-
+        for n in nodes {
             // Perform the appropriate traversal.
+            let node = **n;
             let mut children_to_process = 0isize;
             traversal.process_preorder(&mut traversal_data, &mut *tlc, node);
             if let Some(el) = node.as_element() {
                 D::traverse_children(el, |kid| {
                     children_to_process += 1;
-                    discovered_child_nodes.push(kid.to_unsafe())
+                    discovered_child_nodes.push(unsafe { SendNode::new(kid) })
                 });
             }
 
@@ -104,7 +117,7 @@ fn top_down_dom<'a, 'scope, N, D>(unsafe_nodes: &'a [UnsafeNode],
             if D::needs_postorder_traversal() {
                 if children_to_process == 0 {
                     // If there were no more children, start walking back up.
-                    bottom_up_dom(traversal, &mut *tlc, root, *unsafe_node)
+                    bottom_up_dom(traversal, &mut *tlc, root, node)
                 } else {
                     // Otherwise record the number of children to process when the
                     // time comes.
@@ -121,12 +134,12 @@ fn top_down_dom<'a, 'scope, N, D>(unsafe_nodes: &'a [UnsafeNode],
     traverse_nodes(discovered_child_nodes, root, traversal_data, scope, traversal, tls);
 }
 
-fn traverse_nodes<'a, 'scope, N, D>(nodes: Vec<UnsafeNode>, root: OpaqueNode,
+fn traverse_nodes<'a, 'scope, N, D>(nodes: Vec<SendNode<N>>, root: OpaqueNode,
                                     traversal_data: PerLevelTraversalData,
                                     scope: &'a rayon::Scope<'scope>,
                                     traversal: &'scope D,
                                     tls: &'scope ScopedTLS<'scope, D::ThreadLocalContext>)
-    where N: TNode,
+    where N: TNode + 'scope,
           D: DomTraversal<N>,
 {
     if nodes.is_empty() {
@@ -163,16 +176,13 @@ fn traverse_nodes<'a, 'scope, N, D>(nodes: Vec<UnsafeNode>, root: OpaqueNode,
 ///
 /// The only communication between siblings is that they both
 /// fetch-and-subtract the parent's children count.
-#[allow(unsafe_code)]
 fn bottom_up_dom<N, D>(traversal: &D,
                        thread_local: &mut D::ThreadLocalContext,
                        root: OpaqueNode,
-                       unsafe_node: UnsafeNode)
+                       mut node: N)
     where N: TNode,
           D: DomTraversal<N>
 {
-    // Get a real layout node.
-    let mut node = unsafe { N::from_unsafe(&unsafe_node) };
     loop {
         // Perform the appropriate operation.
         traversal.process_postorder(thread_local, node);

servo/components/style/tid.rs

@@ -1,26 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-use std::cell::RefCell;
-use std::rc::Rc;
-use std::sync::atomic::{ATOMIC_USIZE_INIT, AtomicUsize, Ordering};
-
-static NEXT_TID: AtomicUsize = ATOMIC_USIZE_INIT;
-
-thread_local!(static TASK_LOCAL_TID: Rc<RefCell<Option<usize>>> = Rc::new(RefCell::new(None)));
-
-/// Every thread gets one, that's unique.
-pub fn tid() -> usize {
-    TASK_LOCAL_TID.with(|ref k| {
-        let ret =
-            match *k.borrow() {
-                None => NEXT_TID.fetch_add(1, Ordering::SeqCst),
-                Some(x) => x,
-            };
-
-        *k.borrow_mut() = Some(ret);
-
-        ret
-    })
-}

servo/components/style/traversal.rs

@@ -5,93 +5,24 @@
 //! Traversing the DOM tree; the bloom filter.
 
 use atomic_refcell::{AtomicRefCell, AtomicRefMut};
-use bloom::StyleBloom;
 use context::{SharedStyleContext, StyleContext};
 use data::{ElementData, StoredRestyleHint};
-use dom::{OpaqueNode, TElement, TNode};
+use dom::{TElement, TNode};
 use matching::{MatchMethods, StyleSharingResult};
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_SELF};
 use selector_parser::RestyleDamage;
 use selectors::Element;
 use selectors::matching::StyleRelations;
 use servo_config::opts;
-use std::cell::RefCell;
 use std::mem;
 use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering};
 use stylist::Stylist;
 
-/// Every time we do another layout, the old bloom filters are invalid. This is
-/// detected by ticking a generation number every layout.
-pub type Generation = u32;
-
 /// Style sharing candidate cache stats. These are only used when
 /// `-Z style-sharing-stats` is given.
 pub static STYLE_SHARING_CACHE_HITS: AtomicUsize = ATOMIC_USIZE_INIT;
 pub static STYLE_SHARING_CACHE_MISSES: AtomicUsize = ATOMIC_USIZE_INIT;
 
-thread_local!(
-    static STYLE_BLOOM: RefCell<Option<StyleBloom>> = RefCell::new(None));
-
-/// Returns the thread local bloom filter.
-///
-/// If one does not exist, a new one will be made for you. If it is out of date,
-/// it will be cleared and reused.
-pub fn take_thread_local_bloom_filter(context: &SharedStyleContext)
-                                      -> StyleBloom
-{
-    trace!("{} taking bf", ::tid::tid());
-
-    STYLE_BLOOM.with(|style_bloom| {
-        style_bloom.borrow_mut().take()
-            .unwrap_or_else(|| StyleBloom::new(context.generation))
-    })
-}
-
-pub fn put_thread_local_bloom_filter(bf: StyleBloom) {
-    trace!("[{}] putting bloom filter back", ::tid::tid());
-
-    STYLE_BLOOM.with(move |style_bloom| {
-        debug_assert!(style_bloom.borrow().is_none(),
-                     "Putting into a never-taken thread-local bloom filter");
-        *style_bloom.borrow_mut() = Some(bf);
-    })
-}
-
-/// Remove `element` from the bloom filter if it's the last element we inserted.
-///
-/// Restores the bloom filter if this is not the root of the reflow.
-///
-/// This is mostly useful for sequential traversal, where the element will
-/// always be the last one.
-pub fn remove_from_bloom_filter<E: TElement>(context: &SharedStyleContext,
-                                             root: OpaqueNode, element: E)
-{
-    trace!("[{}] remove_from_bloom_filter", ::tid::tid());
-
-    // We may have arrived to `reconstruct_flows` without entering in style
-    // recalc at all due to our optimizations, nor that it's up to date, so we
-    // can't ensure there's a bloom filter at all.
-    let bf = STYLE_BLOOM.with(|style_bloom| {
-        style_bloom.borrow_mut().take()
-    });
-
-    if let Some(mut bf) = bf {
-        if context.generation == bf.generation() {
-            bf.maybe_pop(element);
-
-            // If we're the root of the reflow, just get rid of the bloom
-            // filter.
-            //
-            // FIXME: We might want to just leave it in TLS? You don't do 4k
-            // allocations every day. Also, this just clears one thread's bloom
-            // filter, which is... not great?
-            if element.as_node().opaque() != root {
-                put_thread_local_bloom_filter(bf);
-            }
-        }
-    }
-}
-
 // NB: Keep this as small as possible, please!
 #[derive(Clone, Debug)]
 pub struct PerLevelTraversalData {
@@ -429,11 +360,9 @@ fn compute_style<E, D>(_traversal: &D,
           D: DomTraversal<E::ConcreteNode>,
 {
     let shared_context = context.shared;
-    let mut bf = take_thread_local_bloom_filter(shared_context);
     // Ensure the bloom filter is up to date.
-    let dom_depth = bf.insert_parents_recovering(element,
-                                                 traversal_data.current_dom_depth,
-                                                 shared_context.generation);
+    let dom_depth = context.thread_local.bloom_filter
+                           .insert_parents_recovering(element, traversal_data.current_dom_depth);
 
     // Update the dom depth with the up-to-date dom depth.
     //
@@ -441,7 +370,7 @@ fn compute_style<E, D>(_traversal: &D,
     // change from unknown to known at this step.
     traversal_data.current_dom_depth = Some(dom_depth);
 
-    bf.assert_complete(element);
+    context.thread_local.bloom_filter.assert_complete(element);
 
     // Check to see whether we can share a style with someone.
     let sharing_result = if element.parent_element().is_none() {
@@ -461,7 +390,8 @@ fn compute_style<E, D>(_traversal: &D,
                 }
 
                 // Perform the CSS selector matching.
-                match_results = element.match_element(context, Some(bf.filter()));
+                let filter = context.thread_local.bloom_filter.filter();
+                match_results = element.match_element(context, Some(filter));
                 if match_results.primary_is_shareable() {
                     Some(element)
                 } else {
@@ -503,13 +433,6 @@ fn compute_style<E, D>(_traversal: &D,
         clear_descendant_data(element, &|e| unsafe { D::clear_element_data(&e) });
     }
 
-    // TODO(emilio): It's pointless to insert the element in the parallel
-    // traversal, but it may be worth todo it for sequential restyling. What we
-    // do now is trying to recover it which in that case is really cheap, so
-    // we'd save a few instructions, but probably not worth given the added
-    // complexity.
-    put_thread_local_bloom_filter(bf);
-
     // FIXME(bholley): Compute this accurately from the call to CalcStyleDifference.
     let inherited_styles_changed = true;
 

servo/ports/geckolib/glue.rs

@@ -101,7 +101,6 @@ fn create_shared_context(mut per_doc_data: &mut AtomicRefMut<PerDocumentStyleDat
         // FIXME (bug 1303229): Use the actual viewport size here
         viewport_size: Size2D::new(Au(0), Au(0)),
         screen_size_changed: false,
-        generation: per_doc_data.next_generation(),
         goal: ReflowGoal::ForScriptQuery,
         stylist: per_doc_data.stylist.clone(),
         running_animations: per_doc_data.running_animations.clone(),

servo/tests/unit/script/size_of.rs

@@ -38,3 +38,7 @@ sizeof_checker!(size_span, HTMLSpanElement, 336);
 sizeof_checker!(size_text, Text, 184);
 sizeof_checker!(size_characterdata, CharacterData, 184);
 sizeof_checker!(size_servothreadsafelayoutnode, ServoThreadSafeLayoutNode, 16);
+
+// We use these types in the parallel traversal. They should stay pointer-sized.
+sizeof_checker!(size_sendelement, SendElement, 8);
+sizeof_checker!(size_sendnode, SendNode, 8);