Measure Arc<Locked<T>> fields properly.

Currently when we measure various Arc<Locked<T>> fields we don't measure the T itself, but only the descendants of the T. This patch fixes this. This fix requires introducing a new trait, MallocUnconditionalShallowSizeOf, which is implemented for servo_arc::Arc. A similar trait, MallocConditionalShallowSizeOf, is also introduced, though it has no uses as yet.
servo · Sep 12, 2017 · 779fbda · 779fbda
1 parent 7f4cb18
commit 779fbda
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 22 deletions.
diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs
@@ -141,9 +141,10 @@ pub trait MallocSizeOf {
 
 /// Trait for measuring the "shallow" heap usage of a container.
 pub trait MallocShallowSizeOf {
-    /// Measure the heap usage of direct descendant structures, but not the
-    /// space taken up by the value itself. Anything pointed to by the
-    /// immediate children must be measured separately, using iteration.
+    /// Measure the heap usage of immediate heap-allocated descendant
+    /// structures, but not the space taken up by the value itself. Anything
+    /// beyond the immediate descendants must be measured separately, using
+    /// iteration.
     fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize;
 }
 
@@ -156,6 +157,12 @@ pub trait MallocUnconditionalSizeOf {
     fn unconditional_size_of(&self, ops: &mut MallocSizeOfOps) -> usize;
 }
 
+/// `MallocUnconditionalSizeOf` combined with `MallocShallowSizeOf`.
+pub trait MallocUnconditionalShallowSizeOf {
+    /// `unconditional_size_of` combined with `shallow_size_of`.
+    fn unconditional_shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize;
+}
+
 /// Like `MallocSizeOf`, but only measures if the value hasn't already been
 /// measured. For use with types like `Rc` and `Arc` when appropriate (e.g.
 /// when there is no "primary" reference).
@@ -166,6 +173,12 @@ pub trait MallocConditionalSizeOf {
     fn conditional_size_of(&self, ops: &mut MallocSizeOfOps) -> usize;
 }
 
+/// `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> {
     fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         ops.malloc_size_of(&**self)
@@ -294,9 +307,25 @@ impl<K, V, S> MallocSizeOf for HashMap<K, V, S>
 //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())
+    }
+}
+
 impl<T: MallocSizeOf> MallocUnconditionalSizeOf for Arc<T> {
     fn unconditional_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
-        ops.malloc_size_of(self.heap_ptr()) + (**self).size_of(ops)
+        self.unconditional_shallow_size_of(ops) + (**self).size_of(ops)
+    }
+}
+
+impl<T> MallocConditionalShallowSizeOf for Arc<T> {
+    fn conditional_shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
+        if ops.have_seen_ptr(self.heap_ptr()) {
+            0
+        } else {
+            self.unconditional_shallow_size_of(ops)
+        }
     }
 }
 

diff --git a/components/style/stylesheets/document_rule.rs b/components/style/stylesheets/document_rule.rs
@@ -8,7 +8,7 @@
 
 use cssparser::{Parser, Token, SourceLocation, BasicParseError};
 #[cfg(feature = "gecko")]
-use malloc_size_of::MallocSizeOfOps;
+use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use media_queries::Device;
 use parser::{Parse, ParserContext};
 use servo_arc::Arc;
@@ -34,7 +34,8 @@ impl DocumentRule {
     #[cfg(feature = "gecko")]
     pub fn size_of(&self, guard: &SharedRwLockReadGuard, ops: &mut MallocSizeOfOps) -> usize {
         // Measurement of other fields may be added later.
-        self.rules.read_with(guard).size_of(guard, ops)
+        self.rules.unconditional_shallow_size_of(ops) +
+            self.rules.read_with(guard).size_of(guard, ops)
     }
 }
 

diff --git a/components/style/stylesheets/media_rule.rs b/components/style/stylesheets/media_rule.rs
@@ -8,7 +8,7 @@
 
 use cssparser::SourceLocation;
 #[cfg(feature = "gecko")]
-use malloc_size_of::MallocSizeOfOps;
+use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use media_queries::MediaList;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
@@ -34,7 +34,8 @@ impl MediaRule {
     #[cfg(feature = "gecko")]
     pub fn size_of(&self, guard: &SharedRwLockReadGuard, ops: &mut MallocSizeOfOps) -> usize {
         // Measurement of other fields may be added later.
-        self.rules.read_with(guard).size_of(guard, ops)
+        self.rules.unconditional_shallow_size_of(ops) +
+            self.rules.read_with(guard).size_of(guard, ops)
     }
 }
 

diff --git a/components/style/stylesheets/mod.rs b/components/style/stylesheets/mod.rs
@@ -26,7 +26,7 @@ pub mod viewport_rule;
 use cssparser::{parse_one_rule, Parser, ParserInput};
 use error_reporting::NullReporter;
 #[cfg(feature = "gecko")]
-use malloc_size_of::MallocSizeOfOps;
+use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use parser::{ParserContext, ParserErrorContext};
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
@@ -120,21 +120,26 @@ impl CssRule {
             // it on the C++ side in the child list of the ServoStyleSheet.
             CssRule::Import(_) => 0,
 
-            CssRule::Style(ref lock) => lock.read_with(guard).size_of(guard, ops),
+            CssRule::Style(ref lock) =>
+                lock.unconditional_shallow_size_of(ops) + lock.read_with(guard).size_of(guard, ops),
 
-            CssRule::Media(ref lock) => lock.read_with(guard).size_of(guard, ops),
+            CssRule::Media(ref lock) =>
+                lock.unconditional_shallow_size_of(ops) + lock.read_with(guard).size_of(guard, ops),
 
             CssRule::FontFace(_) => 0,
             CssRule::FontFeatureValues(_) => 0,
             CssRule::CounterStyle(_) => 0,
             CssRule::Viewport(_) => 0,
             CssRule::Keyframes(_) => 0,
 
-            CssRule::Supports(ref lock) => lock.read_with(guard).size_of(guard, ops),
+            CssRule::Supports(ref lock) =>
+                lock.unconditional_shallow_size_of(ops) + lock.read_with(guard).size_of(guard, ops),
 
-            CssRule::Page(ref lock) => lock.read_with(guard).size_of(guard, ops),
+            CssRule::Page(ref lock) =>
+                lock.unconditional_shallow_size_of(ops) + lock.read_with(guard).size_of(guard, ops),
 
-            CssRule::Document(ref lock) => lock.read_with(guard).size_of(guard, ops),
+            CssRule::Document(ref lock) =>
+                lock.unconditional_shallow_size_of(ops) + lock.read_with(guard).size_of(guard, ops),
         }
     }
 }

diff --git a/components/style/stylesheets/page_rule.rs b/components/style/stylesheets/page_rule.rs
@@ -8,7 +8,7 @@
 
 use cssparser::SourceLocation;
 #[cfg(feature = "gecko")]
-use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
+use malloc_size_of::{MallocSizeOf, MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use properties::PropertyDeclarationBlock;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
@@ -37,7 +37,7 @@ impl PageRule {
     #[cfg(feature = "gecko")]
     pub fn size_of(&self, guard: &SharedRwLockReadGuard, ops: &mut MallocSizeOfOps) -> usize {
         // Measurement of other fields may be added later.
-        self.block.read_with(guard).size_of(ops)
+        self.block.unconditional_shallow_size_of(ops) + self.block.read_with(guard).size_of(ops)
     }
 }
 

diff --git a/components/style/stylesheets/style_rule.rs b/components/style/stylesheets/style_rule.rs
@@ -6,7 +6,7 @@
 
 use cssparser::SourceLocation;
 #[cfg(feature = "gecko")]
-use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps};
+use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use properties::PropertyDeclarationBlock;
 use selector_parser::SelectorImpl;
 use selectors::SelectorList;
@@ -58,7 +58,8 @@ impl StyleRule {
             n += ops.malloc_size_of(selector.thin_arc_heap_ptr());
         }
 
-        n += self.block.read_with(guard).size_of(ops);
+        n += self.block.unconditional_shallow_size_of(ops) +
+             self.block.read_with(guard).size_of(ops);
 
         n
     }

diff --git a/components/style/stylesheets/stylesheet.rs b/components/style/stylesheets/stylesheet.rs
@@ -10,7 +10,7 @@ use fallible::FallibleVec;
 use fnv::FnvHashMap;
 use invalidation::media_queries::{MediaListKey, ToMediaListKey};
 #[cfg(feature = "gecko")]
-use malloc_size_of::MallocSizeOfOps;
+use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use media_queries::{MediaList, Device};
 use parking_lot::RwLock;
 use parser::{ParserContext, ParserErrorContext};
@@ -122,7 +122,8 @@ impl StylesheetContents {
     #[cfg(feature = "gecko")]
     pub fn size_of(&self, guard: &SharedRwLockReadGuard, ops: &mut MallocSizeOfOps) -> usize {
         // Measurement of other fields may be added later.
-        self.rules.read_with(guard).size_of(guard, ops)
+        self.rules.unconditional_shallow_size_of(ops) +
+            self.rules.read_with(guard).size_of(guard, ops)
     }
 }
 

diff --git a/components/style/stylesheets/supports_rule.rs b/components/style/stylesheets/supports_rule.rs
@@ -7,7 +7,7 @@
 use cssparser::{BasicParseError, ParseError as CssParseError, ParserInput};
 use cssparser::{Delimiter, parse_important, Parser, SourceLocation, Token};
 #[cfg(feature = "gecko")]
-use malloc_size_of::MallocSizeOfOps;
+use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use parser::ParserContext;
 use properties::{PropertyId, PropertyDeclaration, PropertyParserContext, SourcePropertyDeclaration};
 use selectors::parser::SelectorParseError;
@@ -37,7 +37,8 @@ impl SupportsRule {
     #[cfg(feature = "gecko")]
     pub fn size_of(&self, guard: &SharedRwLockReadGuard, ops: &mut MallocSizeOfOps) -> usize {
         // Measurement of other fields may be added later.
-        self.rules.read_with(guard).size_of(guard, ops)
+        self.rules.unconditional_shallow_size_of(ops) +
+            self.rules.read_with(guard).size_of(guard, ops)
     }
 }