Skip to content

Commit

Permalink
Auto merge of #16379 - bholley:quieter_stats, r=SimonSapin
Browse files Browse the repository at this point in the history
Limit traversal statistics dumps to subtrees of 50 or more elements and add more values.

On Gecko, we get tons of console spam from 1-element traversals if we don't do this.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16379)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Apr 13, 2017
2 parents f312891 + 687ea0e commit b514168
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 15 deletions.
36 changes: 36 additions & 0 deletions components/style/context.rs
Expand Up @@ -171,6 +171,16 @@ pub struct TraversalStatistics {
pub elements_matched: u32,
/// The number of cache hits from the StyleSharingCache.
pub styles_shared: u32,
/// 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.
Expand All @@ -183,11 +193,21 @@ impl<'a> Add for &'a 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,
selectors: 0,
revalidation_selectors: 0,
dependency_selectors: 0,
declarations: 0,
stylist_rebuilds: 0,
traversal_time_ms: 0.0,
is_parallel: None,
}
Expand All @@ -209,6 +229,11 @@ impl fmt::Display for TraversalStatistics {
try!(writeln!(f, "[PERF],elements_styled,{}", self.elements_styled));
try!(writeln!(f, "[PERF],elements_matched,{}", self.elements_matched));
try!(writeln!(f, "[PERF],styles_shared,{}", self.styles_shared));
try!(writeln!(f, "[PERF],selectors,{}", self.selectors));
try!(writeln!(f, "[PERF],revalidation_selectors,{}", self.revalidation_selectors));
try!(writeln!(f, "[PERF],dependency_selectors,{}", self.dependency_selectors));
try!(writeln!(f, "[PERF],declarations,{}", self.declarations));
try!(writeln!(f, "[PERF],stylist_rebuilds,{}", self.stylist_rebuilds));
try!(writeln!(f, "[PERF],traversal_time_ms,{}", self.traversal_time_ms));
writeln!(f, "[PERF] perf block end")
}
Expand All @@ -222,6 +247,17 @@ impl TraversalStatistics {
{
self.is_parallel = Some(traversal.is_parallel());
self.traversal_time_ms = (time::precise_time_s() - start) * 1000.0;
self.selectors = traversal.shared_context().stylist.num_selectors() as u32;
self.revalidation_selectors = traversal.shared_context().stylist.num_revalidation_selectors() as u32;
self.dependency_selectors = traversal.shared_context().stylist.num_dependencies() as u32;
self.declarations = traversal.shared_context().stylist.num_declarations() as u32;
self.stylist_rebuilds = traversal.shared_context().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.elements_traversed >= 50
}
}

Expand Down
4 changes: 3 additions & 1 deletion components/style/parallel.rs
Expand Up @@ -86,7 +86,9 @@ pub fn traverse_dom<E, D>(traversal: &D,
}
});
aggregate.finish(traversal, start_time.unwrap());
println!("{}", aggregate);
if aggregate.is_large_traversal() {
println!("{}", aggregate);
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions components/style/properties/declaration_block.rs
Expand Up @@ -60,6 +60,11 @@ impl fmt::Debug for PropertyDeclarationBlock {
}

impl PropertyDeclarationBlock {
/// Returns the number of declarations in the block.
pub fn len(&self) -> usize {
self.declarations.len()
}

/// Create an empty block
pub fn new() -> Self {
PropertyDeclarationBlock {
Expand Down
4 changes: 3 additions & 1 deletion components/style/sequential.rs
Expand Up @@ -64,6 +64,8 @@ pub fn traverse_dom<E, D>(traversal: &D,
if dump_stats {
let tlsc = tlc.borrow_mut();
tlsc.statistics.finish(traversal, start_time.unwrap());
println!("{}", tlsc.statistics);
if tlsc.statistics.is_large_traversal() {
println!("{}", tlsc.statistics);
}
}
}
61 changes: 48 additions & 13 deletions components/style/stylist.rs
Expand Up @@ -109,13 +109,22 @@ pub struct Stylist {
rules_source_order: usize,

/// Selector dependencies used to compute restyle hints.
state_deps: DependencySet,
dependencies: DependencySet,

/// Selectors that require explicit cache revalidation (i.e. which depend
/// on state that is not otherwise visible to the cache, like attributes or
/// tree-structural state like child index and pseudos).
#[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
selectors_for_cache_revalidation: Vec<Selector<SelectorImpl>>,

/// The total number of selectors.
num_selectors: usize,

/// The total number of declarations.
num_declarations: usize,

/// The total number of times the stylist has been rebuilt.
num_rebuilds: usize,
}

/// This struct holds data which user of Stylist may want to extract
Expand Down Expand Up @@ -165,9 +174,11 @@ impl Stylist {
precomputed_pseudo_element_decls: Default::default(),
rules_source_order: 0,
rule_tree: RuleTree::new(),
state_deps: DependencySet::new(),

dependencies: DependencySet::new(),
selectors_for_cache_revalidation: vec![],
num_selectors: 0,
num_declarations: 0,
num_rebuilds: 0,
};

SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
Expand All @@ -179,6 +190,31 @@ impl Stylist {
stylist
}

/// Returns the number of selectors.
pub fn num_selectors(&self) -> usize {
self.num_selectors
}

/// Returns the number of declarations.
pub fn num_declarations(&self) -> usize {
self.num_declarations
}

/// Returns the number of times the stylist has been rebuilt.
pub fn num_rebuilds(&self) -> usize {
self.num_rebuilds
}

/// Returns the number of dependencies in the DependencySet.
pub fn num_dependencies(&self) -> usize {
self.dependencies.len()
}

/// Returns the number of revalidation_selectors.
pub fn num_revalidation_selectors(&self) -> usize {
self.selectors_for_cache_revalidation.len()
}

/// Update the stylist for the given document stylesheets, and optionally
/// with a set of user agent stylesheets.
///
Expand All @@ -194,6 +230,7 @@ impl Stylist {
if !(self.is_device_dirty || stylesheets_changed) {
return false;
}
self.num_rebuilds += 1;

let cascaded_rule = ViewportRule {
declarations: viewport::Cascade::from_stylesheets(
Expand All @@ -218,10 +255,11 @@ impl Stylist {

self.precomputed_pseudo_element_decls = Default::default();
self.rules_source_order = 0;
self.state_deps.clear();
self.dependencies.clear();
self.animations.clear();

self.selectors_for_cache_revalidation.clear();
self.num_selectors = 0;
self.num_declarations = 0;

extra_data.clear_font_faces();

Expand All @@ -240,12 +278,6 @@ impl Stylist {
self.add_stylesheet(stylesheet, guards.author, extra_data);
}

debug!("Stylist stats:");
debug!(" - Got {} selectors for cache revalidation",
self.selectors_for_cache_revalidation.len());
debug!(" - Got {} deps for style-hint calculation",
self.state_deps.len());

SelectorImpl::each_precomputed_pseudo_element(|pseudo| {
if let Some(map) = self.pseudos_map.remove(&pseudo) {
let declarations =
Expand Down Expand Up @@ -273,7 +305,10 @@ impl Stylist {
match *rule {
CssRule::Style(ref locked) => {
let style_rule = locked.read_with(&guard);
self.num_declarations += style_rule.block.read_with(&guard).len();

for selector in &style_rule.selectors.0 {
self.num_selectors += 1;
let map = if let Some(ref pseudo) = selector.pseudo_element {
self.pseudos_map
.entry(pseudo.clone())
Expand All @@ -294,7 +329,7 @@ impl Stylist {

for selector in &style_rule.selectors.0 {
let needs_cache_revalidation =
self.state_deps.note_selector(&selector.complex_selector);
self.dependencies.note_selector(&selector.complex_selector);
if needs_cache_revalidation {
self.selectors_for_cache_revalidation.push(selector.clone());
}
Expand Down Expand Up @@ -800,7 +835,7 @@ impl Stylist {
-> RestyleHint
where E: TElement,
{
self.state_deps.compute_hint(element, snapshot)
self.dependencies.compute_hint(element, snapshot)
}
}

Expand Down

0 comments on commit b514168

Please sign in to comment.