Diagnostic map semantics.

MozReview-Commit-ID: C0a5g6xMPY0

components/hashglobe/src/diagnostic.rs

@@ -0,0 +1,143 @@
+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

@@ -77,10 +77,24 @@ 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

@@ -11,6 +11,7 @@
 extern crate heapsize;
 
 pub mod alloc;
+pub mod diagnostic;
 pub mod hash_map;
 pub mod hash_set;
 mod shim;

components/malloc_size_of/lib.rs

@@ -339,6 +339,25 @@ 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)
+    }
+}
+
 // XXX: we don't want MallocSizeOf to be defined for Rc and Arc. If negative
 // trait bounds are ever allowed, this code should be uncommented.
 // (We do have a compile-fail test for this:

components/style/context.rs

@@ -322,8 +322,6 @@ 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.
@@ -344,7 +342,6 @@ 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 {
@@ -355,7 +352,6 @@ 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,
@@ -383,7 +379,6 @@ 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)?;
@@ -405,7 +400,6 @@ 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

@@ -11,7 +11,7 @@ use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSeri
 use parser::ParserContext;
 use precomputed_hash::PrecomputedHash;
 use properties::{CSSWideKeyword, DeclaredValue};
-use selector_map::{PrecomputedHashSet, PrecomputedHashMap};
+use selector_map::{PrecomputedHashSet, PrecomputedDiagnosticHashMap};
 use selectors::parser::SelectorParseError;
 use servo_arc::Arc;
 use std::ascii::AsciiExt;
@@ -105,7 +105,7 @@ where
     /// Key index.
     index: Vec<K>,
     /// Key-value map.
-    values: PrecomputedHashMap<K, V>,
+    values: PrecomputedDiagnosticHashMap<K, V>,
 }
 
 impl<K, V> OrderedMap<K, V>
@@ -116,7 +116,7 @@ where
     pub fn new() -> Self {
         OrderedMap {
             index: Vec::new(),
-            values: PrecomputedHashMap::default(),
+            values: PrecomputedDiagnosticHashMap::default(),
         }
     }
 
@@ -125,7 +125,9 @@ where
         if !self.values.contains_key(&key) {
             self.index.push(key.clone());
         }
-        self.values.insert(key, value);
+        self.values.begin_mutation();
+        self.values.try_insert(key, value).unwrap();
+        self.values.end_mutation();
     }
 
     /// Get a value given its key.
@@ -165,7 +167,10 @@ where
             None => return None,
         };
         self.index.remove(index);
-        self.values.remove(key)
+        self.values.begin_mutation();
+        let result = self.values.remove(key);
+        self.values.end_mutation();
+        result
     }
 }
 
@@ -196,7 +201,7 @@ where
         };
 
         self.pos += 1;
-        let value = &self.inner.values[key];
+        let value = &self.inner.values.get(key).unwrap();
 
         Some((key, value))
     }

components/style/hash.rs

@@ -13,11 +13,16 @@ 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

@@ -178,18 +178,6 @@ 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(
@@ -252,8 +240,7 @@ impl InvalidationMap {
 
             for class in compound_visitor.classes {
                 self.class_to_selector
-                    .entry(class, quirks_mode)
-                    .or_insert_with(SmallVec::new)
+                    .try_get_or_insert_with(class, quirks_mode, SmallVec::new)?
                     .try_push(Dependency {
                         selector: selector.clone(),
                         selector_offset: sequence_start,
@@ -262,8 +249,7 @@ impl InvalidationMap {
 
             for id in compound_visitor.ids {
                 self.id_to_selector
-                    .entry(id, quirks_mode)
-                    .or_insert_with(SmallVec::new)
+                    .try_get_or_insert_with(id, quirks_mode, SmallVec::new)?
                     .try_push(Dependency {
                         selector: selector.clone(),
                         selector_offset: sequence_start,
@@ -299,6 +285,22 @@ 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.