Bug 1518045 - Make Servo use a single thread-pool for layout-related tasks per-process. r=jdm
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 17 Dec 2018 23:46:42 +0100
changeset 452669 4cfc198196e989cbda49033f03879d7f1b53fae7
parent 452663 640985215424f83155c5fecdcadbe5f284d6c565
child 452670 c62ba89a005dc61b5bc627c6779dd780609eadab
push id35325
push useropoprus@mozilla.com
push dateMon, 07 Jan 2019 09:30:40 +0000
treeherdermozilla-central@0dcf945b3871 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs1518045, 22478, 22487
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518045 - Make Servo use a single thread-pool for layout-related tasks per-process. r=jdm 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 cherry-picks https://github.com/servo/servo/pull/22487, with a few minor fixes to the build that are landing as part of the sync associated to this bug, and an lsan exception tweak to point to the right module since it's moving.
build/sanitizers/lsan_suppressions.txt
servo/components/style/Cargo.toml
servo/components/style/gecko/global_style_data.rs
servo/components/style/gecko/mod.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/global_style_data.rs
servo/components/style/lib.rs
servo/ports/geckolib/glue.rs
servo/ports/geckolib/stylesheet_loader.rs
--- a/build/sanitizers/lsan_suppressions.txt
+++ b/build/sanitizers/lsan_suppressions.txt
@@ -22,17 +22,17 @@ leak:pixman_implementation_lookup_compos
 leak:libfontconfig.so
 leak:GI___strdup
 # The symbol is really __GI___strdup, but if you have the leading _, it doesn't suppress it.
 
 # Bug 1078015 - If the process terminates during a PR_Sleep, LSAN  detects a leak
 leak:PR_Sleep
 
 # Bug 1363976 - Stylo holds some global data alive forever.
-leak:style::gecko::global_style_data
+leak:style::global_style_data
 
 ###
 ### Many leaks only affect some test suites.  The suite annotations are not checked.
 ###
 
 # Bug 979928 - WebRTC leaks in different mochitest suites.
 leak:NR_reg_init
 # nr_reg_local_init should be redundant with NR_reg_init, but on Aurora
--- a/servo/components/style/Cargo.toml
+++ b/servo/components/style/Cargo.toml
@@ -11,18 +11,17 @@ build = "build.rs"
 links = "for some reason the links key is required to pass data around between build scripts"
 
 [lib]
 name = "style"
 path = "lib.rs"
 doctest = false
 
 [features]
-gecko = ["nsstring", "num_cpus",
-         "style_traits/gecko", "fallible/known_system_malloc"]
+gecko = ["nsstring", "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",
          "servo_url", "string_cache", "crossbeam-channel"]
 gecko_debug = []
 
 [dependencies]
 app_units = "0.7"
@@ -43,17 +42,17 @@ html5ever = {version = "0.22", optional 
 itertools = "0.7.6"
 itoa = "0.4"
 lazy_static = "1"
 log = "0.4"
 malloc_size_of = { path = "../malloc_size_of" }
 malloc_size_of_derive = { path = "../malloc_size_of_derive" }
 matches = "0.1"
 nsstring = {path = "../../../xpcom/rust/nsstring/", optional = true}
-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"
 ordered-float = "1.0"
 owning_ref = "0.3.3"
 parking_lot = "0.6"
 precomputed-hash = "0.1.1"
 rayon = "1"
deleted file mode 100644
--- a/servo/components/style/gecko/global_style_data.rs
+++ /dev/null
@@ -1,117 +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 https://mozilla.org/MPL/2.0/. */
-
-//! Global style data
-
-use crate::context::StyleSystemOptions;
-use crate::gecko_bindings::bindings;
-use crate::parallel::STYLE_THREAD_STACK_SIZE_KB;
-use crate::shared_lock::SharedRwLock;
-use crate::thread_state;
-use num_cpus;
-use rayon;
-use std::cmp;
-use std::env;
-use std::ffi::CString;
-
-/// Global style data
-pub struct GlobalStyleData {
-    /// Shared RWLock for CSSOM objects
-    pub shared_lock: SharedRwLock,
-
-    /// Global style system options determined by env vars.
-    pub options: StyleSystemOptions,
-}
-
-/// Global thread pool
-pub struct StyleThreadPool {
-    /// How many threads parallel styling can use.
-    pub num_threads: usize,
-
-    /// The parallel styling thread pool.
-    pub style_thread_pool: Option<rayon::ThreadPool>,
-}
-
-fn thread_name(index: usize) -> String {
-    format!("StyleThread#{}", index)
-}
-
-fn thread_startup(index: usize) {
-    thread_state::initialize_layout_worker_thread();
-    unsafe {
-        bindings::Gecko_SetJemallocThreadLocalArena(true);
-    }
-    let name = thread_name(index);
-    let name = CString::new(name).unwrap();
-    unsafe {
-        // Gecko_RegisterProfilerThread copies the passed name here.
-        bindings::Gecko_RegisterProfilerThread(name.as_ptr());
-    }
-}
-
-fn thread_shutdown(_: usize) {
-    unsafe {
-        bindings::Gecko_UnregisterProfilerThread();
-        bindings::Gecko_SetJemallocThreadLocalArena(false);
-    }
-}
-
-lazy_static! {
-    /// Global thread pool
-    pub static ref STYLE_THREAD_POOL: StyleThreadPool = {
-        let stylo_threads = env::var("STYLO_THREADS")
-            .map(|s| s.parse::<usize>().expect("invalid STYLO_THREADS value"));
-        let mut num_threads = match stylo_threads {
-            Ok(num) => num,
-            // 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. 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).
-            _ => cmp::min(cmp::max(num_cpus::get() * 3 / 4, 1), 6),
-        };
-
-        // If num_threads is one, there's no point in creating a thread pool, so
-        // force it to zero.
-        //
-        // We allow developers to force a one-thread pool for testing via a
-        // special environmental variable.
-        if num_threads == 1 {
-            let force_pool = env::var("FORCE_STYLO_THREAD_POOL")
-                .ok().map_or(false, |s| s.parse::<usize>().expect("invalid FORCE_STYLO_THREAD_POOL value") == 1);
-            if !force_pool {
-                num_threads = 0;
-            }
-        }
-
-        let pool = if num_threads < 1 {
-            None
-        } else {
-            let workers = rayon::ThreadPoolBuilder::new()
-                .num_threads(num_threads)
-                // 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.
-                .breadth_first()
-                .thread_name(thread_name)
-                .start_handler(thread_startup)
-                .exit_handler(thread_shutdown)
-                .stack_size(STYLE_THREAD_STACK_SIZE_KB * 1024)
-                .build();
-            workers.ok()
-        };
-
-        StyleThreadPool {
-            num_threads: num_threads,
-            style_thread_pool: pool,
-        }
-    };
-    /// Global style data
-    pub static ref GLOBAL_STYLE_DATA: GlobalStyleData = GlobalStyleData {
-        shared_lock: SharedRwLock::new(),
-        options: StyleSystemOptions::default(),
-    };
-}
--- a/servo/components/style/gecko/mod.rs
+++ b/servo/components/style/gecko/mod.rs
@@ -5,17 +5,16 @@
 //! Gecko-specific style-system bits.
 
 #[macro_use]
 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;
 pub mod restyle_damage;
 pub mod rules;
 pub mod selector_parser;
 pub mod snapshot;
 pub mod snapshot_helpers;
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -19,17 +19,16 @@ use atomic_refcell::{AtomicRefCell, Atom
 use crate::applicable_declarations::ApplicableDeclarationBlock;
 use crate::author_styles::AuthorStyles;
 use crate::context::{PostAnimationTasks, QuirksMode, SharedStyleContext, UpdateAnimationsTasks};
 use crate::data::ElementData;
 use crate::dom::{LayoutIterator, NodeInfo, OpaqueNode, TDocument, TElement, TNode, TShadowRoot};
 use crate::element_state::{DocumentState, ElementState};
 use crate::font_metrics::{FontMetrics, FontMetricsProvider, FontMetricsQueryResult};
 use crate::gecko::data::GeckoStyleSheet;
-use crate::gecko::global_style_data::GLOBAL_STYLE_DATA;
 use crate::gecko::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl};
 use crate::gecko::snapshot_helpers;
 use crate::gecko_bindings::bindings;
 use crate::gecko_bindings::bindings::Gecko_ElementHasAnimations;
 use crate::gecko_bindings::bindings::Gecko_ElementHasCSSAnimations;
 use crate::gecko_bindings::bindings::Gecko_ElementHasCSSTransitions;
 use crate::gecko_bindings::bindings::Gecko_GetActiveLinkAttrDeclarationBlock;
 use crate::gecko_bindings::bindings::Gecko_GetAnimationEffectCount;
@@ -54,16 +53,17 @@ use crate::gecko_bindings::structs::ELEM
 use crate::gecko_bindings::structs::ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO;
 use crate::gecko_bindings::structs::ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO;
 use crate::gecko_bindings::structs::ELEMENT_HAS_SNAPSHOT;
 use crate::gecko_bindings::structs::NODE_DESCENDANTS_NEED_FRAMES;
 use crate::gecko_bindings::structs::NODE_NEEDS_FRAME;
 use crate::gecko_bindings::structs::{nsAtom, nsIContent, nsINode_BooleanFlag};
 use crate::gecko_bindings::structs::{RawGeckoElement, RawGeckoNode, RawGeckoXBLBinding};
 use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI};
+use crate::global_style_data::GLOBAL_STYLE_DATA;
 use crate::hash::FxHashMap;
 use crate::logical_geometry::WritingMode;
 use crate::media_queries::Device;
 use crate::properties::animated_properties::{AnimationValue, AnimationValueMap};
 use crate::properties::style_structs::Font;
 use crate::properties::{ComputedValues, LonghandId};
 use crate::properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock};
 use crate::rule_tree::CascadeLevel as ServoCascadeLevel;
new file mode 100644
--- /dev/null
+++ b/servo/components/style/global_style_data.rs
@@ -0,0 +1,131 @@
+/* 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 https://mozilla.org/MPL/2.0/. */
+
+//! Global style data
+
+use crate::context::StyleSystemOptions;
+#[cfg(feature = "gecko")]
+use crate::gecko_bindings::bindings;
+use crate::parallel::STYLE_THREAD_STACK_SIZE_KB;
+use crate::shared_lock::SharedRwLock;
+use crate::thread_state;
+use rayon;
+use std::env;
+
+/// Global style data
+pub struct GlobalStyleData {
+    /// Shared RWLock for CSSOM objects
+    pub shared_lock: SharedRwLock,
+
+    /// Global style system options determined by env vars.
+    pub options: StyleSystemOptions,
+}
+
+/// Global thread pool
+pub struct StyleThreadPool {
+    /// How many threads parallel styling can use.
+    pub num_threads: usize,
+
+    /// The parallel styling thread pool.
+    pub style_thread_pool: Option<rayon::ThreadPool>,
+}
+
+fn thread_name(index: usize) -> String {
+    format!("StyleThread#{}", index)
+}
+
+fn thread_startup(_index: usize) {
+    thread_state::initialize_layout_worker_thread();
+    #[cfg(feature = "gecko")]
+    unsafe {
+        use std::ffi::CString;
+
+        bindings::Gecko_SetJemallocThreadLocalArena(true);
+        let name = thread_name(_index);
+        let name = CString::new(name).unwrap();
+        // Gecko_RegisterProfilerThread copies the passed name here.
+        bindings::Gecko_RegisterProfilerThread(name.as_ptr());
+    }
+}
+
+fn thread_shutdown(_: usize) {
+    #[cfg(feature = "gecko")]
+    unsafe {
+        bindings::Gecko_UnregisterProfilerThread();
+        bindings::Gecko_SetJemallocThreadLocalArena(false);
+    }
+}
+
+lazy_static! {
+    /// Global thread pool
+    pub static ref STYLE_THREAD_POOL: StyleThreadPool = {
+        let stylo_threads = env::var("STYLO_THREADS")
+            .map(|s| s.parse::<usize>().expect("invalid STYLO_THREADS value"));
+        let mut num_threads = match stylo_threads {
+            Ok(num) => num,
+            #[cfg(feature = "servo")]
+            _ => {
+                // We always set this pref on startup, before layout or script
+                // have had a chance of accessing (and thus creating) the
+                // thread-pool.
+                use servo_config::prefs::PREFS;
+                PREFS.get("layout.threads").as_u64().unwrap() as usize
+            }
+            #[cfg(feature = "gecko")]
+            _ => {
+                // 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. 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).
+                use num_cpus;
+                use std::cmp;
+                cmp::min(cmp::max(num_cpus::get() * 3 / 4, 1), 6)
+            }
+        };
+
+        // If num_threads is one, there's no point in creating a thread pool, so
+        // force it to zero.
+        //
+        // We allow developers to force a one-thread pool for testing via a
+        // special environmental variable.
+        if num_threads == 1 {
+            let force_pool = env::var("FORCE_STYLO_THREAD_POOL")
+                .ok().map_or(false, |s| s.parse::<usize>().expect("invalid FORCE_STYLO_THREAD_POOL value") == 1);
+            if !force_pool {
+                num_threads = 0;
+            }
+        }
+
+        let pool = if num_threads < 1 {
+            None
+        } else {
+            let workers = rayon::ThreadPoolBuilder::new()
+                .num_threads(num_threads)
+                // 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.
+                .breadth_first()
+                .thread_name(thread_name)
+                .start_handler(thread_startup)
+                .exit_handler(thread_shutdown)
+                .stack_size(STYLE_THREAD_STACK_SIZE_KB * 1024)
+                .build();
+            workers.ok()
+        };
+
+        StyleThreadPool {
+            num_threads: num_threads,
+            style_thread_pool: pool,
+        }
+    };
+    /// Global style data
+    pub static ref GLOBAL_STYLE_DATA: GlobalStyleData = GlobalStyleData {
+        shared_lock: SharedRwLock::new(),
+        options: StyleSystemOptions::default(),
+    };
+}
--- a/servo/components/style/lib.rs
+++ b/servo/components/style/lib.rs
@@ -128,16 +128,17 @@ pub mod element_state;
 #[cfg(feature = "servo")]
 mod encoding_support;
 pub mod error_reporting;
 pub mod font_face;
 pub mod font_metrics;
 #[cfg(feature = "gecko")]
 #[allow(unsafe_code)]
 pub mod gecko_bindings;
+pub mod global_style_data;
 pub mod hash;
 pub mod invalidation;
 #[allow(missing_docs)] // TODO.
 pub mod logical_geometry;
 pub mod matching;
 #[macro_use]
 pub mod media_queries;
 pub mod parallel;
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -25,17 +25,16 @@ use style::context::{CascadeInputs, Quir
 use style::counter_style;
 use style::data::{self, ElementStyles};
 use style::dom::{ShowSubtreeData, TDocument, TElement, TNode};
 use style::driver;
 use style::element_state::{DocumentState, ElementState};
 use style::error_reporting::{ContextualParseError, ParseErrorReporter};
 use style::font_metrics::{get_metrics_provider_for_product, FontMetricsProvider};
 use style::gecko::data::{GeckoStyleSheet, PerDocumentStyleData, PerDocumentStyleDataImpl};
-use style::gecko::global_style_data::{GlobalStyleData, GLOBAL_STYLE_DATA, STYLE_THREAD_POOL};
 use style::gecko::restyle_damage::GeckoRestyleDamage;
 use style::gecko::selector_parser::{NonTSPseudoClass, PseudoElement};
 use style::gecko::traversal::RecalcStyleOnly;
 use style::gecko::url::CssUrlData;
 use style::gecko::wrapper::{GeckoElement, GeckoNode};
 use style::gecko_bindings::bindings;
 use style::gecko_bindings::bindings::nsACString;
 use style::gecko_bindings::bindings::nsAString;
@@ -163,16 +162,17 @@ use style::gecko_bindings::structs::Shee
 use style::gecko_bindings::structs::StyleContentType;
 use style::gecko_bindings::structs::StyleRuleInclusion;
 use style::gecko_bindings::structs::StyleSheet as DomStyleSheet;
 use style::gecko_bindings::structs::URLExtraData;
 use style::gecko_bindings::sugar::ownership::{FFIArcHelpers, HasArcFFI, HasFFI};
 use style::gecko_bindings::sugar::ownership::{HasSimpleFFI, Strong};
 use style::gecko_bindings::sugar::refptr::RefPtr;
 use style::gecko_properties;
+use style::global_style_data::{GlobalStyleData, GLOBAL_STYLE_DATA, STYLE_THREAD_POOL};
 use style::invalidation::element::restyle_hints;
 use style::media_queries::MediaList;
 use style::parser::{self, Parse, ParserContext};
 use style::properties::animated_properties::AnimationValue;
 use style::properties::{parse_one_declaration_into, parse_style_attribute};
 use style::properties::{ComputedValues, Importance, NonCustomPropertyId};
 use style::properties::{LonghandId, LonghandIdSet, PropertyDeclarationBlock, PropertyId};
 use style::properties::{PropertyDeclarationId, ShorthandId};
--- a/servo/ports/geckolib/stylesheet_loader.rs
+++ b/servo/ports/geckolib/stylesheet_loader.rs
@@ -2,17 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use cssparser::SourceLocation;
 use nsstring::nsCString;
 use servo_arc::Arc;
 use style::context::QuirksMode;
 use style::gecko::data::GeckoStyleSheet;
-use style::gecko::global_style_data::GLOBAL_STYLE_DATA;
+use style::global_style_data::GLOBAL_STYLE_DATA;
 use style::gecko_bindings::bindings;
 use style::gecko_bindings::bindings::Gecko_LoadStyleSheet;
 use style::gecko_bindings::structs::{Loader, LoaderReusableStyleSheets};
 use style::gecko_bindings::structs::{
     SheetLoadData, SheetLoadDataHolder, StyleSheet as DomStyleSheet,
 };
 use style::gecko_bindings::sugar::ownership::{FFIArcHelpers, HasBoxFFI, OwnedOrNull};
 use style::gecko_bindings::sugar::refptr::RefPtr;