Split TraversalStatistics into two parts. r?bholley draft
authorXidorn Quan <me@upsuper.org>
Tue, 13 Mar 2018 10:20:38 +1100
changeset 766528 5cbc14c3690ee8c67ace2ea423a0611704a51960
parent 766125 06e0361a90b902ded79f73f952f6646686752a5d
child 766529 8a05e2e428fb5e0ac423df6e2289e13e9e3bf31c
push id102344
push userxquan@mozilla.com
push dateMon, 12 Mar 2018 23:49:09 +0000
reviewersbholley
milestone60.0a1
Split TraversalStatistics into two parts. r?bholley MozReview-Commit-ID: FRiThhriYrr
servo/components/style/context.rs
servo/components/style/driver.rs
--- a/servo/components/style/context.rs
+++ b/servo/components/style/context.rs
@@ -291,125 +291,122 @@ impl ElementCascadeInputs {
         }
     }
 }
 
 /// Statistics gathered during the traversal. We gather statistics on each
 /// thread and then combine them after the threads join via the Add
 /// implementation below.
 #[derive(Default)]
-pub struct TraversalStatistics {
+pub struct TraversalStatisticsInner {
     /// The total number of elements traversed.
     pub elements_traversed: u32,
     /// The number of elements where has_styles() went from false to true.
     pub elements_styled: u32,
     /// The number of elements for which we performed selector matching.
     pub elements_matched: u32,
     /// The number of cache hits from the StyleSharingCache.
     pub styles_shared: u32,
     /// The number of styles reused via rule node comparison from the
     /// StyleSharingCache.
     pub styles_reused: u32,
+}
+
+/// Implementation of Add to aggregate statistics across different threads.
+impl<'a> ops::Add for &'a TraversalStatisticsInner {
+    type Output = TraversalStatisticsInner;
+    fn add(self, other: Self) -> TraversalStatisticsInner {
+        TraversalStatisticsInner {
+            elements_traversed: self.elements_traversed + other.elements_traversed,
+            elements_styled: self.elements_styled + other.elements_styled,
+            elements_matched: self.elements_matched + other.elements_matched,
+            styles_shared: self.styles_shared + other.styles_shared,
+            styles_reused: self.styles_reused + other.styles_reused,
+        }
+    }
+}
+
+/// Statistics gathered during the traversal plus some other information
+/// get from other sources including stylist.
+#[derive(Default)]
+pub struct TraversalStatistics {
+    /// Statistics gathered during the traversal.
+    pub inner: TraversalStatisticsInner,
     /// The number of selectors in the stylist.
     pub selectors: u32,
     /// The number of revalidation selectors.
     pub revalidation_selectors: u32,
     /// The number of state/attr dependencies in the dependency set.
     pub dependency_selectors: u32,
     /// The number of declarations in the stylist.
     pub declarations: u32,
     /// The number of times the stylist was rebuilt.
     pub stylist_rebuilds: u32,
     /// Time spent in the traversal, in milliseconds.
     pub traversal_time_ms: f64,
     /// Whether this was a parallel traversal.
-    pub is_parallel: Option<bool>,
+    pub is_parallel: bool,
     /// Whether this is a "large" traversal.
-    pub is_large: Option<bool>,
-}
-
-/// Implementation of Add to aggregate statistics across different threads.
-impl<'a> ops::Add for &'a TraversalStatistics {
-    type Output = TraversalStatistics;
-    fn add(self, other: Self) -> TraversalStatistics {
-        debug_assert!(self.traversal_time_ms == 0.0 && other.traversal_time_ms == 0.0,
-                      "traversal_time_ms should be set at the end by the caller");
-        debug_assert!(self.selectors == 0, "set at the end");
-        debug_assert!(self.revalidation_selectors == 0, "set at the end");
-        debug_assert!(self.dependency_selectors == 0, "set at the end");
-        debug_assert!(self.declarations == 0, "set at the end");
-        debug_assert!(self.stylist_rebuilds == 0, "set at the end");
-        TraversalStatistics {
-            elements_traversed: self.elements_traversed + other.elements_traversed,
-            elements_styled: self.elements_styled + other.elements_styled,
-            elements_matched: self.elements_matched + other.elements_matched,
-            styles_shared: self.styles_shared + other.styles_shared,
-            styles_reused: self.styles_reused + other.styles_reused,
-            selectors: 0,
-            revalidation_selectors: 0,
-            dependency_selectors: 0,
-            declarations: 0,
-            stylist_rebuilds: 0,
-            traversal_time_ms: 0.0,
-            is_parallel: None,
-            is_large: None,
-        }
-    }
+    pub is_large: bool,
 }
 
 /// Format the statistics in a way that the performance test harness understands.
 /// See https://bugzilla.mozilla.org/show_bug.cgi?id=1331856#c2
 impl fmt::Display for TraversalStatistics {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         debug_assert!(self.traversal_time_ms != 0.0, "should have set traversal time");
         writeln!(f, "[PERF] perf block start")?;
-        writeln!(f, "[PERF],traversal,{}", if self.is_parallel.unwrap() {
+        writeln!(f, "[PERF],traversal,{}", if self.is_parallel {
             "parallel"
         } else {
             "sequential"
         })?;
-        writeln!(f, "[PERF],elements_traversed,{}", self.elements_traversed)?;
-        writeln!(f, "[PERF],elements_styled,{}", self.elements_styled)?;
-        writeln!(f, "[PERF],elements_matched,{}", self.elements_matched)?;
-        writeln!(f, "[PERF],styles_shared,{}", self.styles_shared)?;
-        writeln!(f, "[PERF],styles_reused,{}", self.styles_reused)?;
+        writeln!(f, "[PERF],elements_traversed,{}", self.inner.elements_traversed)?;
+        writeln!(f, "[PERF],elements_styled,{}", self.inner.elements_styled)?;
+        writeln!(f, "[PERF],elements_matched,{}", self.inner.elements_matched)?;
+        writeln!(f, "[PERF],styles_shared,{}", self.inner.styles_shared)?;
+        writeln!(f, "[PERF],styles_reused,{}", self.inner.styles_reused)?;
         writeln!(f, "[PERF],selectors,{}", self.selectors)?;
         writeln!(f, "[PERF],revalidation_selectors,{}", self.revalidation_selectors)?;
         writeln!(f, "[PERF],dependency_selectors,{}", self.dependency_selectors)?;
         writeln!(f, "[PERF],declarations,{}", self.declarations)?;
         writeln!(f, "[PERF],stylist_rebuilds,{}", self.stylist_rebuilds)?;
         writeln!(f, "[PERF],traversal_time_ms,{}", self.traversal_time_ms)?;
         writeln!(f, "[PERF] perf block end")
     }
 }
 
 impl TraversalStatistics {
-    /// Computes the traversal time given the start time in seconds.
-    pub fn finish<E, D>(&mut self, traversal: &D, parallel: bool, start: f64)
+    /// Generate complete traversal statistics.
+    ///
+    /// The traversal time is computed given the start time in seconds.
+    pub fn new<E, D>(
+        inner: TraversalStatisticsInner,
+        traversal: &D,
+        parallel: bool,
+        start: f64
+    ) -> TraversalStatistics
     where
         E: TElement,
         D: DomTraversal<E>,
     {
         let threshold = traversal.shared_context().options.style_statistics_threshold;
         let stylist = traversal.shared_context().stylist;
-
-        self.is_parallel = Some(parallel);
-        self.is_large = Some(self.elements_traversed as usize >= threshold);
-        self.traversal_time_ms = (time::precise_time_s() - start) * 1000.0;
-        self.selectors = stylist.num_selectors() as u32;
-        self.revalidation_selectors = stylist.num_revalidation_selectors() as u32;
-        self.dependency_selectors = stylist.num_invalidations() as u32;
-        self.declarations = stylist.num_declarations() as u32;
-        self.stylist_rebuilds = stylist.num_rebuilds() as u32;
-    }
-
-    /// Returns whether this traversal is 'large' in order to avoid console spam
-    /// from lots of tiny traversals.
-    pub fn is_large_traversal(&self) -> bool {
-        self.is_large.unwrap()
+        let is_large = inner.elements_traversed as usize >= threshold;
+        TraversalStatistics {
+            inner,
+            selectors: stylist.num_selectors() as u32,
+            revalidation_selectors: stylist.num_revalidation_selectors() as u32,
+            dependency_selectors: stylist.num_invalidations() as u32,
+            declarations: stylist.num_declarations() as u32,
+            stylist_rebuilds: stylist.num_rebuilds() as u32,
+            traversal_time_ms: (time::precise_time_s() - start) * 1000.0,
+            is_parallel: parallel,
+            is_large
+        }
     }
 }
 
 #[cfg(feature = "gecko")]
 bitflags! {
     /// Represents which tasks are performed in a SequentialTask of
     /// UpdateAnimations which is a result of normal restyle.
     pub struct UpdateAnimationsTasks: u8 {
@@ -709,17 +706,17 @@ pub struct ThreadLocalStyleContext<E: TE
     /// could create another ThreadLocalStyleContext for style computation.
     pub tasks: SequentialTaskList<E>,
     /// ElementSelectorFlags that need to be applied after the traversal is
     /// complete. This map is used in cases where the matching algorithm needs
     /// to set flags on elements it doesn't have exclusive access to (i.e. other
     /// than the current element).
     pub selector_flags: SelectorFlagsMap<E>,
     /// Statistics about the traversal.
-    pub statistics: TraversalStatistics,
+    pub statistics: TraversalStatisticsInner,
     /// The struct used to compute and cache font metrics from style
     /// for evaluation of the font-relative em/ch units and font-size
     pub font_metrics_provider: E::FontMetricsProvider,
     /// A checker used to ensure that parallel.rs does not recurse indefinitely
     /// even on arbitrarily deep trees.  See Gecko bug 1376883.
     pub stack_limit_checker: StackLimitChecker,
     /// A cache for nth-index-like selectors.
     pub nth_index_cache: NthIndexCache,
@@ -731,34 +728,34 @@ impl<E: TElement> ThreadLocalStyleContex
     pub fn new(shared: &SharedStyleContext) -> Self {
         ThreadLocalStyleContext {
             sharing_cache: StyleSharingCache::new(),
             rule_cache: RuleCache::new(),
             bloom_filter: StyleBloom::new(),
             new_animations_sender: shared.local_context_creation_data.lock().unwrap().new_animations_sender.clone(),
             tasks: SequentialTaskList(Vec::new()),
             selector_flags: SelectorFlagsMap::new(),
-            statistics: TraversalStatistics::default(),
+            statistics: TraversalStatisticsInner::default(),
             font_metrics_provider: E::FontMetricsProvider::create_from(shared),
             stack_limit_checker: StackLimitChecker::new(
                 (STYLE_THREAD_STACK_SIZE_KB - STACK_SAFETY_MARGIN_KB) * 1024),
             nth_index_cache: NthIndexCache::default(),
         }
     }
 
     #[cfg(feature = "gecko")]
     /// Creates a new `ThreadLocalStyleContext` from a shared one.
     pub fn new(shared: &SharedStyleContext) -> Self {
         ThreadLocalStyleContext {
             sharing_cache: StyleSharingCache::new(),
             rule_cache: RuleCache::new(),
             bloom_filter: StyleBloom::new(),
             tasks: SequentialTaskList(Vec::new()),
             selector_flags: SelectorFlagsMap::new(),
-            statistics: TraversalStatistics::default(),
+            statistics: TraversalStatisticsInner::default(),
             font_metrics_provider: E::FontMetricsProvider::create_from(shared),
             stack_limit_checker: StackLimitChecker::new(
                 (STYLE_THREAD_STACK_SIZE_KB - STACK_SAFETY_MARGIN_KB) * 1024),
             nth_index_cache: NthIndexCache::default(),
         }
     }
 }
 
--- a/servo/components/style/driver.rs
+++ b/servo/components/style/driver.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 http://mozilla.org/MPL/2.0/. */
 
 //! Implements traversal over the DOM tree. The traversal starts in sequential
 //! mode, and optionally parallelizes as it discovers work.
 
 #![deny(missing_docs)]
 
-use context::{StyleContext, ThreadLocalStyleContext};
+use context::{StyleContext, ThreadLocalStyleContext, TraversalStatistics};
 use dom::{SendNode, TElement, TNode};
 use parallel;
 use parallel::{DispatchMode, WORK_UNIT_MAX};
 use rayon;
 use scoped_tls::ScopedTLS;
 use std::collections::VecDeque;
 use std::mem;
 use time;
@@ -123,14 +123,19 @@ where
             aggregate = slots.iter().fold(aggregate, |acc, t| {
                 match *t.borrow() {
                     None => acc,
                     Some(ref cx) => &cx.statistics + &acc,
                 }
             });
         }
 
-        aggregate.finish(traversal, parallel, start_time.unwrap());
-        if aggregate.is_large_traversal() {
-             println!("{}", aggregate);
+        let stats = TraversalStatistics::new(
+            aggregate,
+            traversal,
+            parallel,
+            start_time.unwrap()
+        );
+        if stats.is_large {
+             println!("{}", stats);
         }
     }
 }