servo: Merge #18474 - malloc_size_of tweaks (from nnethercote:malloc_size_of-tweaks); r=jdm
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 13 Sep 2017 08:02:45 -0500
changeset 430064 1873e0e3b4c84f052309a61bb9c9e6bcc36a1d8d
parent 430063 b80e267bdf307f08a49810729a31a567e39fa0c5
child 430065 1fef4a3d0683813959cb02085698b39ce3497a35
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
milestone57.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
servo: Merge #18474 - malloc_size_of tweaks (from nnethercote:malloc_size_of-tweaks); r=jdm <!-- Please describe your changes on the following line: --> A couple of tweaks. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #18473 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [X] These changes do not require tests because they are tested in Gecko. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 8ee055fdc147a7e971077c91775cc002560231df
servo/components/malloc_size_of/lib.rs
servo/components/style/rule_tree/mod.rs
servo/components/style/stylesheets/style_rule.rs
--- a/servo/components/malloc_size_of/lib.rs
+++ b/servo/components/malloc_size_of/lib.rs
@@ -98,35 +98,42 @@ impl MallocSizeOfOps {
             size_of_op: size_of,
             enclosing_size_of_op: malloc_enclosing_size_of,
             have_seen_ptr_op: have_seen_ptr,
         }
     }
 
     /// Check if an allocation is empty. This relies on knowledge of how Rust
     /// handles empty allocations, which may change in the future.
-    fn is_empty<T>(ptr: *const T) -> bool {
-        return ptr as usize <= ::std::mem::align_of::<T>();
+    fn is_empty<T: ?Sized>(ptr: *const T) -> bool {
+        // The correct condition is this:
+        //   `ptr as usize <= ::std::mem::align_of::<T>()`
+        // But we can't call align_of() on a ?Sized T. So we approximate it
+        // with the following. 256 is large enough that it should always be
+        // larger than the required alignment, but small enough that it is
+        // always in the first page of memory and therefore not a legitimate
+        // address.
+        return ptr as *const usize as usize <= 256
     }
 
     /// Call `size_of_op` on `ptr`, first checking that the allocation isn't
     /// empty, because some types (such as `Vec`) utilize empty allocations.
-    pub fn malloc_size_of<T>(&self, ptr: *const T) -> usize {
+    pub unsafe fn malloc_size_of<T: ?Sized>(&self, ptr: *const T) -> usize {
         if MallocSizeOfOps::is_empty(ptr) {
             0
         } else {
-            unsafe { (self.size_of_op)(ptr as *const c_void) }
+            (self.size_of_op)(ptr as *const c_void)
         }
     }
 
     /// Call `enclosing_size_of_op` on `ptr`, which must not be empty.
-    pub fn malloc_enclosing_size_of<T>(&self, ptr: *const T) -> usize {
+    pub unsafe fn malloc_enclosing_size_of<T>(&self, ptr: *const T) -> usize {
         assert!(!MallocSizeOfOps::is_empty(ptr));
         let enclosing_size_of_op = self.enclosing_size_of_op.expect("missing enclosing_size_of_op");
-        unsafe { enclosing_size_of_op(ptr as *const c_void) }
+        enclosing_size_of_op(ptr as *const c_void)
     }
 
     /// Call `have_seen_ptr_op` on `ptr`.
     pub fn have_seen_ptr<T>(&mut self, ptr: *const T) -> bool {
         let have_seen_ptr_op = self.have_seen_ptr_op.as_mut().expect("missing have_seen_ptr_op");
         have_seen_ptr_op(ptr as *const c_void)
     }
 }
@@ -174,23 +181,23 @@ pub trait MallocConditionalSizeOf {
 }
 
 /// `MallocConditionalSizeOf` combined with `MallocShallowSizeOf`.
 pub trait MallocConditionalShallowSizeOf {
     /// `conditional_size_of` combined with `shallow_size_of`.
     fn conditional_shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize;
 }
 
-impl<T> MallocShallowSizeOf for Box<T> {
+impl<T: ?Sized> MallocShallowSizeOf for Box<T> {
     fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
-        ops.malloc_size_of(&**self)
+        unsafe { ops.malloc_size_of(&**self) }
     }
 }
 
-impl<T: MallocSizeOf> MallocSizeOf for Box<T> {
+impl<T: MallocSizeOf + ?Sized> MallocSizeOf for Box<T> {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         self.shallow_size_of(ops) + (**self).size_of(ops)
     }
 }
 
 impl<A: MallocSizeOf, B: MallocSizeOf> MallocSizeOf for (A, B) {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         self.0.size_of(ops) + self.1.size_of(ops)
@@ -202,36 +209,46 @@ impl<T: MallocSizeOf> MallocSizeOf for O
         if let Some(val) = self.as_ref() {
             val.size_of(ops)
         } else {
             0
         }
     }
 }
 
+impl<T: MallocSizeOf> MallocSizeOf for [T] {
+    fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
+        let mut n = 0;
+        for elem in self.iter() {
+            n += elem.size_of(ops);
+        }
+        n
+    }
+}
+
 impl<T> MallocShallowSizeOf for Vec<T> {
     fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
-        ops.malloc_size_of(self.as_ptr())
+        unsafe { ops.malloc_size_of(self.as_ptr()) }
     }
 }
 
 impl<T: MallocSizeOf> MallocSizeOf for Vec<T> {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         let mut n = self.shallow_size_of(ops);
         for elem in self.iter() {
             n += elem.size_of(ops);
         }
         n
     }
 }
 
 impl<A: Array> MallocShallowSizeOf for SmallVec<A> {
     fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         if self.spilled() {
-            ops.malloc_size_of(self.as_ptr())
+            unsafe { ops.malloc_size_of(self.as_ptr()) }
         } else {
             0
         }
     }
 }
 
 impl<A> MallocSizeOf for SmallVec<A>
     where A: Array,
@@ -250,17 +267,17 @@ impl<T, S> MallocShallowSizeOf for HashS
     where T: Eq + Hash,
           S: BuildHasher
 {
     fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         // The first value from the iterator gives us an interior pointer.
         // `ops.malloc_enclosing_size_of()` then gives us the storage size.
         // This assumes that the `HashSet`'s contents (values and hashes) are
         // all stored in a single contiguous heap allocation.
-        self.iter().next().map_or(0, |t| ops.malloc_enclosing_size_of(t))
+        self.iter().next().map_or(0, |t| unsafe { ops.malloc_enclosing_size_of(t) })
     }
 }
 
 impl<T, S> MallocSizeOf for HashSet<T, S>
     where T: Eq + Hash + MallocSizeOf,
           S: BuildHasher,
 {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
@@ -276,17 +293,17 @@ impl<K, V, S> MallocShallowSizeOf for Ha
     where K: Eq + Hash,
           S: BuildHasher
 {
     fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         // The first value from the iterator gives us an interior pointer.
         // `ops.malloc_enclosing_size_of()` then gives us the storage size.
         // This assumes that the `HashMap`'s contents (keys, values, and
         // hashes) are all stored in a single contiguous heap allocation.
-        self.values().next().map_or(0, |v| ops.malloc_enclosing_size_of(v))
+        self.values().next().map_or(0, |v| unsafe { ops.malloc_enclosing_size_of(v) })
     }
 }
 
 impl<K, V, S> MallocSizeOf for HashMap<K, V, S>
     where K: Eq + Hash + MallocSizeOf,
           V: MallocSizeOf,
           S: BuildHasher,
 {
@@ -304,17 +321,17 @@ impl<K, V, S> MallocSizeOf for HashMap<K
 // trait bounds are ever allowed, this code should be uncommented.
 // (We do have a compile-fail test for this:
 // rc_arc_must_not_derive_malloc_size_of.rs)
 //impl<T> !MallocSizeOf for Arc<T> { }
 //impl<T> !MallocShallowSizeOf for Arc<T> { }
 
 impl<T> MallocUnconditionalShallowSizeOf for Arc<T> {
     fn unconditional_shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
-        ops.malloc_size_of(self.heap_ptr())
+        unsafe { ops.malloc_size_of(self.heap_ptr()) }
     }
 }
 
 impl<T: MallocSizeOf> MallocUnconditionalSizeOf for Arc<T> {
     fn unconditional_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         self.unconditional_shallow_size_of(ops) + (**self).size_of(ops)
     }
 }
--- a/servo/components/style/rule_tree/mod.rs
+++ b/servo/components/style/rule_tree/mod.rs
@@ -62,17 +62,17 @@ impl Drop for RuleTree {
         // and will trigger synchronous dropping of the Rule nodes.
         self.root.get().next_free.store(ptr::null_mut(), Ordering::Relaxed);
     }
 }
 
 #[cfg(feature = "gecko")]
 impl MallocSizeOf for RuleTree {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
-        let mut n = ops.malloc_size_of(self.root.ptr());
+        let mut n = unsafe { ops.malloc_size_of(self.root.ptr()) };
         n += self.root.get().size_of(ops);
         n
     }
 }
 
 /// A style source for the rule node. It can either be a CSS style rule or a
 /// declaration block.
 ///
@@ -801,17 +801,17 @@ impl RuleNode {
     }
 }
 
 #[cfg(feature = "gecko")]
 impl MallocSizeOf for RuleNode {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         let mut n = 0;
         for child in self.iter_children() {
-            n += ops.malloc_size_of(child.ptr());
+            n += unsafe { ops.malloc_size_of(child.ptr()) };
             n += unsafe { (*child.ptr()).size_of(ops) };
         }
         n
     }
 }
 
 #[derive(Clone)]
 struct WeakRuleNode {
--- a/servo/components/style/stylesheets/style_rule.rs
+++ b/servo/components/style/stylesheets/style_rule.rs
@@ -50,17 +50,17 @@ impl StyleRule {
 
         // We may add measurement of things hanging off the embedded Components
         // later.
         n += self.selectors.0.shallow_size_of(ops);
         for selector in self.selectors.0.iter() {
             // It's safe to measure this ThinArc directly because it's the
             // "primary" reference. (The secondary references are on the
             // Stylist.)
-            n += ops.malloc_size_of(selector.thin_arc_heap_ptr());
+            n += unsafe { ops.malloc_size_of(selector.thin_arc_heap_ptr()) };
         }
 
         n += self.block.unconditional_shallow_size_of(ops) +
              self.block.read_with(guard).size_of(ops);
 
         n
     }
 }