Auto merge of #17710 - emilio:revert-bloom-sharing, r=heycam

Revert "Auto merge of #17701 - bholley:reuse_allocations, r=emilio"

This reverts commit ebfc8f5858, reversing
changes made to 5585ff2c44.

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 -->

components/layout/Cargo.toml

@@ -26,7 +26,7 @@ log = "0.3.5"
 msg = {path = "../msg"}
 net_traits = {path = "../net_traits"}
 ordered-float = "0.4"
-parking_lot = "0.4"
+parking_lot = "0.3.3"
 profile_traits = {path = "../profile_traits"}
 range = {path = "../range"}
 rayon = "0.8"

components/layout_thread/Cargo.toml

@@ -23,7 +23,7 @@ lazy_static = "0.2"
 log = "0.3.5"
 msg = {path = "../msg"}
 net_traits = {path = "../net_traits"}
-parking_lot = {version = "0.4", features = ["nightly"]}
+parking_lot = {version = "0.3.3", features = ["nightly"]}
 profile_traits = {path = "../profile_traits"}
 rayon = "0.8"
 script = {path = "../script"}

components/script/Cargo.toml

@@ -65,7 +65,7 @@ net_traits = {path = "../net_traits"}
 num-traits = "0.1.32"
 offscreen_gl_context = { version = "0.11", features = ["serde"] }
 open = "1.1.1"
-parking_lot = "0.4"
+parking_lot = "0.3"
 phf = "0.7.18"
 profile_traits = {path = "../profile_traits"}
 range = {path = "../range"}

components/servo_arc/Cargo.toml

@@ -14,6 +14,5 @@ servo = ["serde", "heapsize"]
 
 [dependencies]
 heapsize = {version = "0.4.0", optional = true}
-nodrop = {version = "0.1.8"}
 serde = {version = "1.0", optional = true}
-stable_deref_trait = "1.0.0"
+nodrop = {version = "0.1.8"}

components/servo_arc/lib.rs

@@ -22,16 +22,14 @@
 // duplicate those here.
 #![allow(missing_docs)]
 
-extern crate nodrop;
 #[cfg(feature = "servo")] extern crate serde;
-extern crate stable_deref_trait;
+extern crate nodrop;
 
 #[cfg(feature = "servo")]
 use heapsize::HeapSizeOf;
 use nodrop::NoDrop;
 #[cfg(feature = "servo")]
 use serde::{Deserialize, Serialize};
-use stable_deref_trait::{CloneStableDeref, StableDeref};
 use std::{isize, usize};
 use std::borrow;
 use std::cmp::Ordering;
@@ -434,9 +432,6 @@ impl<T: ?Sized> AsRef<T> for Arc<T> {
     }
 }
 
-unsafe impl<T: ?Sized> StableDeref for Arc<T> {}
-unsafe impl<T: ?Sized> CloneStableDeref for Arc<T> {}
-
 // This is what the HeapSize crate does for regular arc, but is questionably
 // sound. See https://github.com/servo/heapsize/issues/37
 #[cfg(feature = "servo")]

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"

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(),
         }

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;

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
     }