Semantics for ProtectedHashMap.

MozReview-Commit-ID: K0m65uZi7iw

Cargo.lock

@@ -1061,6 +1061,7 @@ dependencies = [
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "cssparser 0.21.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "hashglobe 0.1.0",
  "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "malloc_size_of 0.0.1",

components/hashglobe/src/lib.rs

@@ -13,6 +13,7 @@ extern crate heapsize;
 mod alloc;
 pub mod hash_map;
 pub mod hash_set;
+pub mod protected;
 mod shim;
 mod table;
 

components/hashglobe/src/protected.rs

@@ -0,0 +1,180 @@
+use hash_map::{Entry, HashMap, Iter, IterMut, Keys, RandomState, Values};
+use std::borrow::Borrow;
+use std::hash::{BuildHasher, Hash};
+
+use FailedAllocationError;
+
+#[derive(Clone, Debug)]
+pub struct ProtectedHashMap<K, V, S = RandomState>
+    where K: Eq + Hash,
+          S: BuildHasher
+{
+    map: HashMap<K, V, S>,
+    readonly: bool,
+}
+
+impl<K: Hash + Eq, V, S: BuildHasher> ProtectedHashMap<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 keys(&self) -> Keys<K, V> {
+        self.map.keys()
+    }
+
+    #[inline(always)]
+    pub fn values(&self) -> Values<K, V> {
+        self.map.values()
+    }
+
+    #[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 iter(&self) -> Iter<K, V> {
+        self.map.iter()
+    }
+
+    #[inline(always)]
+    pub fn iter_mut(&mut self) -> IterMut<K, V> {
+        assert!(!self.readonly);
+        self.map.iter_mut()
+    }
+
+    #[inline(always)]
+    pub fn entry(&mut self, key: K) -> Entry<K, V> {
+        assert!(!self.readonly);
+        self.map.entry(key)
+    }
+
+    #[inline(always)]
+    pub fn try_entry(&mut self, key: K) -> Result<Entry<K, V>, FailedAllocationError> {
+        assert!(!self.readonly);
+        self.map.try_entry(key)
+    }
+
+    #[inline(always)]
+    pub fn insert(&mut self, k: K, v: V) -> Option<V> {
+        assert!(!self.readonly);
+        self.map.insert(k, v)
+    }
+
+    #[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> ProtectedHashMap<K, V, RandomState>
+    where K: Eq + Hash,
+{
+    pub fn new() -> Self {
+        Self {
+            map: HashMap::new(),
+            readonly: true,
+        }
+    }
+
+    pub fn with_capacity(capacity: usize) -> Self {
+        Self {
+            map: HashMap::with_capacity(capacity),
+            readonly: true,
+        }
+    }
+}
+
+impl<K, V, S> PartialEq for ProtectedHashMap<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 ProtectedHashMap<K, V, S>
+    where K: Eq + Hash,
+          V: Eq,
+          S: BuildHasher
+{
+}
+
+impl<K, V, S> Default for ProtectedHashMap<K, V, S>
+    where K: Eq + Hash,
+          S: BuildHasher + Default
+{
+    fn default() -> Self {
+        Self {
+            map: HashMap::default(),
+            readonly: true,
+        }
+    }
+}

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::protected::ProtectedHashMap<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::protected::ProtectedHashMap<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/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::protected::ProtectedHashMap;
 
 #[cfg(feature = "servo")]
 pub use hashglobe::fake::{HashMap, HashSet};
 
+/// Alias to use regular HashMaps everywhere in Servo.
+#[cfg(feature = "servo")]
+pub type ProtectedHashMap<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

@@ -299,6 +299,24 @@ impl InvalidationMap {
 
         Ok(())
     }
+
+    /// Allows mutation of this InvalidationMap.
+    #[cfg(feature = "gecko")]
+    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.
+    #[cfg(feature = "gecko")]
+    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,7 @@ use applicable_declarations::ApplicableDeclarationBlock;
 use context::QuirksMode;
 use dom::TElement;
 use fallible::FallibleVec;
-use hash::{HashMap, HashSet};
+use hash::{HashMap, HashSet, ProtectedHashMap};
 use hash::map as hash_map;
 use hashglobe::FailedAllocationError;
 use pdqsort::sort_by;
@@ -38,6 +38,9 @@ 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 PrecomputedProtectedHashMap<K, V> = ProtectedHashMap<K, V, BuildHasherDefault<PrecomputedHasher>>;
+
 /// A simple alias for a hashset using PrecomputedHasher.
 pub type PrecomputedHashSet<K> = HashSet<K, BuildHasherDefault<PrecomputedHasher>>;
 
@@ -102,7 +105,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: PrecomputedHashMap<LocalName, SmallVec<[T; 1]>>,
+    pub local_name_hash: PrecomputedProtectedHashMap<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.
@@ -123,7 +126,7 @@ impl<T: 'static> SelectorMap<T> {
         SelectorMap {
             id_hash: MaybeCaseInsensitiveHashMap::new(),
             class_hash: MaybeCaseInsensitiveHashMap::new(),
-            local_name_hash: HashMap::default(),
+            local_name_hash: ProtectedHashMap::default(),
             other: SmallVec::new(),
             count: 0,
         }
@@ -147,6 +150,30 @@ impl<T: 'static> SelectorMap<T> {
     pub fn len(&self) -> usize {
         self.count
     }
+
+    /// Allows mutation of this SelectorMap.
+    #[cfg(feature = "gecko")]
+    pub fn begin_mutation(&mut self) {
+        self.id_hash.begin_mutation();
+        self.class_hash.begin_mutation();
+        self.local_name_hash.begin_mutation();
+    }
+
+    /// Allows mutation of this SelectorMap. Not enforced in Servo.
+    #[cfg(feature = "servo")]
+    pub fn begin_mutation(&mut self) {}
+
+    /// Disallows mutation of this SelectorMap.
+    #[cfg(feature = "gecko")]
+    pub fn end_mutation(&mut self) {
+        self.id_hash.end_mutation();
+        self.class_hash.end_mutation();
+        self.local_name_hash.end_mutation();
+    }
+
+    /// Disallows mutation of this SelectorMap. Not enforced in Servo.
+    #[cfg(feature = "servo")]
+    pub fn end_mutation(&mut self) {}
 }
 
 impl SelectorMap<Rule> {
@@ -463,7 +490,7 @@ fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> {
 #[derive(Debug)]
 #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-pub struct MaybeCaseInsensitiveHashMap<K: PrecomputedHash + Hash + Eq, V: 'static>(PrecomputedHashMap<K, V>);
+pub struct MaybeCaseInsensitiveHashMap<K: PrecomputedHash + Hash + Eq, V: 'static>(PrecomputedProtectedHashMap<K, V>);
 
 // FIXME(Manishearth) the 'static bound can be removed when
 // our HashMap fork (hashglobe) is able to use NonZero,
@@ -471,7 +498,7 @@ pub struct MaybeCaseInsensitiveHashMap<K: PrecomputedHash + Hash + Eq, V: 'stati
 impl<V: 'static> MaybeCaseInsensitiveHashMap<Atom, V> {
     /// Empty map
     pub fn new() -> Self {
-        MaybeCaseInsensitiveHashMap(PrecomputedHashMap::default())
+        MaybeCaseInsensitiveHashMap(PrecomputedProtectedHashMap::default())
     }
 
     /// HashMap::entry
@@ -512,5 +539,17 @@ impl<V: 'static> MaybeCaseInsensitiveHashMap<Atom, V> {
             self.0.get(key)
         }
     }
+
+    /// ProtectedHashMap::begin_mutation
+    #[cfg(feature = "gecko")]
+    pub fn begin_mutation(&mut self) {
+        self.0.begin_mutation();
+    }
+
+    /// ProtectedHashMap::end_mutation
+    #[cfg(feature = "gecko")]
+    pub fn end_mutation(&mut self) {
+        self.0.end_mutation();
+    }
 }
 

components/style/selector_parser.rs

@@ -159,6 +159,15 @@ 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

@@ -1870,6 +1870,32 @@ 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
@@ -1933,6 +1959,7 @@ 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) => {
@@ -1969,8 +1996,11 @@ impl CascadeData {
                             None => &mut self.element_map,
                             Some(pseudo) => {
                                 self.pseudos_map
-                                    .get_or_insert_with(&pseudo.canonical(), || Box::new(SelectorMap::new()))
+                                    .get_or_insert_with(&pseudo.canonical(), || {
-                                    .expect("Unexpected tree pseudo-element?")
+                                        let mut map = Box::new(SelectorMap::new());
+                                        map.begin_mutation();
+                                        map
+                                    }).expect("Unexpected tree pseudo-element?")
                             }
                         };
 
@@ -2064,6 +2094,7 @@ impl CascadeData {
                 _ => {}
             }
         }
+        self.end_mutation(&rebuild_kind);
 
         Ok(())
     }