servo: Merge #19912 - style: Temporarily use OrderMap on Gecko (from emilio:ordermap); r=Manishearth

This will allow us to discard std hash map as a source of crashes. Source-Repo: https://github.com/servo/servo Source-Revision: e6d9c251d1b9770aa520bf358b79d906a00d71bb --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 3b5a6c2c7cd694eeeebc9c068a0add45545f0642
2025-02-20 17:20:54 +00:00 · 2018-01-31 21:33:36 -06:00 · 2018-01-31 21:33:36 -06:00 · 9cd350ab4e
commit 9cd350ab4e
parent 5d06c194b0
9 changed files with 387 additions and 14 deletions
--- a/servo/Cargo.lock
+++ b/servo/Cargo.lock
@ -1086,6 +1086,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)",
 ]

@ -1555,6 +1556,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)",
@ -2008,6 +2010,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"
@ -3666,6 +3673,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)" = "<none>"
 "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"
--- a/servo/components/hashglobe/Cargo.toml
+++ b/servo/components/hashglobe/Cargo.toml
@ -11,6 +11,7 @@ readme = "README.md"

 [dependencies]
 libc = "0.2"
+ordermap = "0.3"

 [dev-dependencies]
 rand = "0.3"
--- a/servo/components/hashglobe/src/lib.rs
+++ b/servo/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};

--- a/servo/components/hashglobe/src/order.rs
+++ b/servo/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<K, V, S = RandomState>(OrderMap<K, V, S>);
+
+
+use FailedAllocationError;
+
+impl<K, V, S> Deref for HashMap<K, V, S> {
+    type Target = OrderMap<K, V, S>;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<K, V, S> DerefMut for HashMap<K, V, S> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl<K, V, S> HashMap<K, V, S>
+    where K: Eq + Hash,
+          S: BuildHasher
+{
+    #[inline]
+    pub fn try_with_hasher(hash_builder: S) -> Result<HashMap<K, V, S>, FailedAllocationError> {
+        Ok(HashMap(OrderMap::with_hasher(hash_builder)))
+    }
+
+    #[inline]
+    pub fn try_with_capacity_and_hasher(capacity: usize,
+                                        hash_builder: S)
+        -> Result<HashMap<K, V, S>, FailedAllocationError> {
+        Ok(HashMap(OrderMap::with_capacity_and_hasher(capacity, hash_builder)))
+    }
+
+    pub fn with_capacity_and_hasher(capacity: usize, hash_builder: S) -> HashMap<K, V, S> {
+        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<Entry<K, V, S>, FailedAllocationError> {
+        Ok(self.entry(key))
+    }
+
+    #[inline]
+    pub fn try_insert(&mut self, k: K, v: V) -> Result<Option<V>, FailedAllocationError> {
+        Ok(self.insert(k, v))
+    }
+}
+
+#[derive(Clone)]
+pub struct HashSet<T, S = RandomState>(OrderSet<T, S>);
+
+
+impl<T, S> Deref for HashSet<T, S> {
+    type Target = OrderSet<T, S>;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<T, S> DerefMut for HashSet<T, S> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl<T: Hash + Eq> HashSet<T, RandomState> {
+    #[inline]
+    pub fn new() -> HashSet<T, RandomState> {
+        HashSet(OrderSet::new())
+    }
+
+    #[inline]
+    pub fn with_capacity(capacity: usize) -> HashSet<T, RandomState> {
+        HashSet(OrderSet::with_capacity(capacity))
+    }
+}
+
+
+impl<T, S> HashSet<T, S>
+    where T: Eq + Hash,
+          S: BuildHasher
+{
+    #[inline]
+    pub fn with_hasher(hasher: S) -> HashSet<T, S> {
+        HashSet(OrderSet::with_hasher(hasher))
+    }
+
+
+    #[inline]
+    pub fn with_capacity_and_hasher(capacity: usize, hasher: S) -> HashSet<T, S> {
+        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<bool, FailedAllocationError> {
+        Ok(self.insert(value))
+    }
+}
+
+// Pass through trait impls
+// We can't derive these since the bounds are not obvious to the derive macro
+
+impl<K: Hash + Eq, V, S: BuildHasher + Default> Default for HashMap<K, V, S> {
+    fn default() -> Self {
+        HashMap(Default::default())
+    }
+}
+
+impl<K, V, S> fmt::Debug for HashMap<K, V, S>
+    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<K, V, S> PartialEq for HashMap<K, V, S>
+    where K: Eq + Hash,
+          V: PartialEq,
+          S: BuildHasher
+{
+    fn eq(&self, other: &HashMap<K, V, S>) -> bool {
+        self.0.eq(&other.0)
+    }
+}
+
+impl<K, V, S> Eq for HashMap<K, V, S>
+    where K: Eq + Hash,
+          V: Eq,
+          S: BuildHasher
+{
+}
+
+impl<'a, K, V, S> IntoIterator for &'a HashMap<K, V, S>
+    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<K, V, S>
+    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<T: Eq + Hash, S: BuildHasher + Default> Default for HashSet<T, S> {
+    fn default() -> Self {
+        HashSet(Default::default())
+    }
+}
+
+impl<T, S> fmt::Debug for HashSet<T, S>
+    where T: Eq + Hash + fmt::Debug,
+          S: BuildHasher
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
+impl<T, S> PartialEq for HashSet<T, S>
+    where T: Eq + Hash,
+          S: BuildHasher
+{
+    fn eq(&self, other: &HashSet<T, S>) -> bool {
+        self.0.eq(&other.0)
+    }
+}
+
+impl<T, S> Eq for HashSet<T, S>
+    where T: Eq + Hash,
+          S: BuildHasher
+{
+}
+
+impl<'a, T, S> IntoIterator for &'a HashSet<T, S>
+    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<T, S> IntoIterator for HashSet<T, S>
+    where T: Eq + Hash,
+          S: BuildHasher
+{
+    type Item = T;
+    type IntoIter = SetIntoIter<T>;
+
+
+    fn into_iter(self) -> SetIntoIter<T> {
+        self.0.into_iter()
+    }
+}
+
+
--- a/servo/components/malloc_size_of/Cargo.toml
+++ b/servo/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"
--- a/servo/components/malloc_size_of/lib.rs
+++ b/servo/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<T, S> MallocShallowSizeOf for hashglobe::hash_set::HashSet<T, S>
    }
 }

+impl<T, S> MallocSizeOf for ordermap::OrderSet<T, S>
+    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<T, S> MallocShallowSizeOf for ordermap::OrderSet<T, S>
+    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::<T>() + size_of::<usize>())
+        }
+    }
+}
+
 impl<T, S> MallocSizeOf for hashglobe::hash_set::HashSet<T, S>
    where T: Eq + Hash + MallocSizeOf,
          S: BuildHasher,
@ -442,6 +470,26 @@ impl<T, S> MallocSizeOf for hashglobe::fake::HashSet<T, S>
    }
 }

+impl<T, S> MallocShallowSizeOf for hashglobe::order::HashSet<T, S>
+    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<T, S> MallocSizeOf for hashglobe::order::HashSet<T, S>
+    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<K, V, S> MallocShallowSizeOf for std::collections::HashMap<K, V, S>
    where K: Eq + Hash,
          S: BuildHasher
@ -471,6 +519,35 @@ impl<K, V, S> MallocSizeOf for std::collections::HashMap<K, V, S>
    }
 }

+impl<K, V, S> MallocShallowSizeOf for ordermap::OrderMap<K, V, S>
+    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::<V>() + size_of::<K>() + size_of::<usize>())
+        }
+    }
+}
+
+impl<K, V, S> MallocSizeOf for ordermap::OrderMap<K, V, S>
+    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<K, V, S> MallocShallowSizeOf for hashglobe::hash_map::HashMap<K, V, S>
    where K: Eq + Hash,
          S: BuildHasher
@ -521,6 +598,27 @@ impl<K, V, S> MallocSizeOf for hashglobe::fake::HashMap<K, V, S>
    }
 }

+impl<K, V, S> MallocShallowSizeOf for hashglobe::order::HashMap<K, V, S>
+    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<K, V, S> MallocSizeOf for hashglobe::order::HashMap<K, V, S>
+    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<T> MallocSizeOf for std::marker::PhantomData<T> {
    fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
--- a/servo/components/style/hash.rs
+++ b/servo/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};
 }
--- a/servo/components/style/invalidation/element/invalidation_map.rs
+++ b/servo/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(),
--- a/servo/components/style/selector_map.rs
+++ b/servo/components/style/selector_map.rs
@ -512,19 +512,12 @@ impl<V: 'static> MaybeCaseInsensitiveHashMap<Atom, V> {
        MaybeCaseInsensitiveHashMap(PrecomputedHashMap::default())
    }

-    /// HashMap::entry
-    pub fn entry(&mut self, mut key: Atom, quirks_mode: QuirksMode) -> hash_map::Entry<Atom, V> {
-        if quirks_mode == QuirksMode::Quirks {
-            key = key.to_ascii_lowercase()
-        }
-        self.0.entry(key)
-    }
-
    /// HashMap::try_entry
+    #[cfg(not(feature = "gecko"))]
    pub fn try_entry(
        &mut self,
        mut key: Atom,
-        quirks_mode: QuirksMode
+        quirks_mode: QuirksMode,
    ) -> Result<hash_map::Entry<Atom, V>, FailedAllocationError> {
        if quirks_mode == QuirksMode::Quirks {
            key = key.to_ascii_lowercase()
@ -532,6 +525,22 @@ impl<V: 'static> MaybeCaseInsensitiveHashMap<Atom, V> {
        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<hash_map::Entry<Atom, V, BuildHasherDefault<PrecomputedHasher>>, 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()