style: Unify parallel and sequential traversal scheduling

Use in_place_scope_fifo to spawn work into the thread pool while doing
work in the main thread.

Differential Revision: https://phabricator.services.mozilla.com/D179492

components/style/driver.rs

@@ -11,7 +11,6 @@ use crate::context::{PerThreadTraversalStatistics, StyleContext};
 use crate::context::{ThreadLocalStyleContext, TraversalStatistics};
 use crate::dom::{SendNode, TElement, TNode};
 use crate::parallel;
-use crate::parallel::{work_unit_max, DispatchMode};
 use crate::scoped_tls::ScopedTLS;
 use crate::traversal::{DomTraversal, PerLevelTraversalData, PreTraverseToken};
 use rayon;
@@ -48,11 +47,23 @@ fn report_statistics(stats: &PerThreadTraversalStatistics) {
     gecko_stats.mStylesReused += stats.styles_reused;
 }
 
-fn parallelism_threshold() -> usize {
+fn with_pool_in_place_scope<'scope, R>(
-    #[cfg(feature = "gecko")]
+    work_unit_max: usize,
-    return static_prefs::pref!("layout.css.stylo-parallelism-threshold") as usize;
+    pool: Option<&rayon::ThreadPool>,
-    #[cfg(feature = "servo")]
+    closure: impl FnOnce(Option<&rayon::ScopeFifo<'scope>>) -> R,
-    return 16;
+) -> R {
+    if work_unit_max == 0 || pool.is_none() {
+        closure(None)
+    } else {
+        pool.unwrap().in_place_scope_fifo(|scope| {
+            closure(Some(scope))
+        })
+    }
+}
+
+/// See documentation of the pref for performance characteristics.
+fn work_unit_max() -> usize {
+    static_prefs::pref!("layout.css.stylo-work-unit-size") as usize
 }
 
 /// Do a DOM traversal for top-down and (optionally) bottom-up processing, generic over `D`.
@@ -95,87 +106,38 @@ where
     //     ThreadLocalStyleContext on the main thread. If the main thread
     //     ThreadLocalStyleContext has not released its TLS borrow by that point,
     //     we'll panic on double-borrow.
-    let mut tls_slots = None;
+    let mut scoped_tls = pool.map(ScopedTLS::<ThreadLocalStyleContext<E>>::new);
     let mut tlc = ThreadLocalStyleContext::new();
     let mut context = StyleContext {
         shared: traversal.shared_context(),
         thread_local: &mut tlc,
     };
 
-    // Process the nodes breadth-first, just like the parallel traversal does.
+    // Process the nodes breadth-first. This helps keep similar traversal characteristics for the
-    // This helps keep similar traversal characteristics for the style sharing
+    // style sharing cache.
-    // cache.
     let work_unit_max = work_unit_max();
-    let parallelism_threshold = parallelism_threshold();
+    with_pool_in_place_scope(work_unit_max, pool, |maybe_scope| {
-    let mut discovered = VecDeque::<SendNode<E::ConcreteNode>>::with_capacity(work_unit_max * 2);
+        let mut discovered = VecDeque::with_capacity(work_unit_max * 2);
-    let mut depth = root.depth();
+        discovered.push_back(unsafe { SendNode::new(root.as_node()) });
-    let mut nodes_remaining_at_current_depth = 1;
+        parallel::style_trees(
-    discovered.push_back(unsafe { SendNode::new(root.as_node()) });
-    while let Some(node) = discovered.pop_front() {
-        let mut children_to_process = 0isize;
-        let traversal_data = PerLevelTraversalData {
-            current_dom_depth: depth,
-        };
-        traversal.process_preorder(&traversal_data, &mut context, *node, |n| {
-            children_to_process += 1;
-            discovered.push_back(unsafe { SendNode::new(n) });
-        });
-
-        traversal.handle_postorder_traversal(
             &mut context,
+            discovered,
             root.as_node().opaque(),
-            *node,
+            work_unit_max,
-            children_to_process,
+            static_prefs::pref!("layout.css.stylo-local-work-queue.in-main-thread") as usize,
+            PerLevelTraversalData { current_dom_depth: root.depth() },
+            maybe_scope,
+            traversal,
+            scoped_tls.as_ref(),
         );
-
+    });
-        nodes_remaining_at_current_depth -= 1;
-
-        // If there is enough work to parallelize over, and the caller allows parallelism, switch
-        // to the parallel driver. We do this only when moving to the next level in the dom so that
-        // we can pass the same depth for all the children.
-        if nodes_remaining_at_current_depth != 0 {
-            continue;
-        }
-        depth += 1;
-        if pool.is_some() && discovered.len() > parallelism_threshold && parallelism_threshold > 0 {
-            let pool = pool.unwrap();
-            let tls = ScopedTLS::<ThreadLocalStyleContext<E>>::new(pool);
-            let root_opaque = root.as_node().opaque();
-            pool.scope_fifo(|scope| {
-                // Enable a breadth-first rayon traversal. This causes the work
-                // queue to be always FIFO, rather than FIFO for stealers and
-                // FILO for the owner (which is what rayon does by default). This
-                // ensures that we process all the elements at a given depth before
-                // proceeding to the next depth, which is important for style sharing.
-                #[cfg(feature = "gecko")]
-                gecko_profiler_label!(Layout, StyleComputation);
-                parallel::traverse_nodes(
-                    discovered.make_contiguous(),
-                    DispatchMode::TailCall,
-                    /* recursion_ok = */ true,
-                    root_opaque,
-                    PerLevelTraversalData {
-                        current_dom_depth: depth,
-                    },
-                    scope,
-                    pool,
-                    traversal,
-                    &tls,
-                );
-            });
-
-            tls_slots = Some(tls.into_slots());
-            break;
-        }
-        nodes_remaining_at_current_depth = discovered.len();
-    }
 
     // Collect statistics from thread-locals if requested.
     if dump_stats || report_stats {
         let mut aggregate = mem::replace(&mut context.thread_local.statistics, Default::default());
-        let parallel = tls_slots.is_some();
+        let parallel = pool.is_some();
-        if let Some(ref mut tls) = tls_slots {
+        if let Some(ref mut tls) = scoped_tls {
-            for slot in tls.iter_mut() {
+            for slot in tls.slots() {
                 if let Some(cx) = slot.get_mut() {
                     aggregate += cx.statistics.clone();
                 }

components/style/global_style_data.rs

@@ -123,28 +123,29 @@ fn stylo_threads_pref() -> i32 {
     static_prefs::pref!("layout.css.stylo-threads")
 }
 
+/// The performance benefit of additional threads seems to level off at around six, so we cap it
+/// there on many-core machines (see bug 1431285 comment 14).
+pub(crate) const STYLO_MAX_THREADS: usize = 6;
+
 lazy_static! {
     /// Global thread pool
     pub static ref STYLE_THREAD_POOL: std::sync::Mutex<StyleThreadPool> = {
+        use std::cmp;
         // We always set this pref on startup, before layout or script have had a chance of
         // accessing (and thus creating) the thread-pool.
         let threads_pref: i32 = stylo_threads_pref();
-
         let num_threads = if threads_pref >= 0 {
             threads_pref as usize
         } else {
             use num_cpus;
-            use std::cmp;
             // The default heuristic is num_virtual_cores * .75. This gives us three threads on a
             // hyper-threaded dual core, and six threads on a hyper-threaded quad core.
-            //
+            let threads = cmp::max(num_cpus::get() * 3 / 4, 1);
-            // The performance benefit of additional threads seems to level off at around six, so
-            // we cap it there on many-core machines (see bug 1431285 comment 14).
-            let threads = cmp::min(cmp::max(num_cpus::get() * 3 / 4, 1), 6);
             // There's no point in creating a thread pool if there's one thread.
             if threads == 1 { 0 } else { threads }
         };
 
+        let num_threads = cmp::min(num_threads, STYLO_MAX_THREADS);
         let (pool, num_threads) = if num_threads < 1 {
             (None, None)
         } else {

components/style/parallel.rs

@@ -27,7 +27,7 @@ use crate::dom::{OpaqueNode, SendNode, TElement};
 use crate::scoped_tls::ScopedTLS;
 use crate::traversal::{DomTraversal, PerLevelTraversalData};
 use rayon;
-use smallvec::SmallVec;
+use std::collections::VecDeque;
 
 /// The minimum stack size for a thread in the styling pool, in kilobytes.
 #[cfg(feature = "gecko")]
@@ -54,17 +54,8 @@ pub const STYLE_THREAD_STACK_SIZE_KB: usize = 512;
 ///
 /// [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1395708#c15
 /// [2] See Gecko bug 1376883 for more discussion on the measurements.
-///
 pub const STACK_SAFETY_MARGIN_KB: usize = 168;
 
-/// See documentation of the pref for performance characteristics.
-pub fn work_unit_max() -> usize {
-    #[cfg(feature = "gecko")]
-    return static_prefs::pref!("layout.css.stylo-work-unit-size") as usize;
-    #[cfg(feature = "servo")]
-    return 16;
-}
-
 /// A callback to create our thread local context.  This needs to be
 /// out of line so we don't allocate stack space for the entire struct
 /// in the caller.
@@ -76,223 +67,130 @@ where
     *slot = Some(ThreadLocalStyleContext::new());
 }
 
-/// A parallel top-down DOM traversal.
+// Sends one chunk of work to the thread-pool.
-///
+fn distribute_one_chunk<'a, 'scope, E, D>(
-/// This algorithm traverses the DOM in a breadth-first, top-down manner. The
+    items: VecDeque<SendNode<E::ConcreteNode>>,
-/// goals are:
+    traversal_root: OpaqueNode,
-/// * Never process a child before its parent (since child style depends on
+    work_unit_max: usize,
-///   parent style). If this were to happen, the styling algorithm would panic.
+    traversal_data: PerLevelTraversalData,
-/// * Prioritize discovering nodes as quickly as possible to maximize
-///   opportunities for parallelism.  But this needs to be weighed against
-///   styling cousins on a single thread to improve sharing.
-/// * Style all the children of a given node (i.e. all sibling nodes) on
-///   a single thread (with an upper bound to handle nodes with an
-///   abnormally large number of children). This is important because we use
-///   a thread-local cache to share styles between siblings.
-#[inline(always)]
-#[allow(unsafe_code)]
-fn top_down_dom<'a, 'scope, E, D>(
-    nodes: &'a [SendNode<E::ConcreteNode>],
-    root: OpaqueNode,
-    mut traversal_data: PerLevelTraversalData,
     scope: &'a rayon::ScopeFifo<'scope>,
-    pool: &'scope rayon::ThreadPool,
     traversal: &'scope D,
     tls: &'scope ScopedTLS<'scope, ThreadLocalStyleContext<E>>,
 ) where
     E: TElement + 'scope,
     D: DomTraversal<E>,
 {
-    let work_unit_max = work_unit_max();
+    scope.spawn_fifo(move |scope| {
-    debug_assert!(nodes.len() <= work_unit_max);
+        gecko_profiler_label!(Layout, StyleComputation);
-
+        let mut tlc = tls.ensure(create_thread_local_context);
-    // We set this below, when we have a borrow of the thread-local-context
-    // available.
-    let recursion_ok;
-
-    // Collect all the children of the elements in our work unit. This will
-    // contain the combined children of up to work_unit_max nodes, which may
-    // be numerous. As such, we store it in a large SmallVec to minimize heap-
-    // spilling, and never move it.
-    let mut discovered_child_nodes = SmallVec::<[SendNode<E::ConcreteNode>; 128]>::new();
-    {
-        // Scope the borrow of the TLS so that the borrow is dropped before
-        // a potential recursive call when we pass TailCall.
-        let mut tlc = tls.ensure(|slot: &mut Option<ThreadLocalStyleContext<E>>| {
-            create_thread_local_context(slot)
-        });
-
-        // Check that we're not in danger of running out of stack.
-        recursion_ok = !tlc.stack_limit_checker.limit_exceeded();
-
         let mut context = StyleContext {
             shared: traversal.shared_context(),
             thread_local: &mut *tlc,
         };
-
+        style_trees(
-        for n in nodes {
+            &mut context,
-            // If the last node we processed produced children, we may want to
+            items,
-            // spawn them off into a work item. We do this at the beginning of
+            traversal_root,
-            // the loop (rather than at the end) so that we can traverse our
+            work_unit_max,
-            // last bits of work directly on this thread without a spawn call.
+            static_prefs::pref!("layout.css.stylo-local-work-queue.in-worker") as usize,
-            //
-            // This has the important effect of removing the allocation and
-            // context-switching overhead of the parallel traversal for perfectly
-            // linear regions of the DOM, i.e.:
-            //
-            // <russian><doll><tag><nesting></nesting></tag></doll></russian>
-            //
-            // which are not at all uncommon.
-            //
-            // There's a tension here between spawning off a work item as soon
-            // as discovered_child_nodes is nonempty and waiting until we have a
-            // full work item to do so.  The former optimizes for speed of
-            // discovery (we'll start discovering the kids of the things in
-            // "nodes" ASAP).  The latter gives us better sharing (e.g. we can
-            // share between cousins much better, because we don't hand them off
-            // as separate work items, which are likely to end up on separate
-            // threads) and gives us a chance to just handle everything on this
-            // thread for small DOM subtrees, as in the linear example above.
-            //
-            // There are performance and "number of ComputedValues"
-            // measurements for various testcases in
-            // https://bugzilla.mozilla.org/show_bug.cgi?id=1385982#c10 and
-            // following.
-            //
-            // The worst case behavior for waiting until we have a full work
-            // item is a deep tree which has work_unit_max "linear" branches,
-            // hence work_unit_max elements at each level.  Such a tree would
-            // end up getting processed entirely sequentially, because we would
-            // process each level one at a time as a single work unit, whether
-            // via our end-of-loop tail call or not.  If we kicked off a
-            // traversal as soon as we discovered kids, we would instead
-            // process such a tree more or less with a thread-per-branch,
-            // multiplexed across our actual threadpool.
-            if discovered_child_nodes.len() >= work_unit_max {
-                let mut traversal_data_copy = traversal_data.clone();
-                traversal_data_copy.current_dom_depth += 1;
-                traverse_nodes(
-                    &discovered_child_nodes,
-                    DispatchMode::NotTailCall,
-                    recursion_ok,
-                    root,
-                    traversal_data_copy,
-                    scope,
-                    pool,
-                    traversal,
-                    tls,
-                );
-                discovered_child_nodes.clear();
-            }
-
-            let node = **n;
-            let mut children_to_process = 0isize;
-            traversal.process_preorder(&traversal_data, &mut context, node, |n| {
-                children_to_process += 1;
-                let send_n = unsafe { SendNode::new(n) };
-                discovered_child_nodes.push(send_n);
-            });
-
-            traversal.handle_postorder_traversal(&mut context, root, node, children_to_process);
-        }
-    }
-
-    // Handle whatever elements we have queued up but not kicked off traversals
-    // for yet.  If any exist, we can process them (or at least one work unit's
-    // worth of them) directly on this thread by passing TailCall.
-    if !discovered_child_nodes.is_empty() {
-        traversal_data.current_dom_depth += 1;
-        traverse_nodes(
-            &discovered_child_nodes,
-            DispatchMode::TailCall,
-            recursion_ok,
-            root,
             traversal_data,
-            scope,
+            Some(scope),
-            pool,
             traversal,
-            tls,
+            Some(tls),
         );
-    }
+    })
 }
 
-/// Controls whether traverse_nodes may make a recursive call to continue
+/// Distributes all items into the thread pool, in `work_unit_max` chunks.
-/// doing work, or whether it should always dispatch work asynchronously.
+fn distribute_work<'a, 'scope, E, D>(
-#[derive(Clone, Copy, PartialEq)]
+    mut items: VecDeque<SendNode<E::ConcreteNode>>,
-pub enum DispatchMode {
+    traversal_root: OpaqueNode,
-    /// This is the last operation by the caller.
+    work_unit_max: usize,
-    TailCall,
-    /// This is not the last operation by the caller.
-    NotTailCall,
-}
-
-impl DispatchMode {
-    fn is_tail_call(&self) -> bool {
-        matches!(*self, DispatchMode::TailCall)
-    }
-}
-
-/// Enqueues |nodes| for processing, possibly on this thread if the tail call
-/// conditions are met.
-#[inline]
-pub fn traverse_nodes<'a, 'scope, E, D>(
-    nodes: &[SendNode<E::ConcreteNode>],
-    mode: DispatchMode,
-    recursion_ok: bool,
-    root: OpaqueNode,
     traversal_data: PerLevelTraversalData,
     scope: &'a rayon::ScopeFifo<'scope>,
-    pool: &'scope rayon::ThreadPool,
     traversal: &'scope D,
     tls: &'scope ScopedTLS<'scope, ThreadLocalStyleContext<E>>,
 ) where
     E: TElement + 'scope,
     D: DomTraversal<E>,
 {
-    debug_assert_ne!(nodes.len(), 0);
+    while items.len() > work_unit_max {
+        let rest = items.split_off(work_unit_max);
+        distribute_one_chunk(
+            items,
+            traversal_root,
+            work_unit_max,
+            traversal_data,
+            scope,
+            traversal,
+            tls,
+        );
+        items = rest;
+    }
+    distribute_one_chunk(
+        items,
+        traversal_root,
+        work_unit_max,
+        traversal_data,
+        scope,
+        traversal,
+        tls,
+    );
+}
 
-    // This is a tail call from the perspective of the caller. However, we only
+/// Processes `discovered` items, possibly spawning work in other threads as needed.
-    // want to actually dispatch the job as a tail call if there's nothing left
+#[inline]
-    // in our local queue. Otherwise we need to return to it to maintain proper
+pub fn style_trees<'a, 'scope, E, D>(
-    // breadth-first ordering. We also need to take care to avoid stack
+    context: &mut StyleContext<E>,
-    // overflow due to excessive tail recursion. The stack overflow avoidance
+    mut discovered: VecDeque<SendNode<E::ConcreteNode>>,
-    // isn't observable to content -- we're still completely correct, just not
+    traversal_root: OpaqueNode,
-    // using tail recursion any more. See Gecko bugs 1368302 and 1376883.
+    work_unit_max: usize,
-    let may_dispatch_tail =
+    local_queue_size: usize,
-        mode.is_tail_call() && recursion_ok && !pool.current_thread_has_pending_tasks().unwrap();
+    mut traversal_data: PerLevelTraversalData,
+    scope: Option<&'a rayon::ScopeFifo<'scope>>,
+    traversal: &'scope D,
+    tls: Option<&'scope ScopedTLS<'scope, ThreadLocalStyleContext<E>>>,
+) where
+    E: TElement + 'scope,
+    D: DomTraversal<E>,
+{
+    let mut nodes_remaining_at_current_depth = discovered.len();
+    while let Some(node) = discovered.pop_front() {
+        let mut children_to_process = 0isize;
+        traversal.process_preorder(&traversal_data, context, *node, |n| {
+            children_to_process += 1;
+            discovered.push_back(unsafe { SendNode::new(n) });
+        });
 
-    let work_unit_max = work_unit_max();
+        traversal.handle_postorder_traversal(context, traversal_root, *node, children_to_process);
-    // In the common case, our children fit within a single work unit, in which case we can pass
+
-    // the nodes directly and avoid extra allocation.
+        nodes_remaining_at_current_depth -= 1;
-    if nodes.len() <= work_unit_max {
+
-        if may_dispatch_tail {
+        // If we have enough children at the next depth in the DOM, spawn them to a different job
-            top_down_dom(&nodes, root, traversal_data, scope, pool, traversal, tls);
+        // relatively soon, while keeping always at least `local_queue_size` worth of work for
-        } else {
+        // ourselves.
-            let work = nodes.to_vec();
+        let discovered_children = discovered.len() - nodes_remaining_at_current_depth;
-            scope.spawn_fifo(move |scope| {
+        if discovered_children >= work_unit_max &&
-                #[cfg(feature = "gecko")]
+            discovered.len() >= local_queue_size + work_unit_max &&
-                gecko_profiler_label!(Layout, StyleComputation);
+            scope.is_some()
-                top_down_dom(&work, root, traversal_data, scope, pool, traversal, tls);
+        {
-            });
+            let kept_work = std::cmp::max(nodes_remaining_at_current_depth, local_queue_size);
+            let mut traversal_data_copy = traversal_data.clone();
+            traversal_data_copy.current_dom_depth += 1;
+            distribute_work(
+                discovered.split_off(kept_work),
+                traversal_root,
+                work_unit_max,
+                traversal_data_copy,
+                scope.unwrap(),
+                traversal,
+                tls.unwrap(),
+            );
         }
-    } else {
+
-        for chunk in nodes.chunks(work_unit_max) {
+        if nodes_remaining_at_current_depth == 0 {
-            let work = chunk.to_vec();
+            traversal_data.current_dom_depth += 1;
-            let traversal_data_copy = traversal_data.clone();
+            nodes_remaining_at_current_depth = discovered.len();
-            scope.spawn_fifo(move |scope| {
-                #[cfg(feature = "gecko")]
-                gecko_profiler_label!(Layout, StyleComputation);
-                let work = work;
-                top_down_dom(
-                    &work,
-                    root,
-                    traversal_data_copy,
-                    scope,
-                    pool,
-                    traversal,
-                    tls,
-                )
-            });
         }
     }
 }

components/style/scoped_tls.rs

@@ -7,6 +7,7 @@
 #![allow(unsafe_code)]
 #![deny(missing_docs)]
 
+use crate::global_style_data::STYLO_MAX_THREADS;
 use rayon;
 use std::cell::{Ref, RefCell, RefMut};
 use std::ops::DerefMut;
@@ -20,7 +21,7 @@ use std::ops::DerefMut;
 /// the Send bound.
 pub struct ScopedTLS<'scope, T: Send> {
     pool: &'scope rayon::ThreadPool,
-    slots: Box<[RefCell<Option<T>>]>,
+    slots: [RefCell<Option<T>>; STYLO_MAX_THREADS],
 }
 
 /// The scoped TLS is `Sync` because no more than one worker thread can access a
@@ -30,16 +31,11 @@ unsafe impl<'scope, T: Send> Sync for ScopedTLS<'scope, T> {}
 impl<'scope, T: Send> ScopedTLS<'scope, T> {
     /// Create a new scoped TLS that will last as long as this rayon threadpool
     /// reference.
-    pub fn new(p: &'scope rayon::ThreadPool) -> Self {
+    pub fn new(pool: &'scope rayon::ThreadPool) -> Self {
-        let count = p.current_num_threads();
+        debug_assert!(pool.current_num_threads() <= STYLO_MAX_THREADS);
-        let mut v = Vec::with_capacity(count);
-        for _ in 0..count {
-            v.push(RefCell::new(None));
-        }
-
         ScopedTLS {
-            pool: p,
+            pool,
-            slots: v.into_boxed_slice(),
+            slots: Default::default(),
         }
     }
 
@@ -71,8 +67,9 @@ impl<'scope, T: Send> ScopedTLS<'scope, T> {
         RefMut::map(opt, |x| x.as_mut().unwrap())
     }
 
-    /// Returns the slots, consuming the scope.
+    /// Returns the slots. Safe because if we have a mut reference the tls can't be referenced by
-    pub fn into_slots(self) -> Box<[RefCell<Option<T>>]> {
+    /// any other thread.
-        self.slots
+    pub fn slots(&mut self) -> &mut [RefCell<Option<T>>] {
+        &mut self.slots
     }
 }

components/style/traversal.rs

@@ -26,7 +26,7 @@ pub type UndisplayedStyleCache =
 /// currently only holds the dom depth for the bloom filter.
 ///
 /// NB: Keep this as small as possible, please!
-#[derive(Clone, Debug)]
+#[derive(Clone, Copy, Debug)]
 pub struct PerLevelTraversalData {
     /// The current dom depth.
     ///