servo: Merge #18455 - Measure Arc<Locked<T>> fields properly (from nnethercote:measure-Arc-Locked-properly); r=jdm
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 12 Sep 2017 08:40:56 -0500
changeset 429873 92be1f3506bb8a95ca15bbd0621b361ee881b909
parent 429872 e19a2e8394e8b6338b7eba2dbb8cd6ec540cb569
child 429874 15adab29635350835ce0e3b0310280f3e9ba1314
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 #18455 - Measure Arc<Locked<T>> fields properly (from nnethercote:measure-Arc-Locked-properly); r=jdm 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. <!-- Please describe your changes on the following line: --> --- <!-- 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 - [ ] These changes fix #__ (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: bffe158fa40fda72e74afde2407cc02cd84d495d
servo/components/malloc_size_of/lib.rs
servo/components/style/stylesheets/document_rule.rs
servo/components/style/stylesheets/media_rule.rs
servo/components/style/stylesheets/mod.rs
servo/components/style/stylesheets/page_rule.rs
servo/components/style/stylesheets/style_rule.rs
servo/components/style/stylesheets/stylesheet.rs
servo/components/style/stylesheets/supports_rule.rs
--- a/servo/components/malloc_size_of/lib.rs
+++ b/servo/components/malloc_size_of/lib.rs
@@ -136,41 +136,54 @@ impl MallocSizeOfOps {
 pub trait MallocSizeOf {
     /// Measure the heap usage of all descendant heap-allocated structures, but
     /// not the space taken up by the value itself.
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize;
 }
 
 /// 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;
 }
 
 /// Like `MallocSizeOf`, but with a different name so it cannot be used
 /// accidentally with derive(MallocSizeOf). For use with types like `Rc` and
 /// `Arc` when appropriate (e.g. when measuring a "primary" reference).
 pub trait MallocUnconditionalSizeOf {
     /// Measure the heap usage of all heap-allocated descendant structures, but
     /// not the space taken up by the value itself.
     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).
 pub trait MallocConditionalSizeOf {
     /// Measure the heap usage of all heap-allocated descendant structures, but
     /// not the space taken up by the value itself, and only if that heap usage
     /// hasn't already been measured.
     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)
     }
 }
 
 impl<T: MallocSizeOf> MallocSizeOf for Box<T> {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
@@ -289,19 +302,35 @@ impl<K, V, S> MallocSizeOf for HashMap<K
 
 // XXX: we don't want MallocSizeOf to be defined for Rc and Arc. If negative
 // 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())
+    }
+}
+
 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)
