From a2c2d347530c212ad8c906b4b987b37279abce44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 31 Jan 2018 15:13:02 +0100 Subject: [PATCH 1/2] hashglobe: Add a shim on top of OrderMap. --- Cargo.lock | 8 + components/hashglobe/Cargo.toml | 1 + components/hashglobe/src/lib.rs | 3 + components/hashglobe/src/order.rs | 246 +++++++++++++++++++++++++++ components/malloc_size_of/Cargo.toml | 1 + components/malloc_size_of/lib.rs | 98 +++++++++++ 6 files changed, 357 insertions(+) create mode 100644 components/hashglobe/src/order.rs diff --git a/Cargo.lock b/Cargo.lock index 8e5907fe7f4d..fafc72c3b278 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1139,6 +1139,7 @@ name = "hashglobe" version = "0.1.0" dependencies = [ "libc 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)", + "ordermap 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1608,6 +1609,7 @@ dependencies = [ "euclid 0.16.0 (registry+https://github.com/rust-lang/crates.io-index)", "hashglobe 0.1.0", "mozjs 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)", + "ordermap 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", "selectors 0.19.0", "servo_arc 0.1.0", "smallbitvec 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2061,6 +2063,11 @@ dependencies = [ "unreachable 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "ordermap" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "osmesa-src" version = "17.3.1-devel" @@ -3700,6 +3707,7 @@ dependencies = [ "checksum openssl 0.9.22 (registry+https://github.com/rust-lang/crates.io-index)" = "419ef26bb651d72b6c5a603bcc4e4856a362460e62352dfffa53de91d2e81181" "checksum openssl-sys 0.9.22 (registry+https://github.com/rust-lang/crates.io-index)" = "5483bdc56756041ba6aa37c9cb59cc2219f012a2a1377d97ad35556ac6676ee7" "checksum ordered-float 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "da12c96037889ae0be29dd2bdd260e5a62a7df24e6466d5a15bb8131c1c200a8" +"checksum ordermap 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)" = "a86ed3f5f244b372d6b1a00b72ef7f8876d0bc6a78a4c9985c53614041512063" "checksum osmesa-src 17.3.1-devel (git+https://github.com/servo/osmesa-src)" = "" "checksum osmesa-sys 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "88cfece6e95d2e717e0872a7f53a8684712ad13822a7979bc760b9c77ec0013b" "checksum ovr-mobile-sys 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b7b5f9389b2015f8340f0566c488f3e96735e2e8fd7b85d571832cd274ac2998" diff --git a/components/hashglobe/Cargo.toml b/components/hashglobe/Cargo.toml index 6deb04a75311..6122b24ce809 100644 --- a/components/hashglobe/Cargo.toml +++ b/components/hashglobe/Cargo.toml @@ -11,6 +11,7 @@ readme = "README.md" [dependencies] libc = "0.2" +ordermap = "0.3" [dev-dependencies] rand = "0.3" diff --git a/components/hashglobe/src/lib.rs b/components/hashglobe/src/lib.rs index 49038a51859d..22947bdff53c 100644 --- a/components/hashglobe/src/lib.rs +++ b/components/hashglobe/src/lib.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +extern crate ordermap; + pub mod alloc; pub mod hash_map; pub mod hash_set; @@ -15,6 +17,7 @@ mod shim; mod table; pub mod fake; +pub mod order; use std::{error, fmt}; diff --git a/components/hashglobe/src/order.rs b/components/hashglobe/src/order.rs new file mode 100644 index 000000000000..eb75272164f4 --- /dev/null +++ b/components/hashglobe/src/order.rs @@ -0,0 +1,246 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! This module contains shims around the ordermap crate that add fallible +//! methods. +//! +//! These methods are a lie. They are not actually fallible. This is just to +//! make it smooth to switch between hashmap impls the codebase. + +use ordermap::{OrderMap, OrderSet}; +use std::fmt; +use std::hash::{BuildHasher, Hash}; +use std::ops::{Deref, DerefMut}; + +pub use std::collections::hash_map::RandomState; +pub use ordermap::{Entry, Iter as MapIter, IterMut as MapIterMut}; +pub use ordermap::set::{Iter as SetIter, IntoIter as SetIntoIter}; + +#[derive(Clone)] +pub struct HashMap(OrderMap); + + +use FailedAllocationError; + +impl Deref for HashMap { + type Target = OrderMap; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for HashMap { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl HashMap + where K: Eq + Hash, + S: BuildHasher +{ + #[inline] + pub fn try_with_hasher(hash_builder: S) -> Result, FailedAllocationError> { + Ok(HashMap(OrderMap::with_hasher(hash_builder))) + } + + #[inline] + pub fn try_with_capacity_and_hasher(capacity: usize, + hash_builder: S) + -> Result, FailedAllocationError> { + Ok(HashMap(OrderMap::with_capacity_and_hasher(capacity, hash_builder))) + } + + pub fn with_capacity_and_hasher(capacity: usize, hash_builder: S) -> HashMap { + HashMap(OrderMap::with_capacity_and_hasher(capacity, hash_builder)) + } + + + #[inline] + pub fn try_reserve(&mut self, additional: usize) -> Result<(), FailedAllocationError> { + Ok(self.reserve(additional)) + } + + #[inline] + pub fn try_entry(&mut self, key: K) -> Result, FailedAllocationError> { + Ok(self.entry(key)) + } + + #[inline] + pub fn try_insert(&mut self, k: K, v: V) -> Result, FailedAllocationError> { + Ok(self.insert(k, v)) + } +} + +#[derive(Clone)] +pub struct HashSet(OrderSet); + + +impl Deref for HashSet { + type Target = OrderSet; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for HashSet { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl HashSet { + #[inline] + pub fn new() -> HashSet { + HashSet(OrderSet::new()) + } + + #[inline] + pub fn with_capacity(capacity: usize) -> HashSet { + HashSet(OrderSet::with_capacity(capacity)) + } +} + + +impl HashSet + where T: Eq + Hash, + S: BuildHasher +{ + #[inline] + pub fn with_hasher(hasher: S) -> HashSet { + HashSet(OrderSet::with_hasher(hasher)) + } + + + #[inline] + pub fn with_capacity_and_hasher(capacity: usize, hasher: S) -> HashSet { + HashSet(OrderSet::with_capacity_and_hasher(capacity, hasher)) + } + + #[inline] + pub fn try_reserve(&mut self, additional: usize) -> Result<(), FailedAllocationError> { + Ok(self.reserve(additional)) + } + + #[inline] + pub fn try_insert(&mut self, value: T) -> Result { + Ok(self.insert(value)) + } +} + +// Pass through trait impls +// We can't derive these since the bounds are not obvious to the derive macro + +impl Default for HashMap { + fn default() -> Self { + HashMap(Default::default()) + } +} + +impl fmt::Debug for HashMap + where K: Eq + Hash + fmt::Debug, + V: fmt::Debug, + S: BuildHasher { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt(f) + } +} + +impl PartialEq for HashMap + where K: Eq + Hash, + V: PartialEq, + S: BuildHasher +{ + fn eq(&self, other: &HashMap) -> bool { + self.0.eq(&other.0) + } +} + +impl Eq for HashMap + where K: Eq + Hash, + V: Eq, + S: BuildHasher +{ +} + +impl<'a, K, V, S> IntoIterator for &'a HashMap + where K: Eq + Hash, + S: BuildHasher +{ + type Item = (&'a K, &'a V); + type IntoIter = MapIter<'a, K, V>; + + fn into_iter(self) -> MapIter<'a, K, V> { + self.0.iter() + } +} + +impl<'a, K, V, S> IntoIterator for &'a mut HashMap + where K: Eq + Hash, + S: BuildHasher +{ + type Item = (&'a K, &'a mut V); + type IntoIter = MapIterMut<'a, K, V>; + + fn into_iter(self) -> MapIterMut<'a, K, V> { + self.0.iter_mut() + } +} + +impl Default for HashSet { + fn default() -> Self { + HashSet(Default::default()) + } +} + +impl fmt::Debug for HashSet + where T: Eq + Hash + fmt::Debug, + S: BuildHasher +{ + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt(f) + } +} + +impl PartialEq for HashSet + where T: Eq + Hash, + S: BuildHasher +{ + fn eq(&self, other: &HashSet) -> bool { + self.0.eq(&other.0) + } +} + +impl Eq for HashSet + where T: Eq + Hash, + S: BuildHasher +{ +} + +impl<'a, T, S> IntoIterator for &'a HashSet + where T: Eq + Hash, + S: BuildHasher +{ + type Item = &'a T; + type IntoIter = SetIter<'a, T>; + + fn into_iter(self) -> SetIter<'a, T> { + self.0.iter() + } +} + +impl IntoIterator for HashSet + where T: Eq + Hash, + S: BuildHasher +{ + type Item = T; + type IntoIter = SetIntoIter; + + + fn into_iter(self) -> SetIntoIter { + self.0.into_iter() + } +} + + diff --git a/components/malloc_size_of/Cargo.toml b/components/malloc_size_of/Cargo.toml index 9b24d6dade8e..9ff69d28945e 100644 --- a/components/malloc_size_of/Cargo.toml +++ b/components/malloc_size_of/Cargo.toml @@ -17,6 +17,7 @@ cssparser = "0.23.0" euclid = "0.16" hashglobe = { path = "../hashglobe" } mozjs = { version = "0.1.8", features = ["promises"], optional = true } +ordermap = "0.3" selectors = { path = "../selectors" } servo_arc = { path = "../servo_arc" } smallbitvec = "1.0.3" diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index 9609f9890551..f76ae42c8fa8 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -49,6 +49,7 @@ extern crate euclid; extern crate hashglobe; #[cfg(feature = "servo")] extern crate mozjs as js; +extern crate ordermap; extern crate selectors; extern crate servo_arc; extern crate smallbitvec; @@ -409,6 +410,33 @@ impl MallocShallowSizeOf for hashglobe::hash_set::HashSet } } +impl MallocSizeOf for ordermap::OrderSet + where T: Eq + Hash + MallocSizeOf, + S: BuildHasher, +{ + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + let mut n = self.shallow_size_of(ops); + for t in self.iter() { + n += t.size_of(ops); + } + n + } +} + +impl MallocShallowSizeOf for ordermap::OrderSet + where T: Eq + Hash, + S: BuildHasher +{ + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + // See the implementation for std::collections::HashSet for details. + if ops.has_malloc_enclosing_size_of() { + self.iter().next().map_or(0, |t| unsafe { ops.malloc_enclosing_size_of(t) }) + } else { + self.capacity() * (size_of::() + size_of::()) + } + } +} + impl MallocSizeOf for hashglobe::hash_set::HashSet where T: Eq + Hash + MallocSizeOf, S: BuildHasher, @@ -442,6 +470,26 @@ impl MallocSizeOf for hashglobe::fake::HashSet } } +impl MallocShallowSizeOf for hashglobe::order::HashSet + where T: Eq + Hash, + S: BuildHasher, +{ + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + use std::ops::Deref; + self.deref().shallow_size_of(ops) + } +} + +impl MallocSizeOf for hashglobe::order::HashSet + where T: Eq + Hash + MallocSizeOf, + S: BuildHasher, +{ + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + use std::ops::Deref; + self.deref().size_of(ops) + } +} + impl MallocShallowSizeOf for std::collections::HashMap where K: Eq + Hash, S: BuildHasher @@ -471,6 +519,35 @@ impl MallocSizeOf for std::collections::HashMap } } +impl MallocShallowSizeOf for ordermap::OrderMap + where K: Eq + Hash, + S: BuildHasher +{ + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + // See the implementation for std::collections::HashSet for details. + if ops.has_malloc_enclosing_size_of() { + self.values().next().map_or(0, |v| unsafe { ops.malloc_enclosing_size_of(v) }) + } else { + self.capacity() * (size_of::() + size_of::() + size_of::()) + } + } +} + +impl MallocSizeOf for ordermap::OrderMap + where K: Eq + Hash + MallocSizeOf, + V: MallocSizeOf, + S: BuildHasher, +{ + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + let mut n = self.shallow_size_of(ops); + for (k, v) in self.iter() { + n += k.size_of(ops); + n += v.size_of(ops); + } + n + } +} + impl MallocShallowSizeOf for hashglobe::hash_map::HashMap where K: Eq + Hash, S: BuildHasher @@ -521,6 +598,27 @@ impl MallocSizeOf for hashglobe::fake::HashMap } } +impl MallocShallowSizeOf for hashglobe::order::HashMap + where K: Eq + Hash, + S: BuildHasher, +{ + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + use std::ops::Deref; + self.deref().shallow_size_of(ops) + } +} + +impl MallocSizeOf for hashglobe::order::HashMap + where K: Eq + Hash + MallocSizeOf, + V: MallocSizeOf, + S: BuildHasher, +{ + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + use std::ops::Deref; + self.deref().size_of(ops) + } +} + // PhantomData is always 0. impl MallocSizeOf for std::marker::PhantomData { fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { From 4d9ce6b8805dc286861ce5c35b3d76c1ed0dcf03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 31 Jan 2018 15:13:15 +0100 Subject: [PATCH 2/2] style: Temporarily use OrderMap on Gecko. This will allow us to discard std hash map as a source of crashes. --- components/style/hash.rs | 13 ++++++++++--- .../invalidation/element/invalidation_map.rs | 4 ++-- components/style/selector_map.rs | 19 ++++++++++++++----- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/components/style/hash.rs b/components/style/hash.rs index 305357a85b23..3a2d95404a18 100644 --- a/components/style/hash.rs +++ b/components/style/hash.rs @@ -9,19 +9,26 @@ use fnv; +// #[cfg(feature = "gecko")] +// pub use hashglobe::hash_map::HashMap; +// #[cfg(feature = "gecko")] +// pub use hashglobe::hash_set::HashSet; #[cfg(feature = "gecko")] -pub use hashglobe::hash_map::HashMap; +pub use hashglobe::order::HashMap; #[cfg(feature = "gecko")] -pub use hashglobe::hash_set::HashSet; +pub use hashglobe::order::HashSet; #[cfg(feature = "servo")] pub use hashglobe::fake::{HashMap, HashSet}; + /// Appropriate reexports of hash_map types pub mod map { + // #[cfg(feature = "gecko")] + // pub use hashglobe::hash_map::{Entry, Iter}; #[cfg(feature = "gecko")] - pub use hashglobe::hash_map::{Entry, Iter}; + pub use hashglobe::order::{Entry, MapIter as Iter}; #[cfg(feature = "servo")] pub use std::collections::hash_map::{Entry, Iter}; } diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index 854d01896104..05c67bcc3834 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -273,7 +273,7 @@ impl InvalidationMap { for class in compound_visitor.classes { self.class_to_selector - .entry(class, quirks_mode) + .try_entry(class, quirks_mode)? .or_insert_with(SmallVec::new) .try_push(Dependency { selector: selector.clone(), @@ -283,7 +283,7 @@ impl InvalidationMap { for id in compound_visitor.ids { self.id_to_selector - .entry(id, quirks_mode) + .try_entry(id, quirks_mode)? .or_insert_with(SmallVec::new) .try_push(Dependency { selector: selector.clone(), diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 0678f0ec9221..e9ed6ce3536b 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -512,20 +512,29 @@ impl MaybeCaseInsensitiveHashMap { MaybeCaseInsensitiveHashMap(PrecomputedHashMap::default()) } - /// HashMap::entry - pub fn entry(&mut self, mut key: Atom, quirks_mode: QuirksMode) -> hash_map::Entry { + /// HashMap::try_entry + #[cfg(not(feature = "gecko"))] + pub fn try_entry( + &mut self, + mut key: Atom, + quirks_mode: QuirksMode, + ) -> Result, FailedAllocationError> { if quirks_mode == QuirksMode::Quirks { key = key.to_ascii_lowercase() } - self.0.entry(key) + self.0.try_entry(key) } /// HashMap::try_entry + /// + /// FIXME(emilio): Remove the extra Entry parameter and unify when ordermap + /// 0.4 is released. + #[cfg(feature = "gecko")] pub fn try_entry( &mut self, mut key: Atom, - quirks_mode: QuirksMode - ) -> Result, FailedAllocationError> { + quirks_mode: QuirksMode, + ) -> Result>, FailedAllocationError> { if quirks_mode == QuirksMode::Quirks { key = key.to_ascii_lowercase() }