servo: Merge #15160 - style: Expose the traversal kind to the style system (from emilio:expose-traversal-kind); r=bholley
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 24 Jan 2017 17:02:41 -0800
changeset 340644 f5d13b99a0ab451b7f1d189bc9b2fb55bb229cae
parent 340643 7b58090cb1dfd4e403487022f35a05e245156072
child 340645 0d17e94844cfee715e416aeceb83adc146557472
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1332525
servo: Merge #15160 - style: Expose the traversal kind to the style system (from emilio:expose-traversal-kind); r=bholley This way we'll be able to take different paths for the sequential and parallel traversals in some concrete cases. This is a preliminar patch to fix bug 1332525. r? @bholley Source-Repo: https://github.com/servo/servo Source-Revision: 1934a338757a84a6efddcbd3ecf051cd128a8d18
servo/components/layout/traversal.rs
servo/components/layout_thread/lib.rs
servo/components/style/gecko/traversal.rs
servo/components/style/parallel.rs
servo/components/style/sequential.rs
servo/components/style/traversal.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/layout/traversal.rs
+++ b/servo/components/layout/traversal.rs
@@ -12,36 +12,38 @@ use flow::{self, PreorderFlowTraversal};
 use flow::{CAN_BE_FRAGMENTED, Flow, ImmutableFlowUtils, PostorderFlowTraversal};
 use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutNode};
 use servo_config::opts;
 use style::context::{SharedStyleContext, StyleContext};
 use style::data::ElementData;
 use style::dom::{NodeInfo, 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};
+use style::traversal::{DomTraversal, TraversalDriver, recalc_style_at};
 use style::traversal::PerLevelTraversalData;
 use wrapper::{GetRawData, LayoutNodeHelpers, LayoutNodeLayoutData};
 use wrapper::ThreadSafeLayoutNodeHelpers;
 
 pub struct RecalcStyleAndConstructFlows {
     shared: SharedLayoutContext,
+    driver: TraversalDriver,
 }
 
 impl RecalcStyleAndConstructFlows {
     pub fn shared_layout_context(&self) -> &SharedLayoutContext {
         &self.shared
     }
 }
 
 impl RecalcStyleAndConstructFlows {
     /// Creates a traversal context, taking ownership of the shared layout context.
-    pub fn new(shared: SharedLayoutContext) -> Self {
+    pub fn new(shared: SharedLayoutContext, driver: TraversalDriver) -> Self {
         RecalcStyleAndConstructFlows {
             shared: shared,
+            driver: driver,
         }
     }
 
     /// Consumes this traversal context, returning ownership of the shared layout
     /// context to the caller.
     pub fn destroy(self) -> SharedLayoutContext {
         self.shared
     }
@@ -96,16 +98,20 @@ impl<E> DomTraversal<E> for RecalcStyleA
 
     fn shared_context(&self) -> &SharedStyleContext {
         &self.shared.style_context
     }
 
     fn create_thread_local_context(&self) -> Self::ThreadLocalContext {
         ScopedThreadLocalLayoutContext::new(&self.shared)
     }
+
+    fn is_parallel(&self) -> bool {
+        self.driver.is_parallel()
+    }
 }
 
 /// A bottom-up, parallelizable traversal.
 pub trait PostorderNodeMutTraversal<ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> {
     /// The operation to perform. Return true to continue or false to stop.
     fn process(&mut self, node: &ConcreteThreadSafeLayoutNode);
 }
 
--- a/servo/components/layout_thread/lib.rs
+++ b/servo/components/layout_thread/lib.rs
@@ -116,17 +116,17 @@ use style::logical_geometry::LogicalPoin
 use style::media_queries::{Device, MediaType};
 use style::parser::ParserContextExtraData;
 use style::properties::ComputedValues;
 use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION, STORE_OVERFLOW};
 use style::stylesheets::{Origin, Stylesheet, UserAgentStylesheets};
 use style::stylist::Stylist;
 use style::thread_state;
 use style::timer::Timer;
-use style::traversal::DomTraversal;
+use style::traversal::{DomTraversal, TraversalDriver};
 
 /// Information needed by the layout thread.
 pub struct LayoutThread {
     /// The ID of the pipeline that we belong to.
     id: PipelineId,
 
     /// The URL of the pipeline that we belong to.
     url: ServoUrl,
@@ -1154,33 +1154,40 @@ impl LayoutThread {
         }
 
         // Create a layout context for use throughout the following passes.
         let mut shared_layout_context = self.build_shared_layout_context(&*rw_data,
                                                                          viewport_size_changed,
                                                                          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);
+        let traversal_driver = if self.parallel_flag && self.parallel_traversal.is_some() {
+            TraversalDriver::Parallel
+        } else {
+            TraversalDriver::Sequential
+        };
+
+        let traversal = RecalcStyleAndConstructFlows::new(shared_layout_context, traversal_driver);
         let dom_depth = Some(0); // This is always the root node.
         let token = {
             let stylist = &<RecalcStyleAndConstructFlows as
                             DomTraversal<ServoLayoutElement>>::shared_context(&traversal).stylist;
             <RecalcStyleAndConstructFlows as
              DomTraversal<ServoLayoutElement>>::pre_traverse(element, stylist, /* skip_root = */ false)
         };
 
         if token.should_traverse() {
             // Recalculate CSS styles and rebuild flows and fragments.
             profile(time::ProfilerCategory::LayoutStyleRecalc,
                     self.profiler_metadata(),
                     self.time_profiler_chan.clone(),
                     || {
                 // Perform CSS selector matching and flow construction.
-                if let (true, Some(pool)) = (self.parallel_flag, self.parallel_traversal.as_mut()) {
+                if traversal_driver.is_parallel() {
+                    let pool = self.parallel_traversal.as_mut().unwrap();
                     // Parallel mode
                     parallel::traverse_dom::<ServoLayoutElement, RecalcStyleAndConstructFlows>(
                         &traversal, element, dom_depth, token, pool);
                 } else {
                     // Sequential mode
                     sequential::traverse_dom::<ServoLayoutElement, RecalcStyleAndConstructFlows>(
                         &traversal, element, token);
                 }
--- a/servo/components/style/gecko/traversal.rs
+++ b/servo/components/style/gecko/traversal.rs
@@ -4,29 +4,31 @@
 
 //! Gecko-specific bits for the styling DOM traversal.
 
 use atomic_refcell::AtomicRefCell;
 use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext};
 use data::ElementData;
 use dom::{NodeInfo, TNode};
 use gecko::wrapper::{GeckoElement, GeckoNode};
-use traversal::{DomTraversal, PerLevelTraversalData, recalc_style_at};
+use traversal::{DomTraversal, PerLevelTraversalData, TraversalDriver, recalc_style_at};
 
 /// This is the simple struct that Gecko uses to encapsulate a DOM traversal for
 /// styling.
 pub struct RecalcStyleOnly {
     shared: SharedStyleContext,
+    driver: TraversalDriver,
 }
 
 impl RecalcStyleOnly {
     /// Create a `RecalcStyleOnly` traversal from a `SharedStyleContext`.
-    pub fn new(shared: SharedStyleContext) -> Self {
+    pub fn new(shared: SharedStyleContext, driver: TraversalDriver) -> Self {
         RecalcStyleOnly {
             shared: shared,
+            driver: driver,
         }
     }
 }
 
 impl<'le> DomTraversal<GeckoElement<'le>> for RecalcStyleOnly {
     type ThreadLocalContext = ThreadLocalStyleContext<GeckoElement<'le>>;
 
     fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData,
@@ -61,9 +63,13 @@ impl<'le> DomTraversal<GeckoElement<'le>
 
     fn shared_context(&self) -> &SharedStyleContext {
         &self.shared
     }
 
     fn create_thread_local_context(&self) -> Self::ThreadLocalContext {
         ThreadLocalStyleContext::new(&self.shared)
     }
+
+    fn is_parallel(&self) -> bool {
+        self.driver.is_parallel()
+    }
 }
--- a/servo/components/style/parallel.rs
+++ b/servo/components/style/parallel.rs
@@ -40,16 +40,17 @@ pub const CHUNK_SIZE: usize = 64;
 pub fn traverse_dom<E, D>(traversal: &D,
                           root: E,
                           known_root_dom_depth: Option<usize>,
                           token: PreTraverseToken,
                           queue: &rayon::ThreadPool)
     where E: TElement,
           D: DomTraversal<E>,
 {
+    debug_assert!(traversal.is_parallel());
     // Handle Gecko's eager initial styling. We don't currently support it
     // in conjunction with bottom-up traversal. If we did, we'd need to put
     // it on the context to make it available to the bottom-up phase.
     let (nodes, depth) = if token.traverse_unstyled_children_only() {
         debug_assert!(!D::needs_postorder_traversal());
         let mut children = vec![];
         for kid in root.as_node().children() {
             if kid.as_element().map_or(false, |el| el.get_data().is_none()) {
--- a/servo/components/style/sequential.rs
+++ b/servo/components/style/sequential.rs
@@ -13,16 +13,17 @@ use traversal::{DomTraversal, PerLevelTr
 
 /// Do a sequential DOM traversal for layout or styling, generic over `D`.
 pub fn traverse_dom<E, D>(traversal: &D,
                           root: E,
                           token: PreTraverseToken)
     where E: TElement,
           D: DomTraversal<E>,
 {
+    debug_assert!(!traversal.is_parallel());
     debug_assert!(token.should_traverse());
 
     fn doit<E, D>(traversal: &D, traversal_data: &mut PerLevelTraversalData,
                   thread_local: &mut D::ThreadLocalContext, node: E::ConcreteNode)
         where E: TElement,
               D: DomTraversal<E>
     {
         traversal.process_preorder(traversal_data, thread_local, node);
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -57,16 +57,33 @@ pub enum LogBehavior {
     /// We shouldn't log.
     DontLog,
 }
 use self::LogBehavior::*;
 impl LogBehavior {
     fn allow(&self) -> bool { matches!(*self, MayLog) }
 }
 
+/// The kind of traversals we could perform.
+#[derive(Debug, Copy, Clone)]
+pub enum TraversalDriver {
+    /// A potentially parallel traversal.
+    Parallel,
+    /// A sequential traversal.
+    Sequential,
+}
+
+impl TraversalDriver {
+    /// Returns whether this represents a parallel traversal or not.
+    #[inline]
+    pub fn is_parallel(&self) -> bool {
+        matches!(*self, TraversalDriver::Parallel)
+    }
+}
+
 /// A DOM Traversal trait, that is used to generically implement styling for
 /// Gecko and Servo.
 pub trait DomTraversal<E: TElement> : Sync {
     /// The thread-local context, used to store non-thread-safe stuff that needs
     /// to be used in the traversal, and of which we use one per worker, like
     /// the bloom filter, for example.
     type ThreadLocalContext: Send + BorrowMut<ThreadLocalStyleContext<E>>;
 
@@ -279,16 +296,24 @@ pub trait DomTraversal<E: TElement> : Sy
     /// children of |element|.
     unsafe fn clear_element_data(element: &E);
 
     /// Return the shared style context common to all worker threads.
     fn shared_context(&self) -> &SharedStyleContext;
 
     /// Creates a thread-local context.
     fn create_thread_local_context(&self) -> Self::ThreadLocalContext;
+
+    /// Whether we're performing a parallel traversal.
+    ///
+    /// NB: We do this check on runtime. We could guarantee correctness in this
+    /// regard via the type system via a `TraversalDriver` trait for this trait,
+    /// that could be one of two concrete types. It's not clear whether the
+    /// potential code size impact of that is worth it.
+    fn is_parallel(&self) -> bool;
 }
 
 /// Helper for the function below.
 fn resolve_style_internal<E, F>(context: &StyleContext<E>, element: E, ensure_data: &F)
                                 -> Option<E>
     where E: TElement,
           F: Fn(E),
 {
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -63,17 +63,17 @@ use style::restyle_hints::RestyleHint;
 use style::selector_parser::PseudoElementCascadeType;
 use style::sequential;
 use style::string_cache::Atom;
 use style::stylesheets::{CssRule, CssRules, Origin, Stylesheet, StyleRule, ImportRule};
 use style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
 use style::supports::parse_condition_or_declaration;
 use style::thread_state;
 use style::timer::Timer;
-use style::traversal::{resolve_style, DomTraversal};
+use style::traversal::{resolve_style, DomTraversal, TraversalDriver};
 use style_traits::ToCss;
 use stylesheet_loader::StylesheetLoader;
 
 /*
  * For Gecko->Servo function calls, we need to redeclare the same signature that was declared in
  * the C header in Gecko. In order to catch accidental mismatches, we run rust-bindgen against
  * those signatures as well, giving us a second declaration of all the Servo_* functions in this
  * crate. If there's a mismatch, LLVM will assert and abort, which is a rather awful thing to
@@ -134,24 +134,29 @@ fn traverse_subtree(element: GeckoElemen
         error!("Unnecessary call to traverse_subtree");
         return;
     }
 
     debug!("Traversing subtree:");
     debug!("{:?}", ShowSubtreeData(element.as_node()));
 
     let shared_style_context = create_shared_context(&per_doc_data);
-    let traversal = RecalcStyleOnly::new(shared_style_context);
-    let known_depth = None;
+    let traversal_driver = if per_doc_data.num_threads == 1 || per_doc_data.work_queue.is_none() {
+        TraversalDriver::Sequential
+    } else {
+        TraversalDriver::Parallel
+    };
 
-    if per_doc_data.num_threads == 1 || per_doc_data.work_queue.is_none() {
-        sequential::traverse_dom(&traversal, element, token);
-    } else {
+    let traversal = RecalcStyleOnly::new(shared_style_context, traversal_driver);
+    let known_depth = None;
+    if traversal_driver.is_parallel() {
         parallel::traverse_dom(&traversal, element, known_depth, token,
                                per_doc_data.work_queue.as_mut().unwrap());
+    } else {
+        sequential::traverse_dom(&traversal, element, token);
     }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed,
                                         raw_data: RawServoStyleSetBorrowed,
                                         behavior: structs::TraversalRootBehavior) -> () {
     let element = GeckoElement(root);