+        }
     }
 }
 
 impl<T: MallocSizeOf> MallocConditionalSizeOf for Arc<T> {
     fn conditional_size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         if ops.have_seen_ptr(self.heap_ptr()) {
             0
         } else {
--- a/servo/components/style/stylesheets/document_rule.rs
+++ b/servo/components/style/stylesheets/document_rule.rs
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! [@document rules](https://www.w3.org/TR/2012/WD-css3-conditional-20120911/#at-document)
 //! initially in CSS Conditional Rules Module Level 3, @document has been postponed to the level 4.
 //! We implement the prefixed `@-moz-document`.
 
 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;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
 use std::fmt;
 use style_traits::{ToCss, ParseError, StyleParseError};
 use stylesheets::CssRules;
 use values::specified::url::SpecifiedUrl;
@@ -29,17 +29,18 @@ pub struct DocumentRule {
     pub source_location: SourceLocation,
 }
 
 impl DocumentRule {
     /// Measure heap usage.
     #[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)
     }
 }
 
 impl ToCssWithGuard for DocumentRule {
     fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
     where W: fmt::Write {
         dest.write_str("@-moz-document ")?;
         self.condition.to_css(dest)?;
--- a/servo/components/style/stylesheets/media_rule.rs
+++ b/servo/components/style/stylesheets/media_rule.rs
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! An [`@media`][media] urle.
 //!
 //! [media]: https://drafts.csswg.org/css-conditional/#at-ruledef-media
 
 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};
 use std::fmt;
 use style_traits::ToCss;
 use stylesheets::CssRules;
 
 /// An [`@media`][media] urle.
@@ -29,17 +29,18 @@ pub struct MediaRule {
     pub source_location: SourceLocation,
 }
 
 impl MediaRule {
     /// Measure heap usage.
     #[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)
     }
 }
 
 impl ToCssWithGuard for MediaRule {
     // Serialization of MediaRule is not specced.
     // https://drafts.csswg.org/cssom/#serialize-a-css-rule CSSMediaRule
     fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
     where W: fmt::Write {
--- a/servo/components/style/stylesheets/mod.rs
+++ b/servo/components/style/stylesheets/mod.rs
@@ -21,17 +21,17 @@ mod rules_iterator;
 mod style_rule;
 mod stylesheet;
 pub mod supports_rule;
 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};
 use std::fmt;
 use style_traits::PARSING_MODE_DEFAULT;
 
 pub use self::counter_style_rule::CounterStyleRule;
 pub use self::document_rule::DocumentRule;
@@ -115,31 +115,36 @@ impl CssRule {
             // may be added later.
 
             CssRule::Namespace(_) => 0,
 
             // We don't need to measure ImportRule::stylesheet because we measure
             // 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),
         }
     }
 }
 
 #[allow(missing_docs)]
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum CssRuleType {
     // https://drafts.csswg.org/cssom/#the-cssrule-interface
--- a/servo/components/style/stylesheets/page_rule.rs
+++ b/servo/components/style/stylesheets/page_rule.rs
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! A [`@page`][page] rule.
 //!
 //! [page]: https://drafts.csswg.org/css2/page.html#page-box
 
 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};
 use std::fmt;
 use style_traits::ToCss;
 
 /// A [`@page`][page] rule.
 ///
@@ -32,17 +32,17 @@ pub struct PageRule {
     pub source_location: SourceLocation,
 }
 
 impl PageRule {
     /// Measure heap usage.
     #[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)
     }
 }
 
 impl ToCssWithGuard for PageRule {
     /// Serialization of PageRule is not specced, adapted from steps for
     /// StyleRule.
     fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
         where W: fmt::Write,
--- a/servo/components/style/stylesheets/style_rule.rs
+++ b/servo/components/style/stylesheets/style_rule.rs
@@ -1,17 +1,17 @@
 /* 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 http://mozilla.org/MPL/2.0/. */
 
 //! A style rule.
 
 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;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
 use std::fmt;
 use style_traits::ToCss;
 
@@ -53,17 +53,18 @@ impl StyleRule {
         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 += 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
     }
 }
 
 impl ToCssWithGuard for StyleRule {
     /// https://drafts.csswg.org/cssom/#serialize-a-css-rule CSSStyleRule
     fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
--- a/servo/components/style/stylesheets/stylesheet.rs
+++ b/servo/components/style/stylesheets/stylesheet.rs
@@ -5,17 +5,17 @@
 use {Prefix, Namespace};
 use context::QuirksMode;
 use cssparser::{Parser, RuleListParser, ParserInput};
 use error_reporting::{ParseErrorReporter, ContextualParseError};
 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};
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard};
 use std::mem;
 use std::sync::atomic::{AtomicBool, Ordering};
 use style_traits::PARSING_MODE_DEFAULT;
@@ -117,17 +117,18 @@ impl StylesheetContents {
             &self.rules.read_with(guard)
         )
     }
 
     /// Measure heap usage.
     #[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)
     }
 }
 
 impl DeepCloneWithLock for StylesheetContents {
     fn deep_clone_with_lock(
         &self,
         lock: &SharedRwLock,
         guard: &SharedRwLockReadGuard,
--- a/servo/components/style/stylesheets/supports_rule.rs
+++ b/servo/components/style/stylesheets/supports_rule.rs
@@ -2,17 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! [@supports rules](https://drafts.csswg.org/css-conditional-3/#at-supports)
 
 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;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
 use std::fmt;
 use style_traits::{ToCss, ParseError, StyleParseError};
 use stylesheets::{CssRuleType, CssRules};
@@ -32,17 +32,18 @@ pub struct SupportsRule {
     pub source_location: SourceLocation,
 }
 
 impl SupportsRule {
     /// Measure heap usage.
     #[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)
     }
 }
 
 impl ToCssWithGuard for SupportsRule {
     fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
     where W: fmt::Write {
         dest.write_str("@supports ")?;
         self.condition.to_css(dest)?;