Auto merge of #18679 - emilio:precomputed-custom-props, r=manishearth

style: Use PrecomputedHasher for custom properties.

<!-- 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/18679)
<!-- Reviewable:end -->

Cargo.lock

@@ -2337,7 +2337,7 @@ dependencies = [
 
 [[package]]
 name = "precomputed-hash"
-version = "0.1.0"
+version = "0.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
@@ -2729,7 +2729,7 @@ dependencies = [
  "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "phf 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "phf_codegen 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)",
- "precomputed-hash 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "precomputed-hash 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "servo_arc 0.0.1",
  "size_of_test 0.0.1",
  "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3088,7 +3088,7 @@ dependencies = [
  "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "phf_shared 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)",
- "precomputed-hash 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "precomputed-hash 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.14 (registry+https://github.com/rust-lang/crates.io-index)",
  "string_cache_codegen 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "string_cache_shared 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3152,7 +3152,7 @@ dependencies = [
  "owning_ref 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "pdqsort 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "precomputed-hash 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "precomputed-hash 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rayon 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "regex 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.19.0",
@@ -3958,7 +3958,7 @@ dependencies = [
 "checksum pkg-config 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "3a8b4c6b8165cd1a1cd4b9b120978131389f64bdaf456435caa41e630edba903"
 "checksum plane-split 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e57800a97ca52c556db6b6184a3201f05366ad5e11876f7d17e234589ca2fa26"
 "checksum png 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "6535266009941ceac9a17e6d681cd2adc75611cd4833db853282e8d4c470239c"
-"checksum precomputed-hash 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "cdf1fc3616b3ef726a847f2cd2388c646ef6a1f1ba4835c2629004da48184150"
+"checksum precomputed-hash 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c"
 "checksum procedural-masquerade 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c93cdc1fb30af9ddf3debc4afbdb0f35126cbd99daa229dd76cdd5349b41d989"
 "checksum pulldown-cmark 0.0.8 (registry+https://github.com/rust-lang/crates.io-index)" = "1058d7bb927ca067656537eec4e02c2b4b70eaaa129664c5b90c111e20326f41"
 "checksum push-trait 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fdc13b1a53bc505b526086361221aaa612fefb9b0ecf2853f9d31f807764e004"

components/style/Cargo.toml

@@ -61,7 +61,7 @@ ordered-float = "0.4"
 owning_ref = "0.3.3"
 parking_lot = "0.4"
 pdqsort = "0.1.0"
-precomputed-hash = "0.1"
+precomputed-hash = "0.1.1"
 rayon = "0.8"
 selectors = { path = "../selectors" }
 serde = {version = "1.0", optional = true, features = ["derive"]}

components/style/custom_properties.rs

@@ -8,9 +8,10 @@
 
 use Atom;
 use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType};
-use hash::{HashMap, HashSet};
 use parser::ParserContext;
+use precomputed_hash::PrecomputedHash;
 use properties::{CSSWideKeyword, DeclaredValue};
+use selector_map::{PrecomputedHashSet, PrecomputedHashMap};
 use selectors::parser::SelectorParseError;
 use servo_arc::Arc;
 use std::ascii::AsciiExt;
@@ -49,7 +50,7 @@ pub struct SpecifiedValue {
     last_token_type: TokenSerializationType,
 
     /// Custom property names in var() functions.
-    references: HashSet<Name>,
+    references: PrecomputedHashSet<Name>,
 }
 
 /// This struct is a cheap borrowed version of a `SpecifiedValue`.
@@ -57,7 +58,7 @@ pub struct BorrowedSpecifiedValue<'a> {
     css: &'a str,
     first_token_type: TokenSerializationType,
     last_token_type: TokenSerializationType,
-    references: Option<&'a HashSet<Name>>,
+    references: Option<&'a PrecomputedHashSet<Name>>,
 }
 
 /// A computed value is just a set of tokens as well, until we resolve variables
@@ -99,23 +100,23 @@ pub type CustomPropertiesMap = OrderedMap<Name, ComputedValue>;
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct OrderedMap<K, V>
 where
-    K: Eq + Hash + Clone,
+    K: PrecomputedHash + Hash + Eq + Clone,
 {
     /// Key index.
     index: Vec<K>,
     /// Key-value map.
-    values: HashMap<K, V>,
+    values: PrecomputedHashMap<K, V>,
 }
 
 impl<K, V> OrderedMap<K, V>
 where
-    K: Eq + Hash + Clone,
+    K: Eq + PrecomputedHash + Hash + Clone,
 {
     /// Creates a new ordered map.
     pub fn new() -> Self {
         OrderedMap {
             index: Vec::new(),
-            values: HashMap::new(),
+            values: PrecomputedHashMap::default(),
         }
     }
 
@@ -157,7 +158,7 @@ where
     fn remove<Q: ?Sized>(&mut self, key: &Q) -> Option<V>
     where
         K: Borrow<Q>,
-        Q: Hash + Eq,
+        Q: PrecomputedHash + Hash + Eq,
     {
         let index = match self.index.iter().position(|k| k.borrow() == key) {
             Some(p) => p,
@@ -174,7 +175,7 @@ where
 /// added to the key-value map.
 pub struct OrderedMapIterator<'a, K, V>
 where
-    K: 'a + Eq + Hash + Clone, V: 'a,
+    K: 'a + Eq + PrecomputedHash + Hash + Clone, V: 'a,
 {
     /// The OrderedMap itself.
     inner: &'a OrderedMap<K, V>,
@@ -184,7 +185,7 @@ where
 
 impl<'a, K, V> Iterator for OrderedMapIterator<'a, K, V>
 where
-    K: Eq + Hash + Clone,
+    K: Eq + PrecomputedHash + Hash + Clone,
 {
     type Item = (&'a K, &'a V);
 
@@ -239,9 +240,11 @@ impl ComputedValue {
 
 impl SpecifiedValue {
     /// Parse a custom property SpecifiedValue.
-    pub fn parse<'i, 't>(_context: &ParserContext, input: &mut Parser<'i, 't>)
+    pub fn parse<'i, 't>(
-                         -> Result<Box<Self>, ParseError<'i>> {
+        _context: &ParserContext,
-        let mut references = Some(HashSet::new());
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Box<Self>, ParseError<'i>> {
+        let mut references = Some(PrecomputedHashSet::default());
         let (first, css, last) = parse_self_contained_declaration_value(input, &mut references)?;
         Ok(Box::new(SpecifiedValue {
             css: css.into_owned(),
@@ -260,14 +263,14 @@ pub fn parse_non_custom_with_var<'i, 't>
     Ok((first_token_type, css))
 }
 
-fn parse_self_contained_declaration_value<'i, 't>
+fn parse_self_contained_declaration_value<'i, 't>(
-                                         (input: &mut Parser<'i, 't>,
+    input: &mut Parser<'i, 't>,
-                                          references: &mut Option<HashSet<Name>>)
+    references: &mut Option<PrecomputedHashSet<Name>>
-                                          -> Result<(
+) -> Result<
-                                              TokenSerializationType,
+    (TokenSerializationType, Cow<'i, str>, TokenSerializationType),
-                                              Cow<'i, str>,
+    ParseError<'i>
-                                              TokenSerializationType
+>
-                                          ), ParseError<'i>> {
+{
     let start_position = input.position();
     let mut missing_closing_characters = String::new();
     let (first, last) = parse_declaration_value(input, references, &mut missing_closing_characters)?;
@@ -283,11 +286,11 @@ fn parse_self_contained_declaration_value<'i, 't>
 }
 
 /// https://drafts.csswg.org/css-syntax-3/#typedef-declaration-value
-fn parse_declaration_value<'i, 't>
+fn parse_declaration_value<'i, 't>(
-                          (input: &mut Parser<'i, 't>,
+    input: &mut Parser<'i, 't>,
-                           references: &mut Option<HashSet<Name>>,
+    references: &mut Option<PrecomputedHashSet<Name>>,
-                           missing_closing_characters: &mut String)
+    missing_closing_characters: &mut String
-                          -> Result<(TokenSerializationType, TokenSerializationType), ParseError<'i>> {
+) -> Result<(TokenSerializationType, TokenSerializationType), ParseError<'i>> {
     input.parse_until_before(Delimiter::Bang | Delimiter::Semicolon, |input| {
         // Need at least one token
         let start = input.state();
@@ -300,12 +303,11 @@ fn parse_declaration_value<'i, 't>
 
 /// Like parse_declaration_value, but accept `!` and `;` since they are only
 /// invalid at the top level
-fn parse_declaration_value_block<'i, 't>
+fn parse_declaration_value_block<'i, 't>(
-                                (input: &mut Parser<'i, 't>,
+    input: &mut Parser<'i, 't>,
-                                 references: &mut Option<HashSet<Name>>,
+    references: &mut Option<PrecomputedHashSet<Name>>,
-                                 missing_closing_characters: &mut String)
+    missing_closing_characters: &mut String
-                                 -> Result<(TokenSerializationType, TokenSerializationType),
+) -> Result<(TokenSerializationType, TokenSerializationType), ParseError<'i>> {
-                                           ParseError<'i>> {
     let mut token_start = input.position();
     let mut token = match input.next_including_whitespace_and_comments() {
         // FIXME: remove clone() when borrows are non-lexical
@@ -416,9 +418,10 @@ fn parse_declaration_value_block<'i, 't>
 }
 
 // If the var function is valid, return Ok((custom_property_name, fallback))
-fn parse_var_function<'i, 't>(input: &mut Parser<'i, 't>,
+fn parse_var_function<'i, 't>(
-                              references: &mut Option<HashSet<Name>>)
+    input: &mut Parser<'i, 't>,
-                              -> Result<(), ParseError<'i>> {
+    references: &mut Option<PrecomputedHashSet<Name>>
+) -> Result<(), ParseError<'i>> {
     let name = input.expect_ident_cloned()?;
     let name: Result<_, ParseError> =
         parse_name(&name)
@@ -443,11 +446,13 @@ fn parse_var_function<'i, 't>(input: &mut Parser<'i, 't>,
 
 /// Add one custom property declaration to a map, unless another with the same
 /// name was already there.
-pub fn cascade<'a>(custom_properties: &mut Option<OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>>,
+pub fn cascade<'a>(
-                   inherited: &'a Option<Arc<CustomPropertiesMap>>,
+    custom_properties: &mut Option<OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>>,
-                   seen: &mut HashSet<&'a Name>,
+    inherited: &'a Option<Arc<CustomPropertiesMap>>,
-                   name: &'a Name,
+    seen: &mut PrecomputedHashSet<&'a Name>,
-                   specified_value: DeclaredValue<'a, Box<SpecifiedValue>>) {
+    name: &'a Name,
+    specified_value: DeclaredValue<'a, Box<SpecifiedValue>>
+) {
     let was_already_present = !seen.insert(name);
     if was_already_present {
         return;
@@ -514,18 +519,20 @@ pub fn finish_cascade(specified_values_map: Option<OrderedMap<&Name, BorrowedSpe
 /// 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>) {
-    let mut to_remove = HashSet::new();
+    let mut to_remove = PrecomputedHashSet::default();
     {
-        let mut visited = HashSet::new();
+        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);
 
-            fn walk<'a>(map: &OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>,
+            fn walk<'a>(
-                        name: &'a Name,
+                map: &OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>,
-                        stack: &mut Vec<&'a Name>,
+                name: &'a Name,
-                        visited: &mut HashSet<&'a Name>,
+                stack: &mut Vec<&'a Name>,
-                        to_remove: &mut HashSet<Name>) {
+                visited: &mut PrecomputedHashSet<&'a Name>,
+                to_remove: &mut PrecomputedHashSet<Name>,
+            ) {
                 let already_visited_before = !visited.insert(name);
                 if already_visited_before {
                     return
@@ -555,10 +562,11 @@ fn remove_cycles(map: &mut OrderedMap<&Name, BorrowedSpecifiedValue>) {
 }
 
 /// Replace `var()` functions for all custom properties.
-fn substitute_all(specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue>)
+fn substitute_all(
-                  -> CustomPropertiesMap {
+    specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue>
+) -> CustomPropertiesMap {
     let mut custom_properties_map = CustomPropertiesMap::new();
-    let mut invalid = HashSet::new();
+    let mut invalid = PrecomputedHashSet::default();
     for name in &specified_values_map.index {
         let value = specified_values_map.get(name).unwrap();
 
@@ -576,13 +584,14 @@ fn substitute_all(specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue
 /// Also recursively record results for other custom properties referenced by `var()` functions.
 /// Return `Err(())` for invalid at computed time.
 /// or `Ok(last_token_type that was pushed to partial_computed_value)` otherwise.
-fn substitute_one(name: &Name,
+fn substitute_one(
-                  specified_value: &BorrowedSpecifiedValue,
+    name: &Name,
-                  specified_values_map: &OrderedMap<&Name, BorrowedSpecifiedValue>,
+    specified_value: &BorrowedSpecifiedValue,
-                  partial_computed_value: Option<&mut ComputedValue>,
+    specified_values_map: &OrderedMap<&Name, BorrowedSpecifiedValue>,
-                  custom_properties_map: &mut CustomPropertiesMap,
+    partial_computed_value: Option<&mut ComputedValue>,
-                  invalid: &mut HashSet<Name>)
+    custom_properties_map: &mut CustomPropertiesMap,
-                  -> Result<TokenSerializationType, ()> {
+    invalid: &mut PrecomputedHashSet<Name>,
+) -> Result<TokenSerializationType, ()> {
     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)

components/style/properties/declaration_block.rs

@@ -693,8 +693,7 @@ impl PropertyDeclarationBlock {
         inherited_custom_properties: Option<Arc<::custom_properties::CustomPropertiesMap>>,
     ) -> Option<Arc<::custom_properties::CustomPropertiesMap>> {
         let mut custom_properties = None;
-        // FIXME: Use PrecomputedHasher instead.
+        let mut seen_custom = PrecomputedHashSet::default();
-        let mut seen_custom = HashSet::new();
 
         for declaration in self.normal_declaration_iter() {
             if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {

components/style/properties/properties.mako.rs

@@ -15,7 +15,6 @@ use app_units::Au;
 use servo_arc::{Arc, UniqueArc};
 use smallbitvec::SmallBitVec;
 use std::borrow::Cow;
-use hash::HashSet;
 use std::{fmt, mem, ops};
 use std::cell::RefCell;
 #[cfg(feature = "gecko")] use std::ptr;
@@ -35,6 +34,7 @@ use media_queries::Device;
 use parser::ParserContext;
 #[cfg(feature = "gecko")] use properties::longhands::system_font::SystemFont;
 use rule_cache::{RuleCache, RuleCacheConditions};
+use selector_map::PrecomputedHashSet;
 use selector_parser::PseudoElement;
 use selectors::parser::SelectorParseError;
 #[cfg(feature = "servo")] use servo_config::prefs::PREFS;
@@ -3272,7 +3272,7 @@ where
 
     let inherited_custom_properties = inherited_style.custom_properties();
     let mut custom_properties = None;
-    let mut seen_custom = HashSet::new();
+    let mut seen_custom = PrecomputedHashSet::default();
     for (declaration, _cascade_level) in iter_declarations() {
         if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {
             ::custom_properties::cascade(