Bug 1550554 - Implement ArcSlice::default(). r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 16 May 2019 23:22:04 +0000
changeset 474253 7c2ff60588882f354678a676c2028eebd71f7557
parent 474252 7321594dab268ea3f11722178c98d6412f1db078
child 474254 6524a34864c39e0957cc10d124d61a6d795b5894
push id113144
push usershindli@mozilla.com
push dateFri, 17 May 2019 16:44:55 +0000
treeherdermozilla-inbound@f4c4b796f845 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1550554
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1550554 - Implement ArcSlice::default(). r=heycam Share a singleton to avoid allocating for empty lists. Differential Revision: https://phabricator.services.mozilla.com/D30543
Cargo.lock
layout/style/ServoStyleConstsInlines.h
servo/components/servo_arc/lib.rs
servo/components/style_traits/Cargo.toml
servo/components/style_traits/arc_slice.rs
servo/components/style_traits/lib.rs
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2813,16 +2813,17 @@ dependencies = [
 [[package]]
 name = "style_traits"
 version = "0.0.1"
 dependencies = [
  "app_units 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "cssparser 0.25.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "malloc_size_of 0.0.1",
  "malloc_size_of_derive 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.21.0",
  "servo_arc 0.1.1",
  "to_shmem 0.0.1",
  "to_shmem_derive 0.0.1",
 ]
 
--- a/layout/style/ServoStyleConstsInlines.h
+++ b/layout/style/ServoStyleConstsInlines.h
@@ -18,17 +18,17 @@
 namespace mozilla {
 
 // This code is basically a C++ port of the Arc::clone() implementation in
 // servo/components/servo_arc/lib.rs.
 static constexpr const size_t kStaticRefcount =
     std::numeric_limits<size_t>::max();
 static constexpr const size_t kMaxRefcount =
     std::numeric_limits<intptr_t>::max();
-static constexpr const uint32_t kArcSliceCanary = 0xf3f3f3f3;
+static constexpr const uint64_t kArcSliceCanary = 0xf3f3f3f3f3f3f3f3;
 
 #define ASSERT_CANARY \
   MOZ_DIAGNOSTIC_ASSERT(_0.ptr->data.header.header == kArcSliceCanary, "Uh?");
 
 template <typename T>
 inline StyleArcSlice<T>::StyleArcSlice(const StyleArcSlice& aOther) {
   MOZ_DIAGNOSTIC_ASSERT(aOther._0.ptr);
   _0.ptr = aOther._0.ptr;
--- a/servo/components/servo_arc/lib.rs
+++ b/servo/components/servo_arc/lib.rs
@@ -607,26 +607,25 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
     fn from_header_and_iter_alloc<F, I>(alloc: F, header: H, mut items: I, is_static: bool) -> Self
     where
         F: FnOnce(Layout) -> *mut u8,
         I: Iterator<Item = T> + ExactSizeIterator,
     {
         use std::mem::{align_of, size_of};
         assert_ne!(size_of::<T>(), 0, "Need to think about ZST");
 
+        let inner_align = align_of::<ArcInner<HeaderSlice<H, [T; 0]>>>();
+        debug_assert!(inner_align >= align_of::<T>());
+
         // Compute the required size for the allocation.
         let num_items = items.len();
         let size = {
-            // First, determine the alignment of a hypothetical pointer to a
-            // HeaderSlice.
-            let fake_slice_ptr_align: usize = mem::align_of::<ArcInner<HeaderSlice<H, [T; 0]>>>();
-
             // Next, synthesize a totally garbage (but properly aligned) pointer
             // to a sequence of T.
-            let fake_slice_ptr = fake_slice_ptr_align as *const T;
+            let fake_slice_ptr = inner_align as *const T;
 
             // Convert that sequence to a fat pointer. The address component of
             // the fat pointer will be garbage, but the length will be correct.
             let fake_slice = unsafe { slice::from_raw_parts(fake_slice_ptr, num_items) };
 
             // Pretend the garbage address points to our allocation target (with
             // a trailing sequence of T), rather than just a sequence of T.
             let fake_ptr = fake_slice as *const [T] as *const ArcInner<HeaderSlice<H, [T]>>;
@@ -636,23 +635,23 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
             // type with the length from the fat pointer. The garbage address
             // will not be used.
             mem::size_of_val(fake_ref)
         };
 
         let ptr: *mut ArcInner<HeaderSlice<H, [T]>>;
         unsafe {
             // Allocate the buffer.
-            let layout = if mem::align_of::<T>() <= mem::align_of::<usize>() {
-                Layout::from_size_align_unchecked(size, mem::align_of::<usize>())
-            } else if mem::align_of::<T>() <= mem::align_of::<u64>() {
-                // On 32-bit platforms <T> may have 8 byte alignment while usize has 4 byte aligment.
-                // Use u64 to avoid over-alignment.
+            let layout = if inner_align <= align_of::<usize>() {
+                Layout::from_size_align_unchecked(size, align_of::<usize>())
+            } else if inner_align <= align_of::<u64>() {
+                // On 32-bit platforms <T> may have 8 byte alignment while usize
+                // has 4 byte aligment.  Use u64 to avoid over-alignment.
                 // This branch will compile away in optimized builds.
-                Layout::from_size_align_unchecked(size, mem::align_of::<u64>())
+                Layout::from_size_align_unchecked(size, align_of::<u64>())
             } else {
                 panic!("Over-aligned type not handled");
             };
 
             let buffer = alloc(layout);
 
             // Synthesize the fat pointer. We do this by claiming we have a direct
             // pointer to a [T], and then changing the type of the borrow. The key
@@ -684,17 +683,17 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
                             .expect("ExactSizeIterator over-reported length"),
                     );
                     current = current.offset(1);
                 }
                 // We should have consumed the buffer exactly, maybe accounting
                 // for some padding from the alignment.
                 debug_assert!(
                     (buffer.offset(size as isize) as usize - current as *mut u8 as usize) <
-                        align_of::<Self>()
+                        inner_align
                 );
             }
             assert!(
                 items.next().is_none(),
                 "ExactSizeIterator under-reported length"
             );
         }
 
--- a/servo/components/style_traits/Cargo.toml
+++ b/servo/components/style_traits/Cargo.toml
@@ -13,16 +13,17 @@ path = "lib.rs"
 servo = ["serde", "servo_atoms", "cssparser/serde", "webrender_api", "servo_url"]
 gecko = []
 
 [dependencies]
 app_units = "0.7"
 cssparser = "0.25"
 bitflags = "1.0"
 euclid = "0.19"
+lazy_static = "1"
 malloc_size_of = { path = "../malloc_size_of" }
 malloc_size_of_derive = "0.1"
 selectors = { path = "../selectors" }
 serde = {version = "1.0", optional = true}
 webrender_api = {git = "https://github.com/servo/webrender", optional = true}
 servo_atoms = {path = "../atoms", optional = true}
 servo_arc = { path = "../servo_arc" }
 servo_url = { path = "../url", optional = true }
--- a/servo/components/style_traits/arc_slice.rs
+++ b/servo/components/style_traits/arc_slice.rs
@@ -1,66 +1,95 @@
 /* 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 https://mozilla.org/MPL/2.0/. */
 
 //! A thin atomically-reference-counted slice.
 
 use servo_arc::ThinArc;
-use std::mem;
+use std::{iter, mem};
 use std::ops::Deref;
 use std::ptr::NonNull;
 
 /// A canary that we stash in ArcSlices.
 ///
 /// Given we cannot use a zero-sized-type for the header, since well, C++
 /// doesn't have zsts, and we want to use cbindgen for this type, we may as well
 /// assert some sanity at runtime.
-const ARC_SLICE_CANARY: u32 = 0xf3f3f3f3;
+///
+/// We use an u64, to guarantee that we can use a single singleton for every
+/// empty slice, even if the types they hold are aligned differently.
+const ARC_SLICE_CANARY: u64 = 0xf3f3f3f3f3f3f3f3;
 
 /// A wrapper type for a refcounted slice using ThinArc.
 ///
 /// cbindgen:derive-eq=false
 /// cbindgen:derive-neq=false
 #[repr(C)]
 #[derive(Debug, Clone, PartialEq, Eq, ToShmem)]
-pub struct ArcSlice<T>(#[shmem(field_bound)] ThinArc<u32, T>);
+pub struct ArcSlice<T>(#[shmem(field_bound)] ThinArc<u64, T>);
 
 impl<T> Deref for ArcSlice<T> {
     type Target = [T];
 
     #[inline]
     fn deref(&self) -> &Self::Target {
         debug_assert_eq!(self.0.header.header, ARC_SLICE_CANARY);
         &self.0.slice
     }
 }
 
-/// The inner pointer of an ArcSlice<T>, to be sent via FFI.
-/// The type of the pointer is a bit of a lie, we just want to preserve the type
-/// but these pointers cannot be constructed outside of this crate, so we're
-/// good.
-#[repr(C)]
-pub struct ForgottenArcSlicePtr<T>(NonNull<T>);
+lazy_static! {
+    // ThinArc doesn't support alignments greater than align_of::<u64>.
+    static ref EMPTY_ARC_SLICE: ArcSlice<u64> = {
+        ArcSlice(ThinArc::from_header_and_iter(ARC_SLICE_CANARY, iter::empty()))
+    };
+}
+
+impl<T> Default for ArcSlice<T> {
+    #[allow(unsafe_code)]
+    fn default() -> Self {
+        debug_assert!(
+            mem::align_of::<T>() <= mem::align_of::<u64>(),
+            "Need to increase the alignment of EMPTY_ARC_SLICE"
+        );
+        unsafe {
+            let empty: ArcSlice<_> = EMPTY_ARC_SLICE.clone();
+            let empty: Self = mem::transmute(empty);
+            debug_assert_eq!(empty.len(), 0);
+            empty
+        }
+    }
+}
 
 impl<T> ArcSlice<T> {
     /// Creates an Arc for a slice using the given iterator to generate the
     /// slice.
     #[inline]
     pub fn from_iter<I>(items: I) -> Self
     where
         I: Iterator<Item = T> + ExactSizeIterator,
     {
+        if items.len() == 0 {
+            return Self::default();
+        }
         ArcSlice(ThinArc::from_header_and_iter(ARC_SLICE_CANARY, items))
     }
 
     /// Creates a value that can be passed via FFI, and forgets this value
     /// altogether.
     #[inline]
     #[allow(unsafe_code)]
     pub fn forget(self) -> ForgottenArcSlicePtr<T> {
         let ret = unsafe {
             ForgottenArcSlicePtr(NonNull::new_unchecked(self.0.ptr() as *const _ as *mut _))
         };
         mem::forget(self);
         ret
     }
 }
+
+/// The inner pointer of an ArcSlice<T>, to be sent via FFI.
+/// The type of the pointer is a bit of a lie, we just want to preserve the type
+/// but these pointers cannot be constructed outside of this crate, so we're
+/// good.
+#[repr(C)]
+pub struct ForgottenArcSlicePtr<T>(NonNull<T>);
--- a/servo/components/style_traits/lib.rs
+++ b/servo/components/style_traits/lib.rs
@@ -11,16 +11,18 @@
 #![deny(unsafe_code, missing_docs)]
 
 extern crate app_units;
 #[macro_use]
 extern crate bitflags;
 #[macro_use]
 extern crate cssparser;
 extern crate euclid;
+#[macro_use]
+extern crate lazy_static;
 extern crate malloc_size_of;
 #[macro_use]
 extern crate malloc_size_of_derive;
 extern crate selectors;
 #[cfg(feature = "servo")]
 #[macro_use]
 extern crate serde;
 extern crate servo_arc;