From 1d2e9e3ef7005a7913e20b779657d065992a0152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 9 Oct 2017 17:35:17 +0200 Subject: [PATCH] style: Optimize custom properties cycle removal. After #18791, this is the major custom_properties perf bottleneck in the testcase from bug 1405411. I'm looking into how to efficiently merge this into `substitute_all`, but meanwhile this is worth landing, and makes most of the overhead go away. MozReview-Commit-ID: LMUGc8kPhhE --- components/style/custom_properties.rs | 42 ++++++++++++++++----------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 5507739a71f..ca9e4194970 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -13,6 +13,7 @@ use properties::{CSSWideKeyword, DeclaredValue}; use selector_map::{PrecomputedHashSet, PrecomputedHashMap, PrecomputedDiagnosticHashMap}; use selectors::parser::SelectorParseError; use servo_arc::Arc; +use smallvec::SmallVec; use std::ascii::AsciiExt; use std::borrow::{Borrow, Cow}; use std::fmt; @@ -546,41 +547,48 @@ impl<'a> CustomPropertiesBuilder<'a> { fn remove_cycles(map: &mut CustomPropertiesMap) { let mut to_remove = PrecomputedHashSet::default(); { + type VisitedNamesStack<'a> = SmallVec<[&'a Name; 10]>; + let mut visited = PrecomputedHashSet::default(); - let mut stack = Vec::new(); - for name in &map.index { - walk(map, name, &mut stack, &mut visited, &mut to_remove); + let mut stack = VisitedNamesStack::new(); + for (name, value) in map.iter() { + walk(map, name, value, &mut stack, &mut visited, &mut to_remove); fn walk<'a>( map: &'a CustomPropertiesMap, name: &'a Name, - stack: &mut Vec<&'a Name>, + value: &'a Arc, + stack: &mut VisitedNamesStack<'a>, visited: &mut PrecomputedHashSet<&'a Name>, to_remove: &mut PrecomputedHashSet, ) { + if value.references.is_empty() { + return; + } + let already_visited_before = !visited.insert(name); if already_visited_before { return } - if let Some(ref value) = map.get(name) { - if !value.references.is_empty() { - stack.push(name); - for next in value.references.iter() { - if let Some(position) = stack.iter().position(|x| *x == next) { - // Found a cycle - for &in_cycle in &stack[position..] { - to_remove.insert(in_cycle.clone()); - } - } else { - walk(map, next, stack, visited, to_remove); - } + + stack.push(name); + for next in value.references.iter() { + if let Some(position) = stack.iter().position(|x| *x == next) { + // Found a cycle + for &in_cycle in &stack[position..] { + to_remove.insert(in_cycle.clone()); + } + } else { + if let Some(value) = map.get(next) { + walk(map, next, value, stack, visited, to_remove); } - stack.pop(); } } + stack.pop(); } } } + for name in to_remove { map.remove(&name); }