servo: Merge #18366 - style: Avoid dropping the other threads' TLS contexts too early (from emilio:fix-statistics-crash); r=bholley
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 04 Sep 2017 11:54:15 -0500
changeset 428275 1130f8849a635f185ab6d364338368f89e9889c4
parent 428274 714cf1e9165985b199010b120e6ef51e1e3d43e3
child 428276 fc5fc58f42a3ebab01c6e83901a2dde2435b0933
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
milestone57.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
servo: Merge #18366 - style: Avoid dropping the other threads' TLS contexts too early (from emilio:fix-statistics-crash); r=bholley When collecting style statistics, we have this path that moves the TLS contexts to be dropped before the local context. Since destroying the TLS context runs the sequential task queue, that means that sequential tasks would be executed sooner than usual, before we drop the main thread TLS context. Since we have that reuse of the main thread context's bloom filter, and some tasks end up creating one (Servo_StyleSet_GetBaseComputedValuesForElement, I'm looking at you), we may borrow the bloom filter before we're done with it on the traversal code path. This was hitting on YouTube, when DUMP_STYLE_STATISTICS was used. Source-Repo: https://github.com/servo/servo Source-Revision: 4608955949aa20a2d2ed251b56e9a5a3cf5681c5
servo/components/style/driver.rs
--- a/servo/components/style/driver.rs
+++ b/servo/components/style/driver.rs
@@ -8,17 +8,16 @@
 #![deny(missing_docs)]
 
 use context::{StyleContext, ThreadLocalStyleContext};
 use dom::{SendNode, TElement, TNode};
 use parallel;
 use parallel::{DispatchMode, WORK_UNIT_MAX};
 use rayon;
 use scoped_tls::ScopedTLS;
-use std::borrow::Borrow;
 use std::collections::VecDeque;
 use std::mem;
 use time;
 use traversal::{DomTraversal, PerLevelTraversalData, PreTraverseToken};
 
 /// Do a DOM traversal for top-down and (optionally) bottom-up processing,
 /// generic over `D`.
 ///
@@ -110,22 +109,22 @@ where
         }
     }
 
     // Dump statistics to stdout if requested.
     if dump_stats {
         let mut aggregate =
             mem::replace(&mut context.thread_local.statistics, Default::default());
         let parallel = maybe_tls.is_some();
-        if let Some(tls) = maybe_tls {
+        if let Some(ref mut tls) = maybe_tls {
             let slots = unsafe { tls.unsafe_get() };
             aggregate = slots.iter().fold(aggregate, |acc, t| {
                 match *t.borrow() {
                     None => acc,
-                    Some(ref cx) => &cx.borrow().statistics + &acc,
+                    Some(ref cx) => &cx.statistics + &acc,
                 }
             });
         }
         aggregate.finish(traversal, parallel, start_time.unwrap());
         if aggregate.is_large_traversal() {
             println!("{}", aggregate);
         }
     }