From 0041125858cc5ea4df0595efb12ea4bad58b4843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 7 Aug 2017 13:51:52 +0200 Subject: [PATCH 1/3] style: Update string-cache to v0.6.2 --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9485e216d00..7b40353a4f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1678,7 +1678,7 @@ dependencies = [ "phf 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", "phf_codegen 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)", - "string_cache 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "string_cache 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache_codegen 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "tendril 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -2814,7 +2814,7 @@ dependencies = [ name = "servo_atoms" version = "0.0.1" dependencies = [ - "string_cache 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "string_cache 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache_codegen 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -2966,7 +2966,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "string_cache" -version = "0.6.0" +version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "debug_unreachable 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3873,7 +3873,7 @@ dependencies = [ "checksum slab 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "17b4fcaed89ab08ef143da37bc52adbcc04d4a69014f4c1208d6b51f0c47bc23" "checksum smallvec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2e40af10aafe98b4d8294ae8388d8a5cd0707c65d364872efe72d063ec44bee0" "checksum stable_deref_trait 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "15132e0e364248108c5e2c02e3ab539be8d6f5d52a01ca9bbf27ed657316f02b" -"checksum string_cache 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "23429a3aca80e7cc7f0060853a97fbba9a90e30ef36b29d13e22559cd7f3dc54" +"checksum string_cache 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "413fc7852aeeb5472f1986ef755f561ddf0c789d3d796e65f0b6fe293ecd4ef8" "checksum string_cache_codegen 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "479cde50c3539481f33906a387f2bd17c8e87cb848c35b6021d41fb81ff9b4d7" "checksum string_cache_shared 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b1884d1bc09741d466d9b14e6d37ac89d6909cbcac41dd9ae982d4d063bbedfc" "checksum strsim 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b4d15c810519a91cf877e7e36e63fe068815c678181439f2f29e2562147c3694" From 720b17a54b6abea9a407f80870fba6b9b859c8a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 7 Aug 2017 13:36:34 +0200 Subject: [PATCH 2/3] style: Avoid hashing the precomputed hash atoms already have. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This shaves a fair amount of time of stylist rebuild. Most of the extra remaining overhead are memory allocations... I'll try to get those sorted out next, though fixing that may overlap quite a bit with the work Cameron is doing in bug 1382925. MozReview-Commit-ID: 3j2SBY4QnKf Signed-off-by: Emilio Cobos Álvarez --- components/style/lib.rs | 2 +- components/style/selector_map.rs | 51 +++++++++++++++++++++++++++----- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/components/style/lib.rs b/components/style/lib.rs index e1453706977..fcf48752530 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -71,7 +71,7 @@ extern crate ordered_float; extern crate owning_ref; extern crate parking_lot; extern crate pdqsort; -#[cfg(feature = "gecko")] extern crate precomputed_hash; +extern crate precomputed_hash; extern crate rayon; extern crate selectors; #[cfg(feature = "servo")] #[macro_use] extern crate serde; diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index c4bc3dcadb0..d4775f315ab 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -9,19 +9,54 @@ use {Atom, LocalName}; use applicable_declarations::ApplicableDeclarationBlock; use context::QuirksMode; use dom::TElement; -use fnv::FnvHashMap; 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 selectors::parser::LocalName as LocalNameSelector; use smallvec::{SmallVec, VecLike}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::collections::hash_map; -use std::hash::Hash; +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: u32, +} + +impl Default for PrecomputedHasher { + fn default() -> Self { + Self { hash: 0 } + } +} + +/// A simple alias for a hashmap using PrecomputedHasher. +pub type PrecomputedHashMap = HashMap>; + +/// A simple alias for a hashset using PrecomputedHasher. +pub type PrecomputedHashSet = HashSet>; + +impl Hasher for PrecomputedHasher { + #[inline] + fn write(&mut self, _: &[u8]) { + unreachable!() + } + + #[inline] + fn write_u32(&mut self, i: u32) { + self.hash = i; + } + + #[inline] + fn finish(&self) -> u64 { + self.hash as u64 + } +} + /// A trait to abstract over a given selector map entry. pub trait SelectorMapEntry : Sized + Clone { /// Gets the selector we should use to index in the selector map. @@ -63,7 +98,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: FnvHashMap>, + pub local_name_hash: PrecomputedHashMap>, /// Rules that don't have ID, class, or element selectors. pub other: Vec, /// The number of entries in this map. @@ -434,7 +469,7 @@ pub fn get_local_name(iter: SelectorIter) } #[inline] -fn find_push(map: &mut FnvHashMap, +fn find_push(map: &mut PrecomputedHashMap, key: Str, value: V) where VL: VecLike + Default @@ -442,15 +477,15 @@ fn find_push(map: &mut FnvHashMap, map.entry(key).or_insert_with(VL::default).push(value) } -/// Wrapper for FnvHashMap that does ASCII-case-insensitive lookup in quirks mode. +/// Wrapper for PrecomputedHashMap that does ASCII-case-insensitive lookup in quirks mode. #[derive(Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -pub struct MaybeCaseInsensitiveHashMap(FnvHashMap); +pub struct MaybeCaseInsensitiveHashMap(PrecomputedHashMap); impl MaybeCaseInsensitiveHashMap { /// Empty map pub fn new() -> Self { - MaybeCaseInsensitiveHashMap(FnvHashMap::default()) + MaybeCaseInsensitiveHashMap(PrecomputedHashMap::default()) } /// HashMap::entry From 3d54c0710f56193299f6eb6aa173585189a6bb86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 7 Aug 2017 13:50:05 +0200 Subject: [PATCH 3/3] style: Make PrecomputedHasher fail loudly and with error messages when used incorrectly. --- components/style/selector_map.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index d4775f315ab..193196c488a 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -25,12 +25,12 @@ 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: u32, + hash: Option, } impl Default for PrecomputedHasher { fn default() -> Self { - Self { hash: 0 } + Self { hash: None } } } @@ -43,17 +43,19 @@ pub type PrecomputedHashSet = HashSet u64 { - self.hash as u64 + self.hash.expect("PrecomputedHasher wasn't fed?") as u64 } }