Backed out changeset 42ebd8a50978 (bug 1577439) for causing perma leakcheck | tab process: negative leaks caught.
authorRazvan Maries <rmaries@mozilla.com>
Sun, 01 Sep 2019 03:41:33 +0300
changeset 491036 25ee7237b69916f8a42385887c3ccfb1723b68ab
parent 491035 9781c938afa60cd1187eae6ade5b9588a33f551e
child 491037 8867e44d49793d8af6b514089cf4b5ebea446985
push id114012
push usercbrindusan@mozilla.com
push dateSun, 01 Sep 2019 09:54:40 +0000
treeherdermozilla-inbound@8867e44d4979 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1577439
milestone70.0a1
backs out42ebd8a50978189a7247dc029de7d66d97e7bec9
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
Backed out changeset 42ebd8a50978 (bug 1577439) for causing perma leakcheck | tab process: negative leaks caught.
servo/components/style/global_style_data.rs
servo/ports/geckolib/glue.rs
xpcom/build/XPCOMInit.cpp
--- a/servo/components/style/global_style_data.rs
+++ b/servo/components/style/global_style_data.rs
@@ -7,37 +7,33 @@
 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;
-use parking_lot::{RwLock, RwLockReadGuard};
 
 /// 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.
+/// Global thread pool
 pub struct StyleThreadPool {
     /// How many threads parallel styling can use.
     pub num_threads: usize,
 
     /// The parallel styling thread pool.
-    ///
-    /// For leak-checking purposes, we want to terminate the thread-pool, which
-    /// waits for all the async jobs to complete. Thus the RwLock.
-    style_thread_pool: RwLock<Option<rayon::ThreadPool>>,
+    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();
@@ -56,31 +52,16 @@ fn thread_startup(_index: usize) {
 fn thread_shutdown(_: usize) {
     #[cfg(feature = "gecko")]
     unsafe {
         bindings::Gecko_UnregisterProfilerThread();
         bindings::Gecko_SetJemallocThreadLocalArena(false);
     }
 }
 
-impl StyleThreadPool {
-    /// Shuts down the thread pool, waiting for all work to complete.
-    pub fn shutdown(&self) {
-        let _ = self.style_thread_pool.write().take();
-    }
-
-    /// Returns a reference to the thread pool.
-    ///
-    /// We only really want to give read-only access to the pool, except
-    /// for shutdown().
-    pub fn pool(&self) -> RwLockReadGuard<Option<rayon::ThreadPool>> {
-        self.style_thread_pool.read()
-    }
-}
-
 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")]
@@ -127,19 +108,18 @@ lazy_static! {
                 .start_handler(thread_startup)
                 .exit_handler(thread_shutdown)
                 .stack_size(STYLE_THREAD_STACK_SIZE_KB * 1024)
                 .build();
             workers.ok()
         };
 
         StyleThreadPool {
-            num_threads,
-            style_thread_pool: RwLock::new(pool),
+            num_threads: num_threads,
+            style_thread_pool: pool,
         }
     };
-
     /// Global style data
     pub static ref GLOBAL_STYLE_DATA: GlobalStyleData = GlobalStyleData {
         shared_lock: SharedRwLock::new_leaked(),
         options: StyleSystemOptions::default(),
     };
 }
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -248,20 +248,18 @@ fn traverse_subtree(
 
     if !token.should_traverse() {
         return;
     }
 
     debug!("Traversing subtree from {:?}", element);
 
     let thread_pool_holder = &*STYLE_THREAD_POOL;
-    let pool;
     let thread_pool = if traversal_flags.contains(TraversalFlags::ParallelTraversal) {
-        pool = thread_pool_holder.pool();
-        pool.as_ref()
+        thread_pool_holder.style_thread_pool.as_ref()
     } else {
         None
     };
 
     let traversal = RecalcStyleOnly::new(shared_style_context);
     driver::traverse_dom(&traversal, token, thread_pool);
 }
 
@@ -1412,33 +1410,27 @@ pub unsafe extern "C" fn Servo_StyleShee
         extra_data,
         sheet_bytes,
         mode_to_origin(mode),
         quirks_mode.into(),
         line_number_offset,
         should_record_use_counters,
     );
 
-    if let Some(thread_pool) = STYLE_THREAD_POOL.pool().as_ref() {
+    if let Some(thread_pool) = STYLE_THREAD_POOL.style_thread_pool.as_ref() {
         thread_pool.spawn(|| {
             profiler_label!(Parse);
             async_parser.parse();
         });
     } else {
         async_parser.parse();
     }
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn Servo_ShutdownThreadPool() {
-    debug_assert!(is_main_thread() && !is_in_servo_traversal());
-    STYLE_THREAD_POOL.shutdown();
-}
-
-#[no_mangle]
 pub unsafe extern "C" fn Servo_StyleSheet_FromSharedData(
     extra_data: *mut URLExtraData,
     shared_rules: &ServoCssRules,
 ) -> Strong<RawServoStyleSheetContents> {
     let shared_rules = Locked::<CssRules>::as_arc(&shared_rules);
     Arc::new(StylesheetContents::from_shared_data(
         shared_rules.clone_arc(),
         Origin::UserAgent,
--- a/xpcom/build/XPCOMInit.cpp
+++ b/xpcom/build/XPCOMInit.cpp
@@ -83,17 +83,16 @@
 #include "base/command_line.h"
 #include "base/message_loop.h"
 
 #include "mozilla/ipc/BrowserProcessSubThread.h"
 #include "mozilla/AvailableMemoryTracker.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/CountingAllocatorBase.h"
 #include "mozilla/UniquePtr.h"
-#include "mozilla/ServoStyleConsts.h"
 
 #include "mozilla/ipc/GeckoChildProcessHost.h"
 
 #include "ogg/ogg.h"
 
 #include "GeckoProfiler.h"
 
 #include "jsapi.h"
@@ -765,29 +764,17 @@ nsresult ShutdownXPCOM(nsIServiceManager
   nsComponentManagerImpl::gComponentManager = nullptr;
   nsCategoryManager::Destroy();
 
   // Shut down SystemGroup for main thread dispatching.
   SystemGroup::Shutdown();
 
   GkRust_Shutdown();
 
-#ifdef NS_FREE_PERMANENT_DATA
-  // By the time we're shutting down, there may still be async parse tasks going
-  // on in the Servo thread-pool. This is fairly uncommon, though not
-  // impossible. CSS parsing heavily uses the atom table, so obviously it's not
-  // fine to get rid of it.
-  //
-  // In leak-checking / ASAN / etc. builds, shut down the servo thread-pool,
-  // which will wait for all the work to be done. For other builds, we don't
-  // really want to wait on shutdown for possibly slow tasks. So just leak the
-  // atom table in those.
-  Servo_ShutdownThreadPool();
   NS_ShutdownAtomTable();
-#endif
 
   NS_IF_RELEASE(gDebug);
 
   delete sIOThread;
   sIOThread = nullptr;
 
   delete sMessageLoop;
   sMessageLoop = nullptr;