Correct some edge cases with Arc and Rc

Summary:
- `Arc<dyn Trait>` now records pointer size correctly as 16 bytes
- `Arc<CacheLine>` / other large-alignment types no longer under-report size of ArcInner/RcInner. Now they include the padding.
- `triomphe::Arc<T>` now includes padding in the heap part for small T

X-link: facebookexperimental/allocative#14

Reviewed By: JakobDegen

Differential Revision: D66431445

Pulled By: ndmitchell

fbshipit-source-id: aa0c05b2fae94898af21cbc206a79cd974073612

allocative/allocative/src/impls/std/sync.rs

@@ -7,6 +7,7 @@
  * of this source tree.
  */
 
+use std::alloc::Layout;
 use std::mem;
 use std::rc;
 use std::rc::Rc;
@@ -41,25 +42,43 @@ impl<T: Allocative> Allocative for RwLock<T> {
     }
 }
 
+#[allow(dead_code)] // Only used for its size
+#[repr(C)] // as does std
+struct RcBox<T: ?Sized> {
+    a: usize,
+    b: usize,
+    t: T,
+}
+
+impl<T: ?Sized> RcBox<T> {
+    fn layout(val: &T) -> Layout {
+        let val_layout = Layout::for_value(val);
+        // See rcbox_layout_for_value_layout in std
+        Layout::new::<RcBox<()>>()
+            .extend(val_layout)
+            .unwrap()
+            .0
+            .pad_to_align()
+    }
+}
+
 impl<T: Allocative + ?Sized> Allocative for Arc<T> {
     fn visit<'a, 'b: 'a>(&self, visitor: &'a mut Visitor<'b>) {
         let mut visitor = visitor.enter_self_sized::<Self>();
         {
             let visitor = visitor.enter_shared(
                 PTR_NAME,
-                // TODO(nga): 8 or 16 depending on sized or unsized.
-                mem::size_of::<*const ()>(),
+                // Possibly fat pointer for size
+                mem::size_of::<*const T>(),
+                // Force thin pointer for identity, as fat pointers can have
+                // different VTable addresses attached to the same memory
                 Arc::as_ptr(self) as *const (),
             );
             if let Some(mut visitor) = visitor {
-                #[allow(dead_code)] // Only used for its size
-                struct ArcInner(AtomicUsize, AtomicUsize);
                 {
                     let val: &T = self;
-                    let mut visitor = visitor.enter(
-                        Key::new("ArcInner"),
-                        mem::size_of::<ArcInner>() + mem::size_of_val(val),
-                    );
+                    let mut visitor =
+                        visitor.enter(Key::new("ArcInner"), RcBox::layout(val).size());
                     val.visit(&mut visitor);
                     visitor.exit();
                 }
@@ -88,19 +107,16 @@ impl<T: Allocative> Allocative for Rc<T> {
         {
             let visitor = visitor.enter_shared(
                 PTR_NAME,
-                // TODO(nga): 8 or 16 depending on sized or unsized.
-                mem::size_of::<*const ()>(),
+                // Possibly fat pointer for size
+                mem::size_of::<*const T>(),
+                // Force thin pointer for identity, as fat pointers can have
+                // different VTable addresses attached to the same memory
                 Rc::as_ptr(self) as *const (),
             );
             if let Some(mut visitor) = visitor {
-                #[allow(dead_code)] // fields `0` and `1` are never read
-                struct RcInner(AtomicUsize, AtomicUsize, ());
                 {
                     let val: &T = self;
-                    let mut visitor = visitor.enter(
-                        Key::new("RcInner"),
-                        mem::size_of::<RcInner>() + mem::size_of_val(val),
-                    );
+                    let mut visitor = visitor.enter(Key::new("RcInner"), RcBox::layout(val).size());
                     val.visit(&mut visitor);
                     visitor.exit();
                 }
@@ -198,3 +214,26 @@ impl<T: Allocative> Allocative for Mutex<T> {
         visitor.exit();
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::sync::Arc;
+
+    use crate as allocative;
+    use crate::golden::golden_test;
+    use crate::Allocative;
+
+    #[derive(Allocative)]
+    #[repr(align(64))]
+    struct CacheLine(u8);
+
+    #[test]
+    fn test_arc_align() {
+        assert_eq!(std::mem::size_of::<CacheLine>(), 64);
+
+        // ArcInner has two usizes, and then a 64-byte-aligned CacheLine
+        // in repr(C) order. So it should have a self size of 64 including
+        // padding.
+        golden_test!(&Arc::new(CacheLine(0)));
+    }
+}

allocative/allocative/src/impls/std/sync_test_arc_align.src

@@ -0,0 +1,9 @@
+# @generated
+# To regenerate, run:
+# ```
+# ALLOCATIVE_REGENERATE_TESTS=1 cargo test -p allocative
+# ```
+ArcInner 64
+ArcInner;allocative::impls::std::sync::tests::CacheLine 63
+ArcInner;allocative::impls::std::sync::tests::CacheLine;0;u8 1
+alloc::sync::Arc<allocative::impls::std::sync::tests::CacheLine>;ptr 8

allocative/allocative/src/impls/triomphe.rs

@@ -41,6 +41,13 @@ impl<H: Allocative> Allocative for HeaderWithLength<H> {
     }
 }
 
+#[repr(C)]
+struct ThinArcInnerRepr<H> {
+    _counter: AtomicUsize,
+    _header: H,
+    _len: usize,
+}
+
 impl<H: Allocative, T: Allocative> Allocative for ThinArc<H, T> {
     fn visit<'a, 'b: 'a>(&self, visitor: &'a mut Visitor<'b>) {
         let mut visitor = visitor.enter_self_sized::<Self>();
@@ -50,16 +57,11 @@ impl<H: Allocative, T: Allocative> Allocative for ThinArc<H, T> {
                 mem::size_of::<*mut u8>(),
                 ThinArc::ptr(self) as *const (),
             ) {
-                #[repr(C)]
-                struct ThinArcInnerRepr<H> {
-                    _counter: AtomicUsize,
-                    _header: H,
-                    _len: usize,
-                }
                 let size = Layout::new::<ThinArcInnerRepr<H>>()
                     .extend(Layout::array::<T>(self.slice.len()).unwrap())
                     .unwrap()
                     .0
+                    .pad_to_align()
                     .size();
                 {
                     let mut visitor =
@@ -76,6 +78,10 @@ impl<H: Allocative, T: Allocative> Allocative for ThinArc<H, T> {
     }
 }
 
+struct ArcInnerRepr {
+    _counter: AtomicUsize,
+}
+
 impl<T: Allocative + ?Sized> Allocative for Arc<T> {
     fn visit<'a, 'b: 'a>(&self, visitor: &'a mut Visitor<'b>) {
         let mut visitor = visitor.enter_self_sized::<Self>();
@@ -85,19 +91,11 @@ impl<T: Allocative + ?Sized> Allocative for Arc<T> {
                 mem::size_of::<*mut T>(),
                 Arc::heap_ptr(self) as *const (),
             ) {
-                struct ArcInnerRepr {
-                    _counter: AtomicUsize,
-                }
                 let size = Layout::new::<ArcInnerRepr>()
-                    .extend(
-                        Layout::from_size_align(
-                            mem::size_of_val::<T>(self),
-                            mem::align_of_val::<T>(self),
-                        )
-                        .unwrap(),
-                    )
+                    .extend(Layout::for_value::<T>(self))
                     .unwrap()
                     .0
+                    .pad_to_align()
                     .size();
                 {
                     let mut visitor = visitor.enter(Key::for_type_name::<ArcInnerRepr>(), size);
@@ -129,4 +127,10 @@ mod tests {
         let vec = vec![arc.clone(), arc.clone(), arc];
         golden_test!(&vec);
     }
+
+    #[test]
+    fn test_align() {
+        let arc = Arc::new(0u8);
+        golden_test!(&arc);
+    }
 }

allocative/allocative/src/impls/triomphe_test_align.src

@@ -0,0 +1,8 @@
+# @generated
+# To regenerate, run:
+# ```
+# ALLOCATIVE_REGENERATE_TESTS=1 cargo test -p allocative
+# ```
+allocative::impls::triomphe::ArcInnerRepr 15
+allocative::impls::triomphe::ArcInnerRepr;data;u8 1
+triomphe::arc::Arc<u8>;ptr 8

allocative/allocative/src/impls/triomphe_test_shared.src

@@ -6,7 +6,7 @@
 alloc::vec::Vec<triomphe::arc::Arc<alloc::string::String>> 16
 alloc::vec::Vec<triomphe::arc::Arc<alloc::string::String>>;ptr 8
 alloc::vec::Vec<triomphe::arc::Arc<alloc::string::String>>;ptr;triomphe::arc::Arc<alloc::string::String>;ptr 24
-allocative::impls::triomphe::<impl allocative::allocative_trait::Allocative for triomphe::arc::Arc<_>>::visit::ArcInnerRepr 8
-allocative::impls::triomphe::<impl allocative::allocative_trait::Allocative for triomphe::arc::Arc<_>>::visit::ArcInnerRepr;data;alloc::string::String 16
-allocative::impls::triomphe::<impl allocative::allocative_trait::Allocative for triomphe::arc::Arc<_>>::visit::ArcInnerRepr;data;alloc::string::String;ptr 8
-allocative::impls::triomphe::<impl allocative::allocative_trait::Allocative for triomphe::arc::Arc<_>>::visit::ArcInnerRepr;data;alloc::string::String;ptr;capacity;u8 100
+allocative::impls::triomphe::ArcInnerRepr 8
+allocative::impls::triomphe::ArcInnerRepr;data;alloc::string::String 16
+allocative::impls::triomphe::ArcInnerRepr;data;alloc::string::String;ptr 8
+allocative::impls::triomphe::ArcInnerRepr;data;alloc::string::String;ptr;capacity;u8 100

allocative/allocative/src/impls/triomphe_test_simple.src

@@ -3,8 +3,8 @@
 # ```
 # ALLOCATIVE_REGENERATE_TESTS=1 cargo test -p allocative
 # ```
-allocative::impls::triomphe::<impl allocative::allocative_trait::Allocative for triomphe::arc::Arc<_>>::visit::ArcInnerRepr 8
-allocative::impls::triomphe::<impl allocative::allocative_trait::Allocative for triomphe::arc::Arc<_>>::visit::ArcInnerRepr;data;alloc::string::String 16
-allocative::impls::triomphe::<impl allocative::allocative_trait::Allocative for triomphe::arc::Arc<_>>::visit::ArcInnerRepr;data;alloc::string::String;ptr 8
-allocative::impls::triomphe::<impl allocative::allocative_trait::Allocative for triomphe::arc::Arc<_>>::visit::ArcInnerRepr;data;alloc::string::String;ptr;capacity;u8 3
+allocative::impls::triomphe::ArcInnerRepr 8
+allocative::impls::triomphe::ArcInnerRepr;data;alloc::string::String 16
+allocative::impls::triomphe::ArcInnerRepr;data;alloc::string::String;ptr 8
+allocative::impls::triomphe::ArcInnerRepr;data;alloc::string::String;ptr;capacity;u8 3
 triomphe::arc::Arc<alloc::string::String>;ptr 8