diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2017-07-13 01:26:34 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-07-13 01:26:34 -0700 |
commit | 9d74ae890b31fc45452ae60af7d89573fc52a86f (patch) | |
tree | afade45401982b13e9657d63f3c7fecf689af7f7 /components/style | |
parent | 9451b41d33d00016cfd45b0c4fc7a999a83169a9 (diff) | |
parent | a34f288b9864be52829887383219ca39711e51f8 (diff) | |
download | servo-9d74ae890b31fc45452ae60af7d89573fc52a86f.tar.gz servo-9d74ae890b31fc45452ae60af7d89573fc52a86f.zip |
Auto merge of #17710 - emilio:revert-bloom-sharing, r=heycam
Revert "Auto merge of #17701 - bholley:reuse_allocations, r=emilio"
This reverts commit ebfc8f585834948f9b7564ffe382a6e266edf738, reversing
changes made to 5585ff2c440dc27c8e98e834c9c0e9d956581b8e.
Animation code can reenter and create a new TLS context from the traversal
SequentialTask, so this won't work as written, and it's making test fails.
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17710)
<!-- Reviewable:end -->
Diffstat (limited to 'components/style')
-rw-r--r-- | components/style/Cargo.toml | 3 | ||||
-rw-r--r-- | components/style/bloom.rs | 32 | ||||
-rw-r--r-- | components/style/lib.rs | 1 | ||||
-rw-r--r-- | components/style/sharing/mod.rs | 90 |
4 files changed, 21 insertions, 105 deletions
diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index 860c652cd02..c6d23ab2b8c 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -54,8 +54,7 @@ num_cpus = {version = "1.1.0", optional = true} num-integer = "0.1.32" num-traits = "0.1.32" ordered-float = "0.4" -owning_ref = "0.3.3" -parking_lot = "0.4" +parking_lot = "0.3.3" pdqsort = "0.1.0" precomputed-hash = "0.1" rayon = "0.8" diff --git a/components/style/bloom.rs b/components/style/bloom.rs index d6d05c2ed20..34b6b608962 100644 --- a/components/style/bloom.rs +++ b/components/style/bloom.rs @@ -7,18 +7,9 @@ #![deny(missing_docs)] -use atomic_refcell::{AtomicRefMut, AtomicRefCell}; use dom::{SendElement, TElement}; -use owning_ref::OwningHandle; use selectors::bloom::BloomFilter; use smallvec::SmallVec; -use stylearc::Arc; - -/// Bloom filters are large allocations, so we store them in thread-local storage -/// such that they can be reused across style traversals. StyleBloom is responsible -/// for ensuring that the bloom filter is zeroed when it is dropped. -thread_local!(static BLOOM_KEY: Arc<AtomicRefCell<BloomFilter>> = - Arc::new(AtomicRefCell::new(BloomFilter::new()))); /// A struct that allows us to fast-reject deep descendant selectors avoiding /// selector-matching. @@ -52,11 +43,8 @@ thread_local!(static BLOOM_KEY: Arc<AtomicRefCell<BloomFilter>> = /// immutable during a restyle. /// pub struct StyleBloom<E: TElement> { - /// A handle to the bloom filter from the thread upon which this StyleBloom - /// was created. We use AtomicRefCell so that this is all |Send|, which allows - /// StyleBloom to live in ThreadLocalStyleContext, which is dropped from the - /// parent thread. - filter: OwningHandle<Arc<AtomicRefCell<BloomFilter>>, AtomicRefMut<'static, BloomFilter>>, + /// The bloom filter per se. + filter: Box<BloomFilter>, /// The stack of elements that this bloom filter contains, along with the /// number of hashes pushed for each element. @@ -113,23 +101,11 @@ fn each_relevant_element_hash<E, F>(element: E, mut f: F) }); } -impl<E: TElement> Drop for StyleBloom<E> { - fn drop(&mut self) { - // Leave the reusable bloom filter in a zeroed state. - self.clear(); - } -} - impl<E: TElement> StyleBloom<E> { - /// Create an empty `StyleBloom`. Because StyleBloom acquires the thread- - /// local filter buffer, creating multiple live StyleBloom instances at - /// the same time on the same thread will panic. + /// Create an empty `StyleBloom`. pub fn new() -> Self { - let bloom_arc = BLOOM_KEY.with(|b| b.clone()); - let filter = OwningHandle::new_with_fn(bloom_arc, |x| unsafe { x.as_ref() }.unwrap().borrow_mut()); - debug_assert!(filter.is_zeroed(), "Forgot to zero the bloom filter last time"); StyleBloom { - filter: filter, + filter: Box::new(BloomFilter::new()), elements: Default::default(), pushed_hashes: Default::default(), } diff --git a/components/style/lib.rs b/components/style/lib.rs index 3280caa5cb3..5775dc1001b 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -67,7 +67,6 @@ pub extern crate nsstring_vendor as nsstring; extern crate num_integer; extern crate num_traits; extern crate ordered_float; -extern crate owning_ref; extern crate parking_lot; extern crate pdqsort; #[cfg(feature = "gecko")] extern crate precomputed_hash; diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index 8f82e6d6ea0..c7c2c946cc8 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -66,7 +66,6 @@ use Atom; use applicable_declarations::ApplicableDeclarationBlock; -use atomic_refcell::{AtomicRefCell, AtomicRefMut}; use bit_vec::BitVec; use bloom::StyleBloom; use cache::{LRUCache, LRUCacheMutIterator}; @@ -74,15 +73,12 @@ use context::{SelectorFlagsMap, SharedStyleContext, StyleContext}; use data::{ElementData, ElementStyles}; use dom::{TElement, SendElement}; use matching::{ChildCascadeRequirement, MatchMethods}; -use owning_ref::OwningHandle; use properties::ComputedValues; use selector_parser::RestyleDamage; use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode}; use smallvec::SmallVec; -use std::marker::PhantomData; use std::mem; use std::ops::Deref; -use stylearc::Arc; use stylist::Stylist; mod checks; @@ -97,8 +93,7 @@ mod checks; /// improvements (e.g. 3x fewer styles having to be resolved than at size 8) and /// slight performance improvements. Sizes larger than 32 haven't really been /// tested. -pub const SHARING_CACHE_SIZE: usize = 31; -const SHARING_CACHE_BACKING_STORE_SIZE: usize = SHARING_CACHE_SIZE + 1; +pub const STYLE_SHARING_CANDIDATE_CACHE_SIZE: usize = 31; /// Controls whether the style sharing cache is used. #[derive(Clone, Copy, PartialEq)] @@ -216,21 +211,14 @@ impl ValidationData { /// Note that this information is stored in TLS and cleared after the traversal, /// and once here, the style information of the element is immutable, so it's /// safe to access. -/// -/// Important: If you change the members/layout here, You need to do the same for -/// FakeCandidate below. #[derive(Debug)] pub struct StyleSharingCandidate<E: TElement> { - /// The element. - element: E, + /// The element. We use SendElement here so that the cache may live in + /// ScopedTLS. + element: SendElement<E>, validation_data: ValidationData, } -struct FakeCandidate { - _element: usize, - _validation_data: ValidationData, -} - impl<E: TElement> Deref for StyleSharingCandidate<E> { type Target = E; @@ -243,12 +231,12 @@ impl<E: TElement> Deref for StyleSharingCandidate<E> { impl<E: TElement> StyleSharingCandidate<E> { /// Get the classlist of this candidate. fn class_list(&mut self) -> &[Atom] { - self.validation_data.class_list(self.element) + self.validation_data.class_list(*self.element) } /// Get the pres hints of this candidate. fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] { - self.validation_data.pres_hints(self.element) + self.validation_data.pres_hints(*self.element) } /// Compute the bit vector of revalidation selector match results @@ -259,7 +247,7 @@ impl<E: TElement> StyleSharingCandidate<E> { bloom: &StyleBloom<E>, ) -> &BitVec { self.validation_data.revalidation_match_results( - self.element, + *self.element, stylist, bloom, /* bloom_known_valid = */ false, @@ -469,81 +457,35 @@ pub enum StyleSharingResult { StyleWasShared(usize, ChildCascadeRequirement), } -/// Style sharing caches are are large allocations, so we store them in thread-local -/// storage such that they can be reused across style traversals. Ideally, we'd just -/// stack-allocate these buffers with uninitialized memory, but right now rustc can't -/// avoid memmoving the entire cache during setup, which gets very expensive. See -/// issues like [1] and [2]. -/// -/// Given that the cache stores entries of type TElement, we transmute to usize -/// before storing in TLS. This is safe as long as we make sure to empty the cache -/// before we let it go. -/// -/// [1] https://github.com/rust-lang/rust/issues/42763 -/// [2] https://github.com/rust-lang/rust/issues/13707 -type SharingCacheBase<Candidate> = LRUCache<[Candidate; SHARING_CACHE_BACKING_STORE_SIZE]>; -type SharingCache<E> = SharingCacheBase<StyleSharingCandidate<E>>; -type TypelessSharingCache = SharingCacheBase<FakeCandidate>; -type StoredSharingCache = Arc<AtomicRefCell<TypelessSharingCache>>; - -thread_local!(static SHARING_CACHE_KEY: StoredSharingCache = - Arc::new(AtomicRefCell::new(LRUCache::new()))); - /// An LRU cache of the last few nodes seen, so that we can aggressively try to /// reuse their styles. /// /// Note that this cache is flushed every time we steal work from the queue, so /// storing nodes here temporarily is safe. pub struct StyleSharingCandidateCache<E: TElement> { - /// The LRU cache, with the type cast away to allow persisting the allocation. - cache_typeless: OwningHandle<StoredSharingCache, AtomicRefMut<'static, TypelessSharingCache>>, - /// Bind this structure to the lifetime of E, since that's what we effectively store. - marker: PhantomData<SendElement<E>>, + cache: LRUCache<[StyleSharingCandidate<E>; STYLE_SHARING_CANDIDATE_CACHE_SIZE + 1]>, /// The DOM depth we're currently at. This is used as an optimization to /// clear the cache when we change depths, since we know at that point /// nothing in the cache will match. dom_depth: usize, } -impl<E: TElement> Drop for StyleSharingCandidateCache<E> { - fn drop(&mut self) { - self.clear(); - } -} - impl<E: TElement> StyleSharingCandidateCache<E> { - fn cache(&self) -> &SharingCache<E> { - let base: &TypelessSharingCache = &*self.cache_typeless; - unsafe { mem::transmute(base) } - } - - fn cache_mut(&mut self) -> &mut SharingCache<E> { - let base: &mut TypelessSharingCache = &mut *self.cache_typeless; - unsafe { mem::transmute(base) } - } - /// Create a new style sharing candidate cache. pub fn new() -> Self { - assert_eq!(mem::size_of::<SharingCache<E>>(), mem::size_of::<TypelessSharingCache>()); - assert_eq!(mem::align_of::<SharingCache<E>>(), mem::align_of::<TypelessSharingCache>()); - let cache_arc = SHARING_CACHE_KEY.with(|c| c.clone()); - let cache = OwningHandle::new_with_fn(cache_arc, |x| unsafe { x.as_ref() }.unwrap().borrow_mut()); - debug_assert_eq!(cache.num_entries(), 0); - StyleSharingCandidateCache { - cache_typeless: cache, - marker: PhantomData, + cache: LRUCache::new(), dom_depth: 0, } } /// Returns the number of entries in the cache. pub fn num_entries(&self) -> usize { - self.cache().num_entries() + self.cache.num_entries() } fn iter_mut(&mut self) -> LRUCacheMutIterator<StyleSharingCandidate<E>> { - self.cache_mut().iter_mut() + self.cache.iter_mut() } /// Tries to insert an element in the style sharing cache. @@ -588,20 +530,20 @@ impl<E: TElement> StyleSharingCandidateCache<E> { self.clear(); self.dom_depth = dom_depth; } - self.cache_mut().insert(StyleSharingCandidate { - element: *element, + self.cache.insert(StyleSharingCandidate { + element: unsafe { SendElement::new(*element) }, validation_data: validation_data, }); } /// Touch a given index in the style sharing candidate cache. pub fn touch(&mut self, index: usize) { - self.cache_mut().touch(index); + self.cache.touch(index); } /// Clear the style sharing candidate cache. pub fn clear(&mut self) { - self.cache_mut().evict_all() + self.cache.evict_all() } /// Attempts to share a style with another node. @@ -668,7 +610,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> { } debug!("{:?} Cannot share style: {} cache entries", target.element, - self.cache().num_entries()); + self.cache.num_entries()); StyleSharingResult::CannotShare } |