diff --git a/Cargo.lock b/Cargo.lock index 5b9c3ec5bed..7e6808bbfcf 100644 --- a/Cargo.lock +++ b/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", diff --git a/components/hashglobe/src/lib.rs b/components/hashglobe/src/lib.rs index cbbdacb4c8d..8a2d070e0e2 100644 --- a/components/hashglobe/src/lib.rs +++ b/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; diff --git a/components/hashglobe/src/protected.rs b/components/hashglobe/src/protected.rs new file mode 100644 index 00000000000..99b0879c276 --- /dev/null +++ b/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 + where K: Eq + Hash, + S: BuildHasher +{ + map: HashMap, + readonly: bool, +} + +impl ProtectedHashMap + where K: Eq + Hash, + S: BuildHasher +{ + #[inline(always)] + pub fn inner(&self) -> &HashMap { + &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::::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(&self, k: &Q) -> bool + where K: Borrow, + Q: Hash + Eq + { + self.map.contains_key(k) + } + + #[inline(always)] + pub fn keys(&self) -> Keys { + self.map.keys() + } + + #[inline(always)] + pub fn values(&self) -> Values { + self.map.values() + } + + #[inline(always)] + pub fn get(&self, k: &Q) -> Option<&V> + where K: Borrow, + Q: Hash + Eq + { + self.map.get(k) + } + + #[inline(always)] + pub fn iter(&self) -> Iter { + self.map.iter() + } + + #[inline(always)] + pub fn iter_mut(&mut self) -> IterMut { + assert!(!self.readonly); + self.map.iter_mut() + } + + #[inline(always)] + pub fn entry(&mut self, key: K) -> Entry { + assert!(!self.readonly); + self.map.entry(key) + } + + #[inline(always)] + pub fn try_entry(&mut self, key: K) -> Result, FailedAllocationError> { + assert!(!self.readonly); + self.map.try_entry(key) + } + + #[inline(always)] + pub fn insert(&mut self, k: K, v: V) -> Option { + assert!(!self.readonly); + self.map.insert(k, v) + } + + #[inline(always)] + pub fn try_insert(&mut self, k: K, v: V) -> Result, FailedAllocationError> { + assert!(!self.readonly); + self.map.try_insert(k, v) + } + + #[inline(always)] + pub fn remove(&mut self, k: &Q) -> Option + where K: Borrow, + 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 ProtectedHashMap + 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 PartialEq for ProtectedHashMap + where K: Eq + Hash, + V: PartialEq, + S: BuildHasher +{ + fn eq(&self, other: &Self) -> bool { + self.map.eq(&other.map) + } +} + +impl Eq for ProtectedHashMap + where K: Eq + Hash, + V: Eq, + S: BuildHasher +{ +} + +impl Default for ProtectedHashMap + where K: Eq + Hash, + S: BuildHasher + Default +{ + fn default() -> Self { + Self { + map: HashMap::default(), + readonly: true, + } + } +} diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index 413942e4322..fc27de0320a 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -339,6 +339,25 @@ impl MallocSizeOf for hashglobe::hash_map::HashMap } } +impl MallocShallowSizeOf for hashglobe::protected::ProtectedHashMap + where K: Eq + Hash, + S: BuildHasher +{ + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + self.inner().shallow_size_of(ops) + } +} + +impl MallocSizeOf for hashglobe::protected::ProtectedHashMap + 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: diff --git a/components/style/hash.rs b/components/style/hash.rs index 305357a85b2..89b7e1979a2 100644 --- a/components/style/hash.rs +++ b/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 = HashMap; + /// Appropriate reexports of hash_map types pub mod map { #[cfg(feature = "gecko")] diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index 301cd2e25a9..2531e0f8f23 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/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. diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 3b14431c06f..b151a36632b 100644 --- a/components/style/selector_map.rs +++ b/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 = HashMap>; +/// A simple alias for a hashmap using PrecomputedHasher. +pub type PrecomputedProtectedHashMap = ProtectedHashMap>; + /// A simple alias for a hashset using PrecomputedHasher. pub type PrecomputedHashSet = HashSet>; @@ -102,7 +105,7 @@ pub struct SelectorMap { /// A hash from a class name to rules which contain that class selector. pub class_hash: MaybeCaseInsensitiveHashMap>, /// A hash from local name to rules which contain that local name selector. - pub local_name_hash: PrecomputedHashMap>, + pub local_name_hash: PrecomputedProtectedHashMap>, /// 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 SelectorMap { 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 SelectorMap { 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 { @@ -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(PrecomputedHashMap); +pub struct MaybeCaseInsensitiveHashMap(PrecomputedProtectedHashMap); // 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 MaybeCaseInsensitiveHashMap { /// Empty map pub fn new() -> Self { - MaybeCaseInsensitiveHashMap(PrecomputedHashMap::default()) + MaybeCaseInsensitiveHashMap(PrecomputedProtectedHashMap::default()) } /// HashMap::entry @@ -512,5 +539,17 @@ impl MaybeCaseInsensitiveHashMap { 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(); + } } diff --git a/components/style/selector_parser.rs b/components/style/selector_parser.rs index f55161a9cc4..316f360daf8 100644 --- a/components/style/selector_parser.rs +++ b/components/style/selector_parser.rs @@ -159,6 +159,15 @@ impl PerPseudoElementMap { *self = Self::default(); } + /// Invokes a callback on each non-None entry. + pub fn for_each(&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. diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 7bf735955b9..0b0b020cf19 100644 --- a/components/style/stylist.rs +++ b/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())) - .expect("Unexpected tree pseudo-element?") + .get_or_insert_with(&pseudo.canonical(), || { + 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(()) }