servo: Merge #15535 - geckolib: use a global thread pool for styling (from froydnj:global-style-thread-pool); r=bholley
authorNathan Froyd <froydnj@gmail.com>
Thu, 23 Feb 2017 10:15:41 -0800
changeset 373544 16ebf28d9b49647b80d6b561745cba24e97b8b74
parent 373543 502b6c944ccf7f8c1bbb94db817d6c63ee2cacb7
child 373545 e4e02bf4f094e464bd55404ce78e1e0aa5b7b2ab
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
milestone54.0a1
servo: Merge #15535 - geckolib: use a global thread pool for styling (from froydnj:global-style-thread-pool); r=bholley By having a single thread pool, rather than one per document, we use less memory. This addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1324250. This may be obvious to an experienced Rust programmer, but I went with raw pointers because trying to use `Option` global variables resulted in complaints about turning on feature flags in nightly Rust. Since this is for stylo, nightly features are not appropriate here. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 56a99577b31a942e340624f97377980b0e612088
servo/Cargo.lock
servo/components/style/Cargo.toml
servo/components/style/gecko/data.rs
servo/components/style/lib.rs
servo/ports/geckolib/Cargo.toml
servo/ports/geckolib/glue.rs
servo/ports/geckolib/lib.rs
servo/tests/unit/stylo/Cargo.toml
servo/tests/unit/stylo/lib.rs
--- a/servo/Cargo.lock
+++ b/servo/Cargo.lock
@@ -901,16 +901,17 @@ dependencies = [
  "cssparser 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "num_cpus 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.18.0",
  "servo_url 0.0.1",
  "style 0.0.1",
  "style_traits 0.0.1",
  "stylo_tests 0.0.1",
 ]
 
 [[package]]
@@ -2716,17 +2717,16 @@ dependencies = [
  "html5ever-atoms 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "nsstring_vendor 0.1.0",
  "num-integer 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-traits 0.1.36 (registry+https://github.com/rust-lang/crates.io-index)",
- "num_cpus 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "ordered-float 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "owning_ref 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "pdqsort 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "phf 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "phf_codegen 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "regex 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2789,16 +2789,17 @@ dependencies = [
  "env_logger 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "geckoservo 0.0.1",
  "lazy_static 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "num_cpus 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.18.0",
  "servo_url 0.0.1",
  "style 0.0.1",
  "style_traits 0.0.1",
 ]
 
 [[package]]
 name = "syn"
--- a/servo/components/style/Cargo.toml
+++ b/servo/components/style/Cargo.toml
@@ -8,17 +8,17 @@ publish = false
 build = "build.rs"
 
 [lib]
 name = "style"
 path = "lib.rs"
 doctest = false
 
 [features]
-gecko = ["nsstring_vendor", "num_cpus", "rayon/unstable"]
+gecko = ["nsstring_vendor", "rayon/unstable"]
 use_bindgen = ["bindgen", "regex"]
 servo = ["serde/unstable", "serde", "serde_derive", "heapsize_derive",
          "style_traits/servo", "servo_atoms", "html5ever-atoms",
          "cssparser/heapsize", "cssparser/serde",
          "rayon/unstable", "servo_url/servo"]
 testing = []
 
 [dependencies]
@@ -52,20 +52,16 @@ serde_derive = {version = "0.9", optiona
 servo_atoms = {path = "../atoms", optional = true}
 servo_config = {path = "../config"}
 smallvec = "0.1"
 style_traits = {path = "../style_traits"}
 servo_url = {path = "../url"}
 time = "0.1"
 unicode-segmentation = "1.0"
 
-[dependencies.num_cpus]
-optional = true
-version = "1.0"
-
 [target.'cfg(windows)'.dependencies]
 kernel32-sys = "0.2"
 
 [build-dependencies]
 lazy_static = "0.2"
 bindgen = { version = "0.22", optional = true }
 phf_codegen = "0.7.20"
 regex = {version = "0.2", optional = true}
--- a/servo/components/style/gecko/data.rs
+++ b/servo/components/style/gecko/data.rs
@@ -6,23 +6,19 @@
 
 use animation::Animation;
 use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
 use dom::OpaqueNode;
 use gecko_bindings::bindings::RawServoStyleSet;
 use gecko_bindings::structs::RawGeckoPresContextOwned;
 use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI};
 use media_queries::Device;
-use num_cpus;
 use parking_lot::RwLock;
 use properties::ComputedValues;
-use rayon;
-use std::cmp;
 use std::collections::HashMap;
-use std::env;
 use std::sync::Arc;
 use std::sync::mpsc::{Receiver, Sender, channel};
 use stylesheets::Stylesheet;
 use stylist::Stylist;
 
 /// The container for data that a Servo-backed Gecko document needs to style
 /// itself.
 pub struct PerDocumentStyleDataImpl {
@@ -43,62 +39,37 @@ pub struct PerDocumentStyleDataImpl {
     /// animations properly.
     pub new_animations_receiver: Receiver<Animation>,
     /// Unused. Will go away when we actually implement transitions and
     /// animations properly.
     pub running_animations: Arc<RwLock<HashMap<OpaqueNode, Vec<Animation>>>>,
     /// Unused. Will go away when we actually implement transitions and
     /// animations properly.
     pub expired_animations: Arc<RwLock<HashMap<OpaqueNode, Vec<Animation>>>>,
-
-    /// The worker thread pool.
-    /// FIXME(bholley): This shouldn't be per-document.
-    pub work_queue: Option<rayon::ThreadPool>,
-
-    /// The number of threads of the work queue.
-    pub num_threads: usize,
 }
 
 /// The data itself is an `AtomicRefCell`, which guarantees the proper semantics
 /// and unexpected races while trying to mutate it.
 pub struct PerDocumentStyleData(AtomicRefCell<PerDocumentStyleDataImpl>);
 
-lazy_static! {
-    /// The number of layout threads, computed statically.
-    pub static ref NUM_THREADS: usize = {
-        match env::var("STYLO_THREADS").map(|s| s.parse::<usize>().expect("invalid STYLO_THREADS")) {
-            Ok(num) => num,
-            _ => cmp::max(num_cpus::get() * 3 / 4, 1),
-        }
-    };
-}
-
 impl PerDocumentStyleData {
     /// Create a dummy `PerDocumentStyleData`.
     pub fn new(pres_context: RawGeckoPresContextOwned) -> Self {
         let device = Device::new(pres_context);
 
         let (new_anims_sender, new_anims_receiver) = channel();
 
         PerDocumentStyleData(AtomicRefCell::new(PerDocumentStyleDataImpl {
             stylist: Arc::new(Stylist::new(device)),
             stylesheets: vec![],
             stylesheets_changed: true,
             new_animations_sender: new_anims_sender,
             new_animations_receiver: new_anims_receiver,
             running_animations: Arc::new(RwLock::new(HashMap::new())),
             expired_animations: Arc::new(RwLock::new(HashMap::new())),
-            work_queue: if *NUM_THREADS <= 1 {
-                None
-            } else {
-                let configuration =
-                    rayon::Configuration::new().set_num_threads(*NUM_THREADS);
-                rayon::ThreadPool::new(configuration).ok()
-            },
-            num_threads: *NUM_THREADS,
         }))
     }
 
     /// Get an immutable reference to this style data.
     pub fn borrow(&self) -> AtomicRef<PerDocumentStyleDataImpl> {
         self.0.borrow()
     }
 
@@ -136,14 +107,8 @@ impl PerDocumentStyleDataImpl {
     }
 }
 
 unsafe impl HasFFI for PerDocumentStyleData {
     type FFIType = RawServoStyleSet;
 }
 unsafe impl HasSimpleFFI for PerDocumentStyleData {}
 unsafe impl HasBoxFFI for PerDocumentStyleData {}
-
-impl Drop for PerDocumentStyleDataImpl {
-    fn drop(&mut self) {
-        let _ = self.work_queue.take();
-    }
-}
--- a/servo/components/style/lib.rs
+++ b/servo/components/style/lib.rs
@@ -56,17 +56,16 @@ extern crate lazy_static;
 #[macro_use]
 extern crate log;
 #[allow(unused_extern_crates)]
 #[macro_use]
 extern crate matches;
 #[cfg(feature = "gecko")] extern crate nsstring_vendor as nsstring;
 extern crate num_integer;
 extern crate num_traits;
-#[cfg(feature = "gecko")] extern crate num_cpus;
 extern crate ordered_float;
 extern crate owning_ref;
 extern crate parking_lot;
 extern crate pdqsort;
 extern crate phf;
 extern crate rayon;
 extern crate rustc_serialize;
 extern crate selectors;
--- a/servo/ports/geckolib/Cargo.toml
+++ b/servo/ports/geckolib/Cargo.toml
@@ -18,15 +18,16 @@ atomic_refcell = "0.1"
 cssparser = "0.10"
 env_logger = {version = "0.4", default-features = false} # disable `regex` to reduce code size
 euclid = "0.11"
 lazy_static = "0.2"
 libc = "0.2"
 log = {version = "0.3.5", features = ["release_max_level_info"]}
 num_cpus = "1.1.0"
 parking_lot = "0.3"
+rayon = "0.6"
 selectors = {path = "../../components/selectors"}
 servo_url = {path = "../../components/url"}
 style = {path = "../../components/style", features = ["gecko"]}
 style_traits = {path = "../../components/style_traits"}
 
 [dev-dependencies]
 stylo_tests = {path = "../../tests/unit/stylo"}
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -3,32 +3,35 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use app_units::Au;
 use atomic_refcell::AtomicRefMut;
 use cssparser::Parser;
 use cssparser::ToCss as ParserToCss;
 use env_logger::LogBuilder;
 use euclid::Size2D;
+use num_cpus;
 use parking_lot::RwLock;
+use rayon;
 use selectors::Element;
 use servo_url::ServoUrl;
 use std::borrow::Cow;
+use std::cmp;
 use std::env;
 use std::fmt::Write;
 use std::mem;
 use std::ptr;
 use std::sync::{Arc, Mutex};
 use style::arc_ptr_eq;
 use style::context::{QuirksMode, ReflowGoal, SharedStyleContext, StyleContext};
 use style::context::{ThreadLocalStyleContext, ThreadLocalStyleContextCreationInfo};
 use style::data::{ElementData, ElementStyles, RestyleData};
 use style::dom::{ShowSubtreeData, TElement, TNode};
 use style::error_reporting::StdoutErrorReporter;
-use style::gecko::data::{NUM_THREADS, PerDocumentStyleData, PerDocumentStyleDataImpl};
+use style::gecko::data::{PerDocumentStyleData, PerDocumentStyleDataImpl};
 use style::gecko::restyle_damage::GeckoRestyleDamage;
 use style::gecko::selector_parser::{SelectorImpl, PseudoElement};
 use style::gecko::traversal::RecalcStyleOnly;
 use style::gecko::wrapper::DUMMY_BASE_URL;
 use style::gecko::wrapper::GeckoElement;
 use style::gecko_bindings::bindings;
 use style::gecko_bindings::bindings::{RawServoDeclarationBlockBorrowed, RawServoDeclarationBlockStrong};
 use style::gecko_bindings::bindings::{RawServoStyleRuleBorrowed, RawServoStyleRuleStrong};
@@ -85,16 +88,55 @@ 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
  * depend on but good enough for our purposes.
  */
 
+struct GlobalStyleData {
+    // How many threads parallel styling can use.
+    pub num_threads: usize,
+
+    // The parallel styling thread pool.
+    pub style_thread_pool: Option<rayon::ThreadPool>,
+}
+
+impl GlobalStyleData {
+    pub fn new() -> Self {
+        let stylo_threads = env::var("STYLO_THREADS")
+            .map(|s| s.parse::<usize>().expect("invalid STYLO_THREADS value"));
+        let num_threads = match stylo_threads {
+            Ok(num) => num,
+            _ => cmp::max(num_cpus::get() * 3 / 4, 1),
+        };
+
+        let pool = if num_threads <= 1 {
+            None
+        } else {
+            let configuration =
+                rayon::Configuration::new().set_num_threads(num_threads);
+            let pool = rayon::ThreadPool::new(configuration).ok();
+            pool
+        };
+
+        GlobalStyleData {
+            num_threads: num_threads,
+            style_thread_pool: pool,
+        }
+    }
+}
+
+lazy_static! {
+    static ref GLOBAL_STYLE_DATA: GlobalStyleData = {
+        GlobalStyleData::new()
+    };
+}
+
 #[no_mangle]
 pub extern "C" fn Servo_Initialize() -> () {
     // Initialize logging.
     let mut builder = LogBuilder::new();
     let default_level = if cfg!(debug_assertions) { "warn" } else { "error" };
     match env::var("RUST_LOG") {
       Ok(v) => builder.parse(&v).init().unwrap(),
       _ => builder.parse(default_level).init().unwrap(),
@@ -143,38 +185,40 @@ fn traverse_subtree(element: GeckoElemen
     // servo to try to style it. Detect that here and bail out.
     if let Some(parent) = element.parent_element() {
         if parent.borrow_data().map_or(true, |d| d.styles().is_display_none()) {
             debug!("{:?} has unstyled parent - ignoring call to traverse_subtree", parent);
             return;
         }
     }
 
-    let mut per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
+    let per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow();
 
     let token = RecalcStyleOnly::pre_traverse(element, &per_doc_data.stylist, unstyled_children_only);
     if !token.should_traverse() {
         return;
     }
 
     debug!("Traversing subtree:");
     debug!("{:?}", ShowSubtreeData(element.as_node()));
 
     let shared_style_context = create_shared_context(&per_doc_data);
-    let traversal_driver = if per_doc_data.num_threads == 1 || per_doc_data.work_queue.is_none() {
+    let ref global_style_data = *GLOBAL_STYLE_DATA;
+
+    let traversal_driver = if global_style_data.style_thread_pool.is_none() {
         TraversalDriver::Sequential
     } else {
         TraversalDriver::Parallel
     };
 
     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());
+                               global_style_data.style_thread_pool.as_ref().unwrap());
     } else {
         sequential::traverse_dom(&traversal, element, token);
     }
 }
 
 /// Traverses the subtree rooted at `root` for restyling.  Returns whether a
 /// Gecko post-traversal (to perform lazy frame construction, or consume any
 /// RestyleData, or drop any ElementData) is required.
@@ -329,17 +373,17 @@ pub extern "C" fn Servo_AnimationValues_
     if iter.next().is_some() || geckoiter.next().is_some() {
         warn!("stylo: Mismatched sizes of Gecko and Servo \
                array during animation value construction");
     }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleWorkerThreadCount() -> u32 {
-    *NUM_THREADS as u32
+    GLOBAL_STYLE_DATA.num_threads as u32
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_Element_ClearData(element: RawGeckoElementBorrowed) -> () {
     GeckoElement(element).clear_data();
 }
 
 #[no_mangle]
--- a/servo/ports/geckolib/lib.rs
+++ b/servo/ports/geckolib/lib.rs
@@ -4,19 +4,22 @@
 
 #![deny(warnings)]
 
 extern crate app_units;
 extern crate atomic_refcell;
 extern crate cssparser;
 extern crate env_logger;
 extern crate euclid;
+#[macro_use] extern crate lazy_static;
 extern crate libc;
 #[macro_use] extern crate log;
+extern crate num_cpus;
 extern crate parking_lot;
+extern crate rayon;
 extern crate selectors;
 extern crate servo_url;
 extern crate style;
 extern crate style_traits;
 
 #[allow(non_snake_case)]
 pub mod glue;
 mod stylesheet_loader;
--- a/servo/tests/unit/stylo/Cargo.toml
+++ b/servo/tests/unit/stylo/Cargo.toml
@@ -17,13 +17,14 @@ atomic_refcell = "0.1"
 cssparser = "0.10"
 env_logger = "0.4"
 euclid = "0.11"
 lazy_static = "0.2"
 libc = "0.2"
 log = {version = "0.3.5", features = ["release_max_level_info"]}
 num_cpus = "1.1.0"
 parking_lot = "0.3"
+rayon = "0.6"
 selectors = {path = "../../../components/selectors"}
 servo_url = {path = "../../../components/url"}
 style_traits = {path = "../../../components/style_traits"}
 geckoservo = {path = "../../../ports/geckolib"}
 style = {path = "../../../components/style", features = ["gecko"]}
--- a/servo/tests/unit/stylo/lib.rs
+++ b/servo/tests/unit/stylo/lib.rs
@@ -3,18 +3,21 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 extern crate app_units;
 extern crate atomic_refcell;
 extern crate cssparser;
 extern crate env_logger;
 extern crate euclid;
 extern crate geckoservo;
+#[macro_use] extern crate lazy_static;
 #[macro_use] extern crate log;
+extern crate num_cpus;
 extern crate parking_lot;
+extern crate rayon;
 extern crate selectors;
 extern crate servo_url;
 extern crate style;
 extern crate style_traits;
 
 mod sanity_checks;
 
 #[path = "../../../ports/geckolib/stylesheet_loader.rs"]