Bug 1550554 - Implement ArcSlice::default(). r=heycam

Share a singleton to avoid allocating for empty lists.

Differential Revision: https://phabricator.services.mozilla.com/D30543

--HG--
extra : moz-landing-system : lando

Cargo.lock

@@ -2818,6 +2818,7 @@ dependencies = [
  "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",

layout/style/ServoStyleConstsInlines.h

@@ -23,7 +23,7 @@ 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?");

servo/components/servo_arc/lib.rs

@@ -612,16 +612,15 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
         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.
@@ -641,13 +640,13 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
         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");
             };
@@ -689,7 +688,7 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
                 // 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!(

servo/components/style_traits/Cargo.toml

@@ -18,6 +18,7 @@ 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" }

servo/components/style_traits/arc_slice.rs

@@ -5,7 +5,7 @@
 //! 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;
 
@@ -14,7 +14,10 @@ use std::ptr::NonNull;
 /// 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.
 ///
@@ -22,7 +25,7 @@ const ARC_SLICE_CANARY: u32 = 0xf3f3f3f3;
 /// 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];
@@ -34,12 +37,28 @@ impl<T> Deref for ArcSlice<T> {
     }
 }
 
-/// 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
@@ -49,6 +68,9 @@ impl<T> ArcSlice<T> {
     where
         I: Iterator<Item = T> + ExactSizeIterator,
     {
+        if items.len() == 0 {
+            return Self::default();
+        }
         ArcSlice(ThinArc::from_header_and_iter(ARC_SLICE_CANARY, items))
     }
 
@@ -64,3 +86,10 @@ impl<T> ArcSlice<T> {
         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>);

servo/components/style_traits/lib.rs

@@ -16,6 +16,8 @@ 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;