From c354f224ff885c9f6c579ee6b59ec8fa4e4a6322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 8 Oct 2017 19:34:57 +0200 Subject: [PATCH] style: Refactor the custom properties map to have less temporary hashmap entries. --- components/style/custom_properties.rs | 126 +++++++++++++------------- 1 file changed, 64 insertions(+), 62 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index b036b2584aa..692db8cfb09 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -57,7 +57,7 @@ pub struct BorrowedSpecifiedValue<'a> { css: &'a str, first_token_type: TokenSerializationType, last_token_type: TokenSerializationType, - references: Option<&'a PrecomputedHashSet>, + references: &'a PrecomputedHashSet, } /// A computed value is just a set of tokens as well, until we resolve variables @@ -155,6 +155,11 @@ where self.values.len() } + /// Returns whether this map is empty. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Remove an item given its key. fn remove(&mut self, key: &Q) -> Option where @@ -461,7 +466,7 @@ fn parse_var_function<'i, 't>( /// properties. pub struct CustomPropertiesBuilder<'a> { seen: PrecomputedHashSet<&'a Name>, - specified_custom_properties: Option>>, + specified_custom_properties: OrderedMap<&'a Name, Option>>, inherited: Option<&'a Arc>, } @@ -470,7 +475,7 @@ impl<'a> CustomPropertiesBuilder<'a> { pub fn new(inherited: Option<&'a Arc>) -> Self { Self { seen: PrecomputedHashSet::default(), - specified_custom_properties: None, + specified_custom_properties: OrderedMap::new(), inherited, } } @@ -486,38 +491,19 @@ impl<'a> CustomPropertiesBuilder<'a> { return; } - let map = match self.specified_custom_properties { - Some(ref mut map) => map, - None => { - let mut map = OrderedMap::new(); - if let Some(inherited) = self.inherited { - for (name, inherited_value) in inherited.iter() { - map.insert(name, BorrowedSpecifiedValue { - css: &inherited_value.css, - first_token_type: inherited_value.first_token_type, - last_token_type: inherited_value.last_token_type, - references: None - }) - } - } - self.specified_custom_properties = Some(map); - self.specified_custom_properties.as_mut().unwrap() - } - }; - match specified_value { DeclaredValue::Value(ref specified_value) => { - map.insert(name, BorrowedSpecifiedValue { + self.specified_custom_properties.insert(name, Some(BorrowedSpecifiedValue { css: &specified_value.css, first_token_type: specified_value.first_token_type, last_token_type: specified_value.last_token_type, - references: Some(&specified_value.references), - }); + references: &specified_value.references, + })); }, DeclaredValue::WithVariables(_) => unreachable!(), DeclaredValue::CSSWideKeyword(keyword) => match keyword { CSSWideKeyword::Initial => { - map.remove(&name); + self.specified_custom_properties.insert(name, None); } CSSWideKeyword::Unset | // Custom properties are inherited by default. CSSWideKeyword::Inherit => {} // The inherited value is what we already have. @@ -532,13 +518,12 @@ impl<'a> CustomPropertiesBuilder<'a> { /// /// Otherwise, just use the inherited custom properties map. pub fn build(mut self) -> Option> { - let mut map = match self.specified_custom_properties.take() { - Some(map) => map, - None => return self.inherited.cloned(), - }; + if self.specified_custom_properties.is_empty() { + return self.inherited.cloned(); + } - remove_cycles(&mut map); - Some(Arc::new(substitute_all(map))) + remove_cycles(&mut self.specified_custom_properties); + Some(Arc::new(substitute_all(self.specified_custom_properties, self.inherited))) } } @@ -546,7 +531,7 @@ impl<'a> CustomPropertiesBuilder<'a> { /// /// The initial value of a custom property is represented by this property not /// being in the map. -fn remove_cycles(map: &mut OrderedMap<&Name, BorrowedSpecifiedValue>) { +fn remove_cycles(map: &mut OrderedMap<&Name, Option>) { let mut to_remove = PrecomputedHashSet::default(); { let mut visited = PrecomputedHashSet::default(); @@ -555,7 +540,7 @@ fn remove_cycles(map: &mut OrderedMap<&Name, BorrowedSpecifiedValue>) { walk(map, name, &mut stack, &mut visited, &mut to_remove); fn walk<'a>( - map: &OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>, + map: &OrderedMap<&'a Name, Option>>, name: &'a Name, stack: &mut Vec<&'a Name>, visited: &mut PrecomputedHashSet<&'a Name>, @@ -565,10 +550,10 @@ fn remove_cycles(map: &mut OrderedMap<&Name, BorrowedSpecifiedValue>) { if already_visited_before { return } - if let Some(value) = map.get(&name) { - if let Some(references) = value.references { + if let Some(&Some(ref value)) = map.get(&name) { + if !value.references.is_empty() { stack.push(name); - for next in references { + for next in value.references { if let Some(position) = stack.iter().position(|&x| x == next) { // Found a cycle for &in_cycle in &stack[position..] { @@ -591,16 +576,33 @@ fn remove_cycles(map: &mut OrderedMap<&Name, BorrowedSpecifiedValue>) { /// Replace `var()` functions for all custom properties. fn substitute_all( - specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue> + specified_values_map: OrderedMap<&Name, Option>, + inherited: Option<&Arc>, ) -> CustomPropertiesMap { - let mut custom_properties_map = CustomPropertiesMap::new(); + let mut custom_properties_map = match inherited { + None => CustomPropertiesMap::new(), + Some(inherited) => (**inherited).clone(), + }; + let mut invalid = PrecomputedHashSet::default(); + for (name, value) in specified_values_map.iter() { - // If this value is invalid at computed-time it won’t be inserted in computed_values_map. - // Nothing else to do. + let value = match *value { + Some(ref v) => v, + None => { + custom_properties_map.remove(*name); + continue; + } + }; + let _ = substitute_one( - name, value, &specified_values_map, None, - &mut custom_properties_map, &mut invalid); + name, + value, + &specified_values_map, + None, + &mut custom_properties_map, + &mut invalid, + ); } custom_properties_map @@ -613,22 +615,23 @@ fn substitute_all( fn substitute_one( name: &Name, specified_value: &BorrowedSpecifiedValue, - specified_values_map: &OrderedMap<&Name, BorrowedSpecifiedValue>, + specified_values_map: &OrderedMap<&Name, Option>, partial_computed_value: Option<&mut ComputedValue>, custom_properties_map: &mut CustomPropertiesMap, invalid: &mut PrecomputedHashSet, ) -> Result { - if let Some(computed_value) = custom_properties_map.get(&name) { - if let Some(partial_computed_value) = partial_computed_value { - partial_computed_value.push_variable(computed_value) - } - return Ok(computed_value.last_token_type) - } - if invalid.contains(name) { return Err(()); } - let computed_value = if specified_value.references.map_or(false, |set| !set.is_empty()) { + + let computed_value = if specified_value.references.is_empty() { + // The specified value contains no var() reference + ComputedValue { + css: specified_value.css.to_owned(), + first_token_type: specified_value.first_token_type, + last_token_type: specified_value.last_token_type, + } + } else { let mut partial_computed_value = ComputedValue::empty(); let mut input = ParserInput::new(&specified_value.css); let mut input = Parser::new(&mut input); @@ -636,9 +639,15 @@ fn substitute_one( let result = substitute_block( &mut input, &mut position, &mut partial_computed_value, &mut |name, partial_computed_value| { - if let Some(other_specified_value) = specified_values_map.get(&name) { - substitute_one(name, other_specified_value, specified_values_map, - Some(partial_computed_value), custom_properties_map, invalid) + if let Some(other_specified_value) = specified_values_map.get(&name).and_then(|v| v.as_ref()) { + substitute_one( + name, + other_specified_value, + specified_values_map, + Some(partial_computed_value), + custom_properties_map, + invalid, + ) } else { Err(()) } @@ -651,13 +660,6 @@ fn substitute_one( invalid.insert(name.clone()); return Err(()) } - } else { - // The specified value contains no var() reference - ComputedValue { - css: specified_value.css.to_owned(), - first_token_type: specified_value.first_token_type, - last_token_type: specified_value.last_token_type, - } }; if let Some(partial_computed_value) = partial_computed_value { partial_computed_value.push_variable(&computed_value)