Auto merge of #22487 - emilio:single-layout-thread-pool, r=jdm

bors-servo · web-flow · commit b5a10e9c9689 · 2018-12-23T09:39:07.000-05:00
style: Make Servo use a single thread-pool for layout-related tasks per process. Instead of per-document. This also allows to reuse this thread-pool if needed for other stuff, like parallel CSS parsing (#22478), and to share more code with Gecko, which is always nice.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22487)
diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs
@@ -219,7 +219,6 @@ impl Pipeline {
                     window_size: window_size,
                     pipeline_port: pipeline_port,
                     content_process_shutdown_chan: Some(layout_content_process_shutdown_chan),
-                    layout_threads: PREFS.get("layout.threads").as_u64().expect("count") as usize,
                 };
 
                 if let Err(e) =
@@ -556,12 +555,6 @@ impl UnprivilegedPipelineContent {
             Some(self.layout_content_process_shutdown_chan),
             self.webrender_api_sender,
             self.webrender_document,
-            self.prefs
-                .get("layout.threads")
-                .expect("exists")
-                .value()
-                .as_u64()
-                .expect("count") as usize,
             paint_time_metrics,
         );
 
diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
@@ -102,10 +102,11 @@ use std::thread;
 use std::time::Duration;
 use style::animation::Animation;
 use style::context::{QuirksMode, RegisteredSpeculativePainter, RegisteredSpeculativePainters};
-use style::context::{SharedStyleContext, StyleSystemOptions, ThreadLocalStyleContextCreationInfo};
+use style::context::{SharedStyleContext, ThreadLocalStyleContextCreationInfo};
 use style::dom::{ShowSubtree, ShowSubtreeDataAndPrimaryValues, TElement, TNode};
 use style::driver;
 use style::error_reporting::RustLogReporter;
+use style::global_style_data::{GLOBAL_STYLE_DATA, STYLE_THREAD_POOL};
 use style::invalidation::element::restyle_hints::RestyleHint;
 use style::logical_geometry::LogicalPoint;
 use style::media_queries::{Device, MediaList, MediaType};
@@ -178,9 +179,6 @@ pub struct LayoutThread {
     /// Is this the first reflow in this LayoutThread?
     first_reflow: Cell<bool>,
 
-    /// The workers that we use for parallel operation.
-    parallel_traversal: Option<rayon::ThreadPool>,
-
     /// Flag to indicate whether to use parallel operations
     parallel_flag: bool,
 
@@ -238,10 +236,6 @@ pub struct LayoutThread {
     /// only be a test-mode timer during testing for animations.
     timer: Timer,
 
-    // Number of layout threads. This is copied from `servo_config::opts`, but we'd
-    // rather limit the dependency on that module here.
-    layout_threads: usize,
-
     /// Paint time metrics.
     paint_time_metrics: PaintTimeMetrics,
 
@@ -270,7 +264,6 @@ impl LayoutThreadFactory for LayoutThread {
         content_process_shutdown_chan: Option<IpcSender<()>>,
         webrender_api_sender: webrender_api::RenderApiSender,
         webrender_document: webrender_api::DocumentId,
-        layout_threads: usize,
         paint_time_metrics: PaintTimeMetrics,
     ) {
         thread::Builder::new()
@@ -308,7 +301,6 @@ impl LayoutThreadFactory for LayoutThread {
                         mem_profiler_chan.clone(),
                         webrender_api_sender,
                         webrender_document,
-                        layout_threads,
                         paint_time_metrics,
                     );
 
@@ -470,7 +462,6 @@ impl LayoutThread {
         mem_profiler_chan: profile_mem::ProfilerChan,
         webrender_api_sender: webrender_api::RenderApiSender,
         webrender_document: webrender_api::DocumentId,
-        layout_threads: usize,
         paint_time_metrics: PaintTimeMetrics,
     ) -> LayoutThread {
         // The device pixel ratio is incorrect (it does not have the hidpi value),
@@ -481,17 +472,6 @@ impl LayoutThread {
             TypedScale::new(opts::get().device_pixels_per_px.unwrap_or(1.0)),
         );
 
-        let workers = rayon::ThreadPoolBuilder::new()
-            .num_threads(layout_threads)
-            .start_handler(|_| thread_state::initialize_layout_worker_thread())
-            .build();
-        let parallel_traversal = if layout_threads > 1 {
-            Some(workers.expect("ThreadPool creation failed"))
-        } else {
-            None
-        };
-        debug!("Possible layout Threads: {}", layout_threads);
-
         // Create the channel on which new animations can be sent.
         let (new_animations_sender, new_animations_receiver) = unbounded();
 
@@ -521,7 +501,6 @@ impl LayoutThread {
             first_reflow: Cell::new(true),
             font_cache_receiver: font_cache_receiver,
             font_cache_sender: ipc_font_cache_sender,
-            parallel_traversal: parallel_traversal,
             parallel_flag: true,
             generation: Cell::new(0),
             new_animations_sender: new_animations_sender,
@@ -563,7 +542,6 @@ impl LayoutThread {
             } else {
                 Timer::new()
             },
-            layout_threads: layout_threads,
             paint_time_metrics: paint_time_metrics,
             layout_query_waiting_time: Histogram::new(),
         }
@@ -596,8 +574,8 @@ impl LayoutThread {
             id: self.id,
             style_context: SharedStyleContext {
                 stylist: &self.stylist,
-                options: StyleSystemOptions::default(),
-                guards: guards,
+                options: GLOBAL_STYLE_DATA.options.clone(),
+                guards,
                 visited_styles_enabled: false,
                 running_animations: self.running_animations.clone(),
                 expired_animations: self.expired_animations.clone(),
@@ -881,7 +859,6 @@ impl LayoutThread {
             info.content_process_shutdown_chan,
             self.webrender_api.clone_sender(),
             self.webrender_document,
-            info.layout_threads,
             info.paint_time_metrics,
         );
     }
@@ -924,8 +901,6 @@ impl LayoutThread {
         );
 
         self.root_flow.borrow_mut().take();
-        // Drop the rayon threadpool if present.
-        let _ = self.parallel_traversal.take();
         self.background_hang_monitor.unregister();
     }
 
@@ -1166,7 +1141,7 @@ impl LayoutThread {
 
         // Parallelize if there's more than 750 objects based on rzambre's suggestion
         // https://github.com/servo/servo/issues/10110
-        self.parallel_flag = self.layout_threads > 1 && data.dom_count > 750;
+        self.parallel_flag = data.dom_count > 750;
         debug!("layout: received layout request for: {}", self.url);
         debug!("Number of objects in DOM: {}", data.dom_count);
         debug!("layout: parallel? {}", self.parallel_flag);
@@ -1373,10 +1348,13 @@ impl LayoutThread {
         // Create a layout context for use throughout the following passes.
         let mut layout_context = self.build_layout_context(guards.clone(), true, &map);
 
-        let thread_pool = if self.parallel_flag {
-            self.parallel_traversal.as_ref()
+        let (thread_pool, num_threads) = if self.parallel_flag {
+            (
+                STYLE_THREAD_POOL.style_thread_pool.as_ref(),
+                STYLE_THREAD_POOL.num_threads,
+            )
         } else {
-            None
+            (None, 1)
         };
 
         let traversal = RecalcStyleAndConstructFlows::new(layout_context);
@@ -1404,14 +1382,14 @@ impl LayoutThread {
                 },
             );
             // TODO(pcwalton): Measure energy usage of text shaping, perhaps?
-            let text_shaping_time = (font::get_and_reset_text_shaping_performance_counter() as u64) /
-                (self.layout_threads as u64);
+            let text_shaping_time =
+                font::get_and_reset_text_shaping_performance_counter() / num_threads;
             profile_time::send_profile_data(
                 profile_time::ProfilerCategory::LayoutTextShaping,
                 self.profiler_metadata(),
                 &self.time_profiler_chan,
                 0,
-                text_shaping_time,
+                text_shaping_time as u64,
                 0,
                 0,
             );
@@ -1742,12 +1720,16 @@ impl LayoutThread {
                 || {
                     let profiler_metadata = self.profiler_metadata();
 
-                    if let (true, Some(traversal)) =
-                        (self.parallel_flag, self.parallel_traversal.as_ref())
-                    {
+                    let thread_pool = if self.parallel_flag {
+                        STYLE_THREAD_POOL.style_thread_pool.as_ref()
+                    } else {
+                        None
+                    };
+
+                    if let Some(pool) = thread_pool {
                         // Parallel mode.
                         LayoutThread::solve_constraints_parallel(
-                            traversal,
+                            pool,
                             FlowRef::deref_mut(root_flow),
                             profiler_metadata,
                             self.time_profiler_chan.clone(),
@@ -1913,7 +1895,8 @@ fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> {
         ))))
     }
 
-    let shared_lock = SharedRwLock::new();
+    let shared_lock = &GLOBAL_STYLE_DATA.shared_lock;
+
     // FIXME: presentational-hints.css should be at author origin with zero specificity.
     //        (Does it make a difference?)
     let mut user_or_user_agent_stylesheets = vec![
@@ -1958,7 +1941,7 @@ fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> {
     )?;
 
     Ok(UserAgentStylesheets {
-        shared_lock: shared_lock,
+        shared_lock: shared_lock.clone(),
         user_or_user_agent_stylesheets: user_or_user_agent_stylesheets,
         quirks_mode_stylesheet: quirks_mode_stylesheet,
     })
diff --git a/components/layout_traits/lib.rs b/components/layout_traits/lib.rs
@@ -43,7 +43,6 @@ pub trait LayoutThreadFactory {
         content_process_shutdown_chan: Option<IpcSender<()>>,
         webrender_api_sender: webrender_api::RenderApiSender,
         webrender_document: webrender_api::DocumentId,
-        layout_threads: usize,
         paint_time_metrics: PaintTimeMetrics,
     );
 }
diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs
@@ -38,7 +38,6 @@ use script_traits::{
     WindowSizeData,
 };
 use script_traits::{NewLayoutInfo, ScriptMsg};
-use servo_config::prefs::PREFS;
 use servo_url::ServoUrl;
 use std::cell::Cell;
 use style::attr::{AttrValue, LengthOrPercentageOrAuto};
@@ -204,7 +203,6 @@ impl HTMLIFrameElement {
                         },
                         device_pixel_ratio: window.device_pixel_ratio(),
                     },
-                    layout_threads: PREFS.get("layout.threads").as_u64().expect("count") as usize,
                 };
 
                 self.pipeline_id.set(Some(new_pipeline_id));
diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs
@@ -45,7 +45,6 @@ use msg::constellation_msg::BrowsingContextId;
 use msg::constellation_msg::PipelineId;
 use msg::constellation_msg::TopLevelBrowsingContextId;
 use script_traits::{AuxiliaryBrowsingContextLoadInfo, LoadData, NewLayoutInfo, ScriptMsg};
-use servo_config::prefs::PREFS;
 use servo_url::ServoUrl;
 use std::cell::Cell;
 use std::ptr;
@@ -290,7 +289,6 @@ impl WindowProxy {
                 pipeline_port: pipeline_receiver,
                 content_process_shutdown_chan: None,
                 window_size: window.window_size(),
-                layout_threads: PREFS.get("layout.threads").as_u64().expect("count") as usize,
             };
             let constellation_msg = ScriptMsg::ScriptNewAuxiliary(load_info, pipeline_sender);
             window.send_to_constellation(constellation_msg);
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
@@ -1910,7 +1910,6 @@ impl ScriptThread {
             window_size,
             pipeline_port,
             content_process_shutdown_chan,
-            layout_threads,
         } = new_layout_info;
 
         let layout_pair = unbounded();
@@ -1927,7 +1926,6 @@ impl ScriptThread {
             script_chan: self.control_chan.clone(),
             image_cache: self.image_cache.clone(),
             content_process_shutdown_chan: content_process_shutdown_chan,
-            layout_threads: layout_threads,
             paint_time_metrics: PaintTimeMetrics::new(
                 new_pipeline_id,
                 self.time_profiler_chan.clone(),
diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs
@@ -220,6 +220,5 @@ pub struct NewLayoutThreadInfo {
     pub script_chan: IpcSender<ConstellationControlMsg>,
     pub image_cache: Arc<dyn ImageCache>,
     pub content_process_shutdown_chan: Option<IpcSender<()>>,
-    pub layout_threads: usize,
     pub paint_time_metrics: PaintTimeMetrics,
 }
diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs
@@ -195,8 +195,6 @@ pub struct NewLayoutInfo {
     pub pipeline_port: IpcReceiver<LayoutControlMsg>,
     /// A shutdown channel so that layout can tell the content process to shut down when it's done.
     pub content_process_shutdown_chan: Option<IpcSender<()>>,
-    /// Number of threads to use for layout.
-    pub layout_threads: usize,
 }
 
 /// When a pipeline is closed, should its browsing context be discarded too?
diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml
@@ -16,8 +16,7 @@ path = "lib.rs"
 doctest = false
 
 [features]
-gecko = ["num_cpus",
-         "style_traits/gecko", "fallible/known_system_malloc"]
+gecko = ["style_traits/gecko", "fallible/known_system_malloc"]
 use_bindgen = ["bindgen", "regex", "toml"]
 servo = ["serde", "style_traits/servo", "servo_atoms", "servo_config", "html5ever",
          "cssparser/serde", "encoding_rs", "malloc_size_of/servo", "arrayvec/use_union",
@@ -47,7 +46,7 @@ log = "0.4"
 malloc_size_of = { path = "../malloc_size_of" }
 malloc_size_of_derive = { path = "../malloc_size_of_derive" }
 matches = "0.1"
-num_cpus = {version = "1.1.0", optional = true}
+num_cpus = {version = "1.1.0"}
 num-integer = "0.1"
 num-traits = "0.2"
 num-derive = "0.2"
diff --git a/components/style/gecko/mod.rs b/components/style/gecko/mod.rs
@@ -10,7 +10,6 @@ mod non_ts_pseudo_class_list;
 pub mod arc_types;
 pub mod conversions;
 pub mod data;
-pub mod global_style_data;
 pub mod media_features;
 pub mod media_queries;
 pub mod pseudo_element;
diff --git a/components/style/global_style_data.rs b/components/style/global_style_data.rs
diff --git a/components/style/lib.rs b/components/style/lib.rs

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,6 @@ pub trait LayoutThreadFactory {`
`43`	`43`	`content_process_shutdown_chan: Option<IpcSender<()>>,`
`44`	`44`	`webrender_api_sender: webrender_api::RenderApiSender,`
`45`	`45`	`webrender_document: webrender_api::DocumentId,`
`46`		`- layout_threads: usize,`
`47`	`46`	`paint_time_metrics: PaintTimeMetrics,`
`48`	`47`	`);`
`49`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -220,6 +220,5 @@ pub struct NewLayoutThreadInfo {`
`220`	`220`	`pub script_chan: IpcSender<ConstellationControlMsg>,`
`221`	`221`	`pub image_cache: Arc<dyn ImageCache>,`
`222`	`222`	`pub content_process_shutdown_chan: Option<IpcSender<()>>,`
`223`		`- pub layout_threads: usize,`
`224`	`223`	`pub paint_time_metrics: PaintTimeMetrics,`
`225`	`224`	`}`
Original file line number	Diff line number	Diff line change
`@@ -195,8 +195,6 @@ pub struct NewLayoutInfo {`
`195`	`195`	`pub pipeline_port: IpcReceiver<LayoutControlMsg>,`
`196`	`196`	`/// A shutdown channel so that layout can tell the content process to shut down when it's done.`
`197`	`197`	`pub content_process_shutdown_chan: Option<IpcSender<()>>,`
`198`		`- /// Number of threads to use for layout.`
`199`		`- pub layout_threads: usize,`
`200`	`198`	`}`
`201`	`199`
`202`	`200`	`/// When a pipeline is closed, should its browsing context be discarded too?`