Bug 1393656 - Part 3: stylo: Replace all hashtable collections with ones from style::hash; r?emilio draft
authorManish Goregaokar <manishearth@gmail.com>
Wed, 30 Aug 2017 17:10:14 -0700
changeset 656310 63522c067a6ca2783e67a95cafa04077e73c4155
parent 656306 90c17784a4f15053e5480c1eb6468b73198ba65e
child 729094 a086c77997cd89a92dc86718fc41eeac71fa03a3
push id77163
push userbmo:manishearth@gmail.com
push dateThu, 31 Aug 2017 00:29:21 +0000
reviewersemilio
bugs1393656
milestone57.0a1
Bug 1393656 - Part 3: stylo: Replace all hashtable collections with ones from style::hash; r?emilio MozReview-Commit-ID: 4we2FsRTO1T
servo/components/style/custom_properties.rs
servo/components/style/dom.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/hash.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/components/style/properties/longhand/position.mako.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/rule_tree/mod.rs
servo/components/style/selector_map.rs
--- a/servo/components/style/custom_properties.rs
+++ b/servo/components/style/custom_properties.rs
@@ -9,17 +9,17 @@
 use Atom;
 use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType};
 use parser::ParserContext;
 use properties::{CSSWideKeyword, DeclaredValue};
 use selectors::parser::SelectorParseError;
 use servo_arc::Arc;
 use std::ascii::AsciiExt;
 use std::borrow::{Borrow, Cow};
-use std::collections::{HashMap, HashSet};
+use hash::{HashMap, HashSet};
 use std::fmt;
 use std::hash::Hash;
 use style_traits::{HasViewportPercentage, ToCss, StyleParseError, ParseError};
 
 /// A custom property name is just an `Atom`.
 ///
 /// Note that this does not include the `--` prefix
 pub type Name = Atom;
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -23,17 +23,17 @@ use rule_tree::CascadeLevel;
 use selector_parser::{AttrValue, ElementExt};
 use selector_parser::{PseudoClassStringArg, PseudoElement};
 use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode};
 use selectors::sink::Push;
 use servo_arc::{Arc, ArcBorrow};
 use shared_lock::Locked;
 use smallvec::VecLike;
 use std::fmt;
-#[cfg(feature = "gecko")] use std::collections::HashMap;
+#[cfg(feature = "gecko")] use hash::HashMap;
 use std::fmt::Debug;
 use std::hash::Hash;
 use std::ops::Deref;
 use stylist::Stylist;
 use traversal_flags::{TraversalFlags, self};
 
 pub use style_traits::UnsafeNode;
 
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -78,17 +78,17 @@ use selector_parser::{AttrValue, Element
 use selectors::Element;
 use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity, NamespaceConstraint};
 use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext};
 use selectors::matching::{RelevantLinkStatus, VisitedHandlingMode};
 use selectors::sink::Push;
 use servo_arc::{Arc, ArcBorrow, RawOffsetArc};
 use shared_lock::Locked;
 use std::cell::RefCell;
-use std::collections::HashMap;
+use hash::HashMap;
 use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::mem;
 use std::ops::DerefMut;
 use std::ptr;
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 use stylesheets::UrlExtraData;
 use stylist::Stylist;
@@ -1365,17 +1365,17 @@ impl<'le> TElement for GeckoElement<'le>
     // update.
     //
     // https://drafts.csswg.org/css-transitions/#starting
     fn needs_transitions_update(&self,
                                 before_change_style: &ComputedValues,
                                 after_change_style: &ComputedValues)
                                 -> bool {
         use gecko_bindings::structs::nsCSSPropertyID;
-        use std::collections::HashSet;
+        use hash::HashSet;
 
         debug_assert!(self.might_need_transitions_update(Some(before_change_style),
                                                          after_change_style),
                       "We should only call needs_transitions_update if \
                        might_need_transitions_update returns true");
 
         let after_change_box_style = after_change_style.get_box();
         let transitions_count = after_change_box_style.transition_property_count();
--- a/servo/components/style/hash.rs
+++ b/servo/components/style/hash.rs
@@ -1,18 +1,33 @@
 /* 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/. */
 
+//! Reexports of hashglobe types in Gecko mode, and stdlib hashmap shims in Servo mode
+
 use fnv;
 
+
 #[cfg(feature = "gecko")]
 pub use hashglobe::hash_map::HashMap;
 #[cfg(feature = "gecko")]
 pub use hashglobe::hash_set::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 = "servo")]
+    pub use std::collections::hash_map::{Entry, Iter};
+}
 
+/// Hash map that uses the FNV hasher
 pub type FnvHashMap<K, V> = HashMap<K, V, fnv::FnvBuildHasher>;
+/// Hash set that uses the FNV hasher
 pub type FnvHashSet<T> = HashSet<T, fnv::FnvBuildHasher>;
+
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -28,17 +28,17 @@ use properties::longhands::transform::co
 use properties::longhands::vertical_align::computed_value::T as VerticalAlign;
 use properties::longhands::visibility::computed_value::T as Visibility;
 #[cfg(feature = "gecko")] use properties::{PropertyId, PropertyDeclarationId, LonghandId};
 #[cfg(feature = "gecko")] use properties::{ShorthandId};
 use selectors::parser::SelectorParseError;
 use smallvec::SmallVec;
 use std::borrow::Cow;
 use std::cmp;
-#[cfg(feature = "gecko")] use fnv::FnvHashMap;
+#[cfg(feature = "gecko")] use hash::FnvHashMap;
 use style_traits::ParseError;
 use super::ComputedValues;
 #[cfg(feature = "gecko")]
 use values::Auto;
 use values::{CSSFloat, CustomIdent, Either};
 use values::animated::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero};
 use values::animated::color::{Color as AnimatedColor, RGBA as AnimatedRGBA};
 use values::animated::effects::BoxShadowList as AnimatedBoxShadowList;
--- a/servo/components/style/properties/longhand/position.mako.rs
+++ b/servo/components/style/properties/longhand/position.mako.rs
@@ -412,17 +412,17 @@ macro_rules! impl_align_conversions {
     }
 </%helpers:longhand>
 
 <%helpers:longhand name="grid-template-areas"
         spec="https://drafts.csswg.org/css-grid/#propdef-grid-template-areas"
         products="gecko"
         animation_value_type="discrete"
         boxed="True">
-    use std::collections::HashMap;
+    use hash::HashMap;
     use std::fmt;
     use std::ops::Range;
     use str::HTML_SPACE_CHARACTERS;
     use style_traits::ToCss;
     use values::computed::ComputedValueAsSpecified;
 
     pub mod computed_value {
         pub use super::SpecifiedValue as T;
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -8,17 +8,17 @@
 // For example, Vec<&Foo> will be mangled as Vec&Foo>. To work around these issues, the code
 // can be escaped. In the above example, Vec<<&Foo> or Vec< &Foo> achieves the desired result of Vec<&Foo>.
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 #[cfg(feature = "servo")] use app_units::Au;
 use servo_arc::{Arc, UniqueArc};
 use std::borrow::Cow;
-use std::collections::HashSet;
+use hash::HashSet;
 use std::{fmt, mem, ops};
 #[cfg(feature = "gecko")] use std::ptr;
 
 #[cfg(feature = "servo")] use cssparser::RGBA;
 use cssparser::{Parser, TokenSerializationType, serialize_identifier};
 use cssparser::ParserInput;
 #[cfg(feature = "servo")] use euclid::SideOffsets2D;
 use computed_values;
--- a/servo/components/style/rule_tree/mod.rs
+++ b/servo/components/style/rule_tree/mod.rs
@@ -981,17 +981,17 @@ impl StrongRuleNode {
 
         debug!("Popping from free list: cur: {:?}, next: {:?}", current, next);
 
         Some(WeakRuleNode::from_ptr(current))
     }
 
     unsafe fn assert_free_list_has_no_duplicates_or_null(&self) {
         assert!(cfg!(debug_assertions), "This is an expensive check!");
-        use std::collections::HashSet;
+        use hash::HashSet;
 
         let me = &*self.ptr();
         assert!(me.is_root());
 
         let mut current = self.ptr();
         let mut seen = HashSet::new();
         while current != FREE_LIST_SENTINEL {
             let next = (*current).next_free.load(Ordering::Relaxed);
--- a/servo/components/style/selector_map.rs
+++ b/servo/components/style/selector_map.rs
@@ -11,18 +11,18 @@ use context::QuirksMode;
 use dom::TElement;
 use pdqsort::sort_by;
 use precomputed_hash::PrecomputedHash;
 use rule_tree::CascadeLevel;
 use selector_parser::SelectorImpl;
 use selectors::matching::{matches_selector, MatchingContext, ElementSelectorFlags};
 use selectors::parser::{Component, Combinator, SelectorIter};
 use smallvec::{SmallVec, VecLike};
-use std::collections::{HashMap, HashSet};
-use std::collections::hash_map;
+use hash::{HashMap, HashSet};
+use hash::map as hash_map;
 use std::hash::{BuildHasherDefault, Hash, Hasher};
 use stylist::Rule;
 
 /// A hasher implementation that doesn't hash anything, because it expects its
 /// input to be a suitable u32 hash.
 pub struct PrecomputedHasher {
     hash: Option<u32>,
 }
@@ -88,17 +88,17 @@ pub trait SelectorMapEntry : Sized + Clo
 /// We use a 1-entry SmallVec to avoid a separate heap allocation in the case
 /// where we only have one entry, which is quite common. See measurements in:
 /// * https://bugzilla.mozilla.org/show_bug.cgi?id=1363789#c5
 /// * https://bugzilla.mozilla.org/show_bug.cgi?id=681755
 ///
 /// TODO: Tune the initial capacity of the HashMap
 #[derive(Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-pub struct SelectorMap<T> {
+pub struct SelectorMap<T: 'static> {
     /// A hash from an ID to rules which contain that ID selector.
     pub id_hash: MaybeCaseInsensitiveHashMap<Atom, SmallVec<[T; 1]>>,
     /// 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]>>,
     /// Rules that don't have ID, class, or element selectors.
     pub other: SmallVec<[T; 1]>,
@@ -106,17 +106,20 @@ pub struct SelectorMap<T> {
     pub count: usize,
 }
 
 #[inline]
 fn sort_by_key<T, F: Fn(&T) -> K, K: Ord>(v: &mut [T], f: F) {
     sort_by(v, |a, b| f(a).cmp(&f(b)))
 }
 
-impl<T> SelectorMap<T> {
+// XXXManishearth the 'static bound can be removed when
+// our HashMap fork (hashglobe) is able to use NonZero,
+// or when stdlib gets fallible collections
+impl<T: 'static> SelectorMap<T> {
     /// Trivially constructs an empty `SelectorMap`.
     pub fn new() -> Self {
         SelectorMap {
             id_hash: MaybeCaseInsensitiveHashMap::new(),
             class_hash: MaybeCaseInsensitiveHashMap::new(),
             local_name_hash: HashMap::default(),
             other: SmallVec::new(),
             count: 0,
@@ -456,19 +459,22 @@ fn find_push<Str: Eq + Hash, V, VL>(map:
     where VL: VecLike<V> + Default
 {
     map.entry(key).or_insert_with(VL::default).push(value)
 }
 
 /// Wrapper for PrecomputedHashMap that does ASCII-case-insensitive lookup in quirks mode.
 #[derive(Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-pub struct MaybeCaseInsensitiveHashMap<K: PrecomputedHash + Hash + Eq, V>(PrecomputedHashMap<K, V>);
+pub struct MaybeCaseInsensitiveHashMap<K: PrecomputedHash + Hash + Eq, V: 'static>(PrecomputedHashMap<K, V>);
 
-impl<V> MaybeCaseInsensitiveHashMap<Atom, V> {
+// XXXManishearth the 'static bound can be removed when
+// our HashMap fork (hashglobe) is able to use NonZero,
+// or when stdlib gets fallible collections
+impl<V: 'static> MaybeCaseInsensitiveHashMap<Atom, V> {
     /// Empty map
     pub fn new() -> Self {
         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 {