Revert "Diagnostic map semantics."

This reverts commit f5c5be00a7.

components/hashglobe/src/diagnostic.rs

@@ -1,143 +0,0 @@
-use hash_map::HashMap;
-use std::borrow::Borrow;
-use std::hash::{BuildHasher, Hash};
-
-use FailedAllocationError;
-
-#[derive(Clone, Debug)]
-pub struct DiagnosticHashMap<K, V, S>
-    where K: Eq + Hash,
-          S: BuildHasher
-{
-    map: HashMap<K, V, S>,
-    readonly: bool,
-}
-
-impl<K: Hash + Eq, V, S: BuildHasher> DiagnosticHashMap<K, V, S>
-    where K: Eq + Hash,
-          S: BuildHasher
-{
-    #[inline(always)]
-    pub fn inner(&self) -> &HashMap<K, V, S> {
-        &self.map
-    }
-
-    #[inline(always)]
-    pub fn begin_mutation(&mut self) {
-        assert!(self.readonly);
-        self.readonly = false;
-    }
-
-    #[inline(always)]
-    pub fn end_mutation(&mut self) {
-        assert!(!self.readonly);
-        self.readonly = true;
-    }
-
-    #[inline(always)]
-    pub fn with_hasher(hash_builder: S) -> Self {
-        Self {
-            map: HashMap::<K, V, S>::with_hasher(hash_builder),
-            readonly: true,
-        }
-    }
-
-    #[inline(always)]
-    pub fn len(&self) -> usize {
-        self.map.len()
-    }
-
-    #[inline(always)]
-    pub fn is_empty(&self) -> bool {
-        self.map.is_empty()
-    }
-
-    #[inline(always)]
-    pub fn contains_key<Q: ?Sized>(&self, k: &Q) -> bool
-        where K: Borrow<Q>,
-              Q: Hash + Eq
-    {
-        self.map.contains_key(k)
-    }
-
-    #[inline(always)]
-    pub fn get<Q: ?Sized>(&self, k: &Q) -> Option<&V>
-        where K: Borrow<Q>,
-              Q: Hash + Eq
-    {
-        self.map.get(k)
-    }
-
-    #[inline(always)]
-    pub fn try_get_or_insert_with<F: FnOnce() -> V>(
-        &mut self,
-        key: K,
-        default: F
-    ) -> Result<&mut V, FailedAllocationError> {
-        assert!(!self.readonly);
-        let entry = self.map.try_entry(key)?;
-        Ok(entry.or_insert_with(default))
-    }
-
-    #[inline(always)]
-    pub fn try_insert(&mut self, k: K, v: V) -> Result<Option<V>, FailedAllocationError> {
-        assert!(!self.readonly);
-        self.map.try_insert(k, v)
-    }
-
-    #[inline(always)]
-    pub fn remove<Q: ?Sized>(&mut self, k: &Q) -> Option<V>
-        where K: Borrow<Q>,
-              Q: Hash + Eq
-    {
-        assert!(!self.readonly);
-        self.map.remove(k)
-    }
-
-    #[inline(always)]
-    pub fn clear(&mut self) where K: 'static, V: 'static  {
-        // We handle scoped mutations for the caller here, since callsites that
-        // invoke clear() don't benefit from the coalescing we do around insertion.
-        self.begin_mutation();
-        self.map.clear();
-        self.end_mutation();
-    }
-}
-
-impl<K, V, S> PartialEq for DiagnosticHashMap<K, V, S>
-    where K: Eq + Hash,
-          V: PartialEq,
-          S: BuildHasher
-{
-    fn eq(&self, other: &Self) -> bool {
-        self.map.eq(&other.map)
-    }
-}
-
-impl<K, V, S> Eq for DiagnosticHashMap<K, V, S>
-    where K: Eq + Hash,
-          V: Eq,
-          S: BuildHasher
-{
-}
-
-impl<K, V, S> Default for DiagnosticHashMap<K, V, S>
-    where K: Eq + Hash,
-          S: BuildHasher + Default
-{
-    fn default() -> Self {
-        Self {
-            map: HashMap::default(),
-            readonly: true,
-        }
-    }
-}
-
-impl<K: Hash + Eq, V, S: BuildHasher> Drop for DiagnosticHashMap<K, V, S>
-    where K: Eq + Hash,
-          S: BuildHasher
-{
-    fn drop(&mut self) {
-        debug_assert!(self.readonly, "Dropped while mutating");
-    }
-}

components/hashglobe/src/fake.rs

@@ -76,24 +76,10 @@ impl<K, V, S> HashMap<K, V, S>
         Ok(self.entry(key))
     }
 
-    #[inline(always)]
-    pub fn try_get_or_insert_with<F: FnOnce() -> V>(
-        &mut self,
-        key: K,
-        default: F
-    ) -> Result<&mut V, FailedAllocationError> {
-        Ok(self.entry(key).or_insert_with(default))
-    }
-
     #[inline]
     pub fn try_insert(&mut self, k: K, v: V) -> Result<Option<V>, FailedAllocationError> {
         Ok(self.insert(k, v))
     }
-
-    #[inline(always)]
-    pub fn begin_mutation(&mut self) {}
-    #[inline(always)]
-    pub fn end_mutation(&mut self) {}
 }
 
 #[derive(Clone)]

components/hashglobe/src/lib.rs

@@ -9,7 +9,6 @@
 // except according to those terms.
 
 pub mod alloc;
-pub mod diagnostic;
 pub mod hash_map;
 pub mod hash_set;
 mod shim;

components/malloc_size_of/lib.rs

@@ -499,25 +499,6 @@ impl<K, V, S> MallocSizeOf for hashglobe::hash_map::HashMap<K, V, S>
     }
 }
 
-impl<K, V, S> MallocShallowSizeOf for hashglobe::diagnostic::DiagnosticHashMap<K, V, S>
-    where K: Eq + Hash,
-          S: BuildHasher
-{
-    fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
-        self.inner().shallow_size_of(ops)
-    }
-}
-
-impl<K, V, S> MallocSizeOf for hashglobe::diagnostic::DiagnosticHashMap<K, V, S>
-    where K: Eq + Hash + MallocSizeOf,
-          V: MallocSizeOf,
-          S: BuildHasher,
-{
-    fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
-        self.inner().size_of(ops)
-    }
-}
-
 impl<K, V, S> MallocShallowSizeOf for hashglobe::fake::HashMap<K, V, S>
     where K: Eq + Hash,
           S: BuildHasher,

components/style/context.rs

@@ -322,6 +322,8 @@ pub struct TraversalStatistics {
     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.
@@ -342,6 +344,7 @@ impl<'a> ops::Add for &'a TraversalStatistics {
                       "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 {
@@ -352,6 +355,7 @@ impl<'a> ops::Add for &'a TraversalStatistics {
             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,
@@ -379,6 +383,7 @@ impl fmt::Display for TraversalStatistics {
         writeln!(f, "[PERF],styles_reused,{}", self.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)?;
@@ -400,6 +405,7 @@ impl TraversalStatistics {
         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;
     }

components/style/custom_properties.rs

@@ -10,7 +10,7 @@ use Atom;
 use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType};
 use precomputed_hash::PrecomputedHash;
 use properties::{CSSWideKeyword, DeclaredValue};
-use selector_map::{PrecomputedHashSet, PrecomputedHashMap, PrecomputedDiagnosticHashMap};
+use selector_map::{PrecomputedHashSet, PrecomputedHashMap};
 use selectors::parser::SelectorParseErrorKind;
 use servo_arc::Arc;
 use smallvec::SmallVec;
@@ -88,7 +88,7 @@ where
     /// Key index.
     index: Vec<K>,
     /// Key-value map.
-    values: PrecomputedDiagnosticHashMap<K, V>,
+    values: PrecomputedHashMap<K, V>,
 }
 
 impl<K, V> OrderedMap<K, V>
@@ -99,7 +99,7 @@ where
     pub fn new() -> Self {
         OrderedMap {
             index: Vec::new(),
-            values: PrecomputedDiagnosticHashMap::default(),
+            values: PrecomputedHashMap::default(),
         }
     }
 
@@ -108,9 +108,7 @@ where
         if !self.values.contains_key(&key) {
             self.index.push(key.clone());
         }
-        self.values.begin_mutation();
+        self.values.insert(key, value);
-        self.values.try_insert(key, value).unwrap();
-        self.values.end_mutation();
     }
 
     /// Get a value given its key.
@@ -160,10 +158,7 @@ where
             None => return None,
         };
         self.index.remove(index);
-        self.values.begin_mutation();
+        self.values.remove(key)
-        let result = self.values.remove(key);
-        self.values.end_mutation();
-        result
     }
 
     fn remove_set<S>(&mut self, set: &::hash::HashSet<K, S>)
@@ -211,7 +206,7 @@ where
         };
 
         self.pos += 1;
-        let value = &self.inner.values.get(key).unwrap();
+        let value = &self.inner.values[key];
 
         Some((key, value))
     }

components/style/hash.rs

@@ -13,16 +13,11 @@ use fnv;
 pub use hashglobe::hash_map::HashMap;
 #[cfg(feature = "gecko")]
 pub use hashglobe::hash_set::HashSet;
-#[cfg(feature = "gecko")]
+
-pub use hashglobe::diagnostic::DiagnosticHashMap;
 
 #[cfg(feature = "servo")]
 pub use hashglobe::fake::{HashMap, HashSet};
 
-/// Alias to use regular HashMaps everywhere in Servo.
-#[cfg(feature = "servo")]
-pub type DiagnosticHashMap<K, V, S> = HashMap<K, V, S>;
-
 /// Appropriate reexports of hash_map types
 pub mod map {
     #[cfg(feature = "gecko")]

components/style/invalidation/element/invalidation_map.rs

@@ -172,6 +172,18 @@ impl InvalidationMap {
         }
     }
 
+    /// Returns the number of dependencies stored in the invalidation map.
+    pub fn len(&self) -> usize {
+        self.state_affecting_selectors.len() +
+        self.other_attribute_affecting_selectors.len() +
+        self.id_to_selector.iter().fold(0, |accum, (_, ref v)| {
+            accum + v.len()
+        }) +
+        self.class_to_selector.iter().fold(0, |accum, (_, ref v)| {
+            accum + v.len()
+        })
+    }
+
     /// Adds a selector to this `InvalidationMap`.  Returns Err(..) to
     /// signify OOM.
     pub fn note_selector(
@@ -234,7 +246,8 @@ impl InvalidationMap {
 
             for class in compound_visitor.classes {
                 self.class_to_selector
-                    .try_get_or_insert_with(class, quirks_mode, SmallVec::new)?
+                    .entry(class, quirks_mode)
+                    .or_insert_with(SmallVec::new)
                     .try_push(Dependency {
                         selector: selector.clone(),
                         selector_offset: sequence_start,
@@ -243,7 +256,8 @@ impl InvalidationMap {
 
             for id in compound_visitor.ids {
                 self.id_to_selector
-                    .try_get_or_insert_with(id, quirks_mode, SmallVec::new)?
+                    .entry(id, quirks_mode)
+                    .or_insert_with(SmallVec::new)
                     .try_push(Dependency {
                         selector: selector.clone(),
                         selector_offset: sequence_start,
@@ -279,22 +293,6 @@ impl InvalidationMap {
 
         Ok(())
     }
-
-    /// Allows mutation of this InvalidationMap.
-    pub fn begin_mutation(&mut self) {
-        self.class_to_selector.begin_mutation();
-        self.id_to_selector.begin_mutation();
-        self.state_affecting_selectors.begin_mutation();
-        self.other_attribute_affecting_selectors.begin_mutation();
-    }
-
-    /// Disallows mutation of this InvalidationMap.
-    pub fn end_mutation(&mut self) {
-        self.class_to_selector.end_mutation();
-        self.id_to_selector.end_mutation();
-        self.state_affecting_selectors.end_mutation();
-        self.other_attribute_affecting_selectors.end_mutation();
-    }
 }
 
 /// A struct that collects invalidations for a given compound selector.

components/style/selector_map.rs

@@ -10,7 +10,8 @@ use applicable_declarations::ApplicableDeclarationBlock;
 use context::QuirksMode;
 use dom::TElement;
 use fallible::FallibleVec;
-use hash::{HashMap, HashSet, DiagnosticHashMap};
+use hash::{HashMap, HashSet};
+use hash::map as hash_map;
 use hashglobe::FailedAllocationError;
 use precomputed_hash::PrecomputedHash;
 use rule_tree::CascadeLevel;
@@ -36,9 +37,6 @@ impl Default for PrecomputedHasher {
 /// A simple alias for a hashmap using PrecomputedHasher.
 pub type PrecomputedHashMap<K, V> = HashMap<K, V, BuildHasherDefault<PrecomputedHasher>>;
 
-/// A simple alias for a hashmap using PrecomputedHasher.
-pub type PrecomputedDiagnosticHashMap<K, V> = DiagnosticHashMap<K, V, BuildHasherDefault<PrecomputedHasher>>;
-
 /// A simple alias for a hashset using PrecomputedHasher.
 pub type PrecomputedHashSet<K> = HashSet<K, BuildHasherDefault<PrecomputedHasher>>;
 
@@ -101,7 +99,7 @@ pub struct SelectorMap<T: 'static> {
     /// A hash from a class name to rules which contain that class selector.
     pub class_hash: MaybeCaseInsensitiveHashMap<Atom, SmallVec<[T; 1]>>,
     /// A hash from local name to rules which contain that local name selector.
-    pub local_name_hash: PrecomputedDiagnosticHashMap<LocalName, SmallVec<[T; 1]>>,
+    pub local_name_hash: PrecomputedHashMap<LocalName, SmallVec<[T; 1]>>,
     /// Rules that don't have ID, class, or element selectors.
     pub other: SmallVec<[T; 1]>,
     /// The number of entries in this map.
@@ -117,7 +115,7 @@ impl<T: 'static> SelectorMap<T> {
         SelectorMap {
             id_hash: MaybeCaseInsensitiveHashMap::new(),
             class_hash: MaybeCaseInsensitiveHashMap::new(),
-            local_name_hash: DiagnosticHashMap::default(),
+            local_name_hash: HashMap::default(),
             other: SmallVec::new(),
             count: 0,
         }
@@ -141,20 +139,6 @@ impl<T: 'static> SelectorMap<T> {
     pub fn len(&self) -> usize {
         self.count
     }
-
-    /// Allows mutation of this SelectorMap.
-    pub fn begin_mutation(&mut self) {
-        self.id_hash.begin_mutation();
-        self.class_hash.begin_mutation();
-        self.local_name_hash.begin_mutation();
-    }
-
-    /// Disallows mutation of this SelectorMap.
-    pub fn end_mutation(&mut self) {
-        self.id_hash.end_mutation();
-        self.class_hash.end_mutation();
-        self.local_name_hash.end_mutation();
-    }
 }
 
 impl SelectorMap<Rule> {
@@ -264,10 +248,12 @@ impl<T: SelectorMapEntry> SelectorMap<T> {
 
         let vector = match find_bucket(entry.selector()) {
             Bucket::ID(id) => {
-                self.id_hash.try_get_or_insert_with(id.clone(), quirks_mode, SmallVec::new)?
+                self.id_hash.try_entry(id.clone(), quirks_mode)?
+                    .or_insert_with(SmallVec::new)
             }
             Bucket::Class(class) => {
-                self.class_hash.try_get_or_insert_with(class.clone(), quirks_mode, SmallVec::new)?
+                self.class_hash.try_entry(class.clone(), quirks_mode)?
+                    .or_insert_with(SmallVec::new)
             }
             Bucket::LocalName { name, lower_name } => {
                 // If the local name in the selector isn't lowercase, insert it
@@ -283,10 +269,12 @@ impl<T: SelectorMapEntry> SelectorMap<T> {
                 // subsequent selector matching work will filter them out.
                 if name != lower_name {
                     self.local_name_hash
-                        .try_get_or_insert_with(lower_name.clone(), SmallVec::new)?
+                        .try_entry(lower_name.clone())?
+                        .or_insert_with(SmallVec::new)
                         .try_push(entry.clone())?;
                 }
-                self.local_name_hash.try_get_or_insert_with(name.clone(), SmallVec::new)?
+                self.local_name_hash.try_entry(name.clone())?
+                    .or_insert_with(SmallVec::new)
             }
             Bucket::Universal => {
                 &mut self.other
@@ -473,7 +461,7 @@ fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> {
 
 /// Wrapper for PrecomputedHashMap that does ASCII-case-insensitive lookup in quirks mode.
 #[derive(Debug, MallocSizeOf)]
-pub struct MaybeCaseInsensitiveHashMap<K: PrecomputedHash + Hash + Eq, V: 'static>(PrecomputedDiagnosticHashMap<K, V>);
+pub struct MaybeCaseInsensitiveHashMap<K: PrecomputedHash + Hash + Eq, V: 'static>(PrecomputedHashMap<K, V>);
 
 // FIXME(Manishearth) the 'static bound can be removed when
 // our HashMap fork (hashglobe) is able to use NonZero,
@@ -481,20 +469,32 @@ pub struct MaybeCaseInsensitiveHashMap<K: PrecomputedHash + Hash + Eq, V: 'stati
 impl<V: 'static> MaybeCaseInsensitiveHashMap<Atom, V> {
     /// Empty map
     pub fn new() -> Self {
-        MaybeCaseInsensitiveHashMap(PrecomputedDiagnosticHashMap::default())
+        MaybeCaseInsensitiveHashMap(PrecomputedHashMap::default())
     }
 
-    /// DiagnosticHashMap::try_get_or_insert_with
+    /// HashMap::entry
-    pub fn try_get_or_insert_with<F: FnOnce() -> V>(
+    pub fn entry(&mut self, mut key: Atom, quirks_mode: QuirksMode) -> hash_map::Entry<Atom, V> {
-        &mut self,
-        mut key: Atom,
-        quirks_mode: QuirksMode,
-        default: F,
-    ) -> Result<&mut V, FailedAllocationError> {
         if quirks_mode == QuirksMode::Quirks {
             key = key.to_ascii_lowercase()
         }
-        self.0.try_get_or_insert_with(key, default)
+        self.0.entry(key)
+    }
+
+    /// HashMap::try_entry
+    pub fn try_entry(
+        &mut self,
+        mut key: Atom,
+        quirks_mode: QuirksMode
+    ) -> Result<hash_map::Entry<Atom, V>, FailedAllocationError> {
+        if quirks_mode == QuirksMode::Quirks {
+            key = key.to_ascii_lowercase()
+        }
+        self.0.try_entry(key)
+    }
+
+    /// HashMap::iter
+    pub fn iter(&self) -> hash_map::Iter<Atom, V> {
+        self.0.iter()
     }
 
     /// HashMap::clear
@@ -510,14 +510,4 @@ impl<V: 'static> MaybeCaseInsensitiveHashMap<Atom, V> {
             self.0.get(key)
         }
     }
-
-    /// DiagnosticHashMap::begin_mutation
-    pub fn begin_mutation(&mut self) {
-        self.0.begin_mutation();
-    }
-
-    /// DiagnosticHashMap::end_mutation
-    pub fn end_mutation(&mut self) {
-        self.0.end_mutation();
-    }
 }

components/style/selector_parser.rs

@@ -145,15 +145,6 @@ impl<T> PerPseudoElementMap<T> {
         *self = Self::default();
     }
 
-    /// Invokes a callback on each non-None entry.
-    pub fn for_each<F: FnMut(&mut T)>(&mut self, mut f: F) {
-        for entry in self.entries.iter_mut() {
-            if entry.is_some() {
-                f(entry.as_mut().unwrap());
-            }
-        }
-    }
-
     /// Set an entry value.
     ///
     /// Returns an error if the element is not a simple pseudo.

components/style/stylist.rs

@@ -479,6 +479,12 @@ impl Stylist {
             .map(|(d, _)| d.selectors_for_cache_revalidation.len()).sum()
     }
 
+    /// Returns the number of entries in invalidation maps.
+    pub fn num_invalidations(&self) -> usize {
+        self.cascade_data.iter_origins()
+            .map(|(d, _)| d.invalidation_map.len()).sum()
+    }
+
     /// Invokes `f` with the `InvalidationMap` for each origin.
     ///
     /// NOTE(heycam) This might be better as an `iter_invalidation_maps`, once
@@ -1906,32 +1912,6 @@ impl CascadeData {
         }
     }
 
-    #[cfg(feature = "gecko")]
-    fn begin_mutation(&mut self, rebuild_kind: &SheetRebuildKind) {
-        self.element_map.begin_mutation();
-        self.pseudos_map.for_each(|m| m.begin_mutation());
-        if rebuild_kind.should_rebuild_invalidation() {
-            self.invalidation_map.begin_mutation();
-            self.selectors_for_cache_revalidation.begin_mutation();
-        }
-    }
-
-    #[cfg(feature = "servo")]
-    fn begin_mutation(&mut self, _: &SheetRebuildKind) {}
-
-    #[cfg(feature = "gecko")]
-    fn end_mutation(&mut self, rebuild_kind: &SheetRebuildKind) {
-        self.element_map.end_mutation();
-        self.pseudos_map.for_each(|m| m.end_mutation());
-        if rebuild_kind.should_rebuild_invalidation() {
-            self.invalidation_map.end_mutation();
-            self.selectors_for_cache_revalidation.end_mutation();
-        }
-    }
-
-    #[cfg(feature = "servo")]
-    fn end_mutation(&mut self, _: &SheetRebuildKind) {}
-
     /// Collects all the applicable media query results into `results`.
     ///
     /// This duplicates part of the logic in `add_stylesheet`, which is
@@ -1995,7 +1975,6 @@ impl CascadeData {
             self.effective_media_query_results.saw_effective(stylesheet);
         }
 
-        self.begin_mutation(&rebuild_kind);
         for rule in stylesheet.effective_rules(device, guard) {
             match *rule {
                 CssRule::Style(ref locked) => {
@@ -2026,11 +2005,7 @@ impl CascadeData {
                             None => &mut self.element_map,
                             Some(pseudo) => {
                                 self.pseudos_map
-                                    .get_or_insert_with(&pseudo.canonical(), || {
+                                    .get_or_insert_with(&pseudo.canonical(), || Box::new(SelectorMap::new()))
-                                        let mut map = Box::new(SelectorMap::new());
-                                        map.begin_mutation();
-                                        map
-                                    })
                             }
                         };
 
@@ -2125,7 +2100,6 @@ impl CascadeData {
                 _ => {}
             }
         }
-        self.end_mutation(&rebuild_kind);
 
         Ok(())
     }