Skip to content

Commit

Permalink
Auto merge of #17044 - nnethercote:MallocSizeOf, r=emilio
Browse files Browse the repository at this point in the history
Introduce and start using the MallocSizeOf trait.

MallocSizeOf is similar to the existing HeapSizeOf trait from the
heapsize crate. The only difference is that MallocSizeOf's
malloc_size_of_children() function takes an additional MallocSizeOfFn
argument, which is used to measure heap blocks. This extra argument
makes MallocSizeOf match how Gecko's memory measurements work, and is
required for Stylo to integrate with DMD.

The patch also introduces a second trait, MallocSizeOfWithGuard, which
is much the same as MallocSizeOf, but with a |guard| argument for the
global style lock.

Finally, the patch uses the new traits to measure a small amount of
Stylo's memory usage.

---
<!-- 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 code is only for Gecko integration.

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17044)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed May 29, 2017
2 parents 4ec2e8b + 6d5b124 commit aca0943
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 5 deletions.
6 changes: 6 additions & 0 deletions components/style/gecko/generated/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use gecko_bindings::structs::FontSizePrefs;
use gecko_bindings::structs::GeckoFontMetrics;
use gecko_bindings::structs::IterationCompositeOperation;
use gecko_bindings::structs::Keyframe;
use gecko_bindings::structs::MallocSizeOf;
use gecko_bindings::structs::ServoBundledURI;
use gecko_bindings::structs::ServoElementSnapshot;
use gecko_bindings::structs::ServoElementSnapshotTable;
Expand Down Expand Up @@ -1765,6 +1766,11 @@ extern "C" {
pub fn Servo_StyleSheet_Clone(sheet: RawServoStyleSheetBorrowed)
-> RawServoStyleSheetStrong;
}
extern "C" {
pub fn Servo_StyleSheet_SizeOfIncludingThis(malloc_size_of: MallocSizeOf,
sheet: RawServoStyleSheetBorrowed)
-> usize;
}
extern "C" {
pub fn Servo_StyleSet_Init(pres_context: RawGeckoPresContextOwned)
-> RawServoStyleSetOwned;
Expand Down
13 changes: 13 additions & 0 deletions components/style/properties/declaration_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::fmt;
use std::slice::Iter;
use style_traits::ToCss;
use stylesheets::{CssRuleType, Origin, UrlExtraData};
use stylesheets::{MallocSizeOf, MallocSizeOfFn};
use super::*;
#[cfg(feature = "gecko")] use properties::animated_properties::AnimationValueMap;

Expand Down Expand Up @@ -46,6 +47,12 @@ pub enum Importance {
Important,
}

impl MallocSizeOf for Importance {
fn malloc_size_of_children(&self, _malloc_size_of: MallocSizeOfFn) -> usize {
0
}
}

impl Importance {
/// Return whether this is an important declaration.
pub fn important(self) -> bool {
Expand All @@ -70,6 +77,12 @@ pub struct PropertyDeclarationBlock {
longhands: LonghandIdSet,
}

impl MallocSizeOf for PropertyDeclarationBlock {
fn malloc_size_of_children(&self, malloc_size_of: MallocSizeOfFn) -> usize {
self.declarations.malloc_size_of_children(malloc_size_of)
}
}

/// Iterator for PropertyDeclaration to be generated from PropertyDeclarationBlock.
#[derive(Clone)]
pub struct PropertyDeclarationIterator<'a> {
Expand Down
10 changes: 9 additions & 1 deletion components/style/properties/properties.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use properties::animated_properties::TransitionProperty;
#[cfg(feature = "servo")] use servo_config::prefs::PREFS;
use shared_lock::StylesheetGuards;
use style_traits::{HasViewportPercentage, ToCss};
use stylesheets::{CssRuleType, Origin, UrlExtraData};
use stylesheets::{CssRuleType, MallocSizeOf, MallocSizeOfFn, Origin, UrlExtraData};
#[cfg(feature = "servo")] use values::Either;
use values::computed;
use cascade_info::CascadeInfo;
Expand Down Expand Up @@ -1171,6 +1171,14 @@ impl ToCss for PropertyDeclaration {
% endif
</%def>

impl MallocSizeOf for PropertyDeclaration {
fn malloc_size_of_children(&self, _malloc_size_of: MallocSizeOfFn) -> usize {
// The variants of PropertyDeclaration mostly (entirely?) contain
// scalars, so this is reasonable.
0
}
}

impl PropertyDeclaration {
/// Given a property declaration, return the property declaration id.
pub fn id(&self) -> PropertyDeclarationId {
Expand Down
99 changes: 98 additions & 1 deletion components/style/stylesheets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ use shared_lock::{SharedRwLock, Locked, ToCssWithGuard, SharedRwLockReadGuard};
use std::borrow::Borrow;
use std::cell::Cell;
use std::fmt;
use std::mem::align_of;
use std::os::raw::c_void;
use std::sync::atomic::{AtomicBool, Ordering};
use str::starts_with_ignore_ascii_case;
use style_traits::ToCss;
Expand Down Expand Up @@ -99,6 +101,57 @@ pub struct Namespaces {
pub prefixes: FnvHashMap<Prefix , Namespace>,
}

/// Like gecko_bindings::structs::MallocSizeOf, but without the Option<> wrapper. Note that
/// functions of this type should not be called via do_malloc_size_of(), rather than directly.
pub type MallocSizeOfFn = unsafe extern "C" fn(ptr: *const c_void) -> usize;

/// Call malloc_size_of on ptr, first checking that the allocation isn't empty.
pub unsafe fn do_malloc_size_of<T>(malloc_size_of: MallocSizeOfFn, ptr: *const T) -> usize {
if ptr as usize <= align_of::<T>() {
0
} else {
malloc_size_of(ptr as *const c_void)
}
}

/// Trait for measuring the size of heap data structures.
pub trait MallocSizeOf {
/// Measure the size of any heap-allocated structures that hang off this value, but not the
/// space taken up by the value itself.
fn malloc_size_of_children(&self, malloc_size_of: MallocSizeOfFn) -> usize;
}

/// Like MallocSizeOf, but operates with the global SharedRwLockReadGuard locked.
pub trait MallocSizeOfWithGuard {
/// Like MallocSizeOf::malloc_size_of_children, but with a |guard| argument.
fn malloc_size_of_children(&self, guard: &SharedRwLockReadGuard,
malloc_size_of: MallocSizeOfFn) -> usize;
}

impl<A: MallocSizeOf, B: MallocSizeOf> MallocSizeOf for (A, B) {
fn malloc_size_of_children(&self, malloc_size_of: MallocSizeOfFn) -> usize {
self.0.malloc_size_of_children(malloc_size_of) +
self.1.malloc_size_of_children(malloc_size_of)
}
}

impl<T: MallocSizeOf> MallocSizeOf for Vec<T> {
fn malloc_size_of_children(&self, malloc_size_of: MallocSizeOfFn) -> usize {
self.iter().fold(
unsafe { do_malloc_size_of(malloc_size_of, self.as_ptr()) },
|n, elem| n + elem.malloc_size_of_children(malloc_size_of))
}
}

impl<T: MallocSizeOfWithGuard> MallocSizeOfWithGuard for Vec<T> {
fn malloc_size_of_children(&self, guard: &SharedRwLockReadGuard,
malloc_size_of: MallocSizeOfFn) -> usize {
self.iter().fold(
unsafe { do_malloc_size_of(malloc_size_of, self.as_ptr()) },
|n, elem| n + elem.malloc_size_of_children(guard, malloc_size_of))
}
}

/// A list of CSS rules.
#[derive(Debug)]
pub struct CssRules(pub Vec<CssRule>);
Expand All @@ -119,6 +172,13 @@ impl CssRules {
}
}

impl MallocSizeOfWithGuard for CssRules {
fn malloc_size_of_children(&self, guard: &SharedRwLockReadGuard,
malloc_size_of: MallocSizeOfFn) -> usize {
self.0.malloc_size_of_children(guard, malloc_size_of)
}
}

#[allow(missing_docs)]
pub enum RulesMutateError {
Syntax,
Expand Down Expand Up @@ -254,9 +314,9 @@ impl CssRulesHelpers for Arc<Locked<CssRules>> {

Ok(new_rule)
}

}


/// The structure servo uses to represent a stylesheet.
#[derive(Debug)]
pub struct Stylesheet {
Expand All @@ -281,6 +341,13 @@ pub struct Stylesheet {
pub quirks_mode: QuirksMode,
}

impl MallocSizeOfWithGuard for Stylesheet {
fn malloc_size_of_children(&self, guard: &SharedRwLockReadGuard,
malloc_size_of: MallocSizeOfFn) -> usize {
// Measurement of other fields may be added later.
self.rules.read_with(guard).malloc_size_of_children(guard, malloc_size_of)
}
}

/// This structure holds the user-agent and user stylesheets.
pub struct UserAgentStylesheets {
Expand Down Expand Up @@ -315,6 +382,28 @@ pub enum CssRule {
Document(Arc<Locked<DocumentRule>>),
}

impl MallocSizeOfWithGuard for CssRule {
fn malloc_size_of_children(&self, guard: &SharedRwLockReadGuard,
malloc_size_of: MallocSizeOfFn) -> usize {
match *self {
CssRule::Style(ref lock) => {
lock.read_with(guard).malloc_size_of_children(guard, malloc_size_of)
},
// Measurement of these fields may be added later.
CssRule::Import(_) => 0,
CssRule::Media(_) => 0,
CssRule::FontFace(_) => 0,
CssRule::CounterStyle(_) => 0,
CssRule::Keyframes(_) => 0,
CssRule::Namespace(_) => 0,
CssRule::Viewport(_) => 0,
CssRule::Supports(_) => 0,
CssRule::Page(_) => 0,
CssRule::Document(_) => 0,
}
}
}

#[allow(missing_docs)]
#[derive(PartialEq, Eq, Copy, Clone)]
pub enum CssRuleType {
Expand Down Expand Up @@ -825,6 +914,14 @@ pub struct StyleRule {
pub source_location: SourceLocation,
}

impl MallocSizeOfWithGuard for StyleRule {
fn malloc_size_of_children(&self, guard: &SharedRwLockReadGuard,
malloc_size_of: MallocSizeOfFn) -> usize {
// Measurement of other fields may be added later.
self.block.read_with(guard).malloc_size_of_children(malloc_size_of)
}
}

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
Expand Down
16 changes: 13 additions & 3 deletions ports/geckolib/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ use style::gecko_bindings::structs::{nsCSSFontFaceRule, nsCSSCounterStyleRule};
use style::gecko_bindings::structs::{nsRestyleHint, nsChangeHint};
use style::gecko_bindings::structs::IterationCompositeOperation;
use style::gecko_bindings::structs::Loader;
use style::gecko_bindings::structs::MallocSizeOf;
use style::gecko_bindings::structs::RawGeckoPresContextOwned;
use style::gecko_bindings::structs::ServoElementSnapshotTable;
use style::gecko_bindings::structs::StyleRuleInclusion;
Expand Down Expand Up @@ -98,9 +99,9 @@ use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards,
use style::string_cache::Atom;
use style::style_adjuster::StyleAdjuster;
use style::stylearc::Arc;
use style::stylesheets::{CssRule, CssRules, CssRuleType, CssRulesHelpers};
use style::stylesheets::{ImportRule, KeyframesRule, MediaRule, NamespaceRule, Origin};
use style::stylesheets::{PageRule, Stylesheet, StyleRule, SupportsRule, DocumentRule};
use style::stylesheets::{CssRule, CssRules, CssRuleType, CssRulesHelpers, DocumentRule};
use style::stylesheets::{ImportRule, KeyframesRule, MallocSizeOfWithGuard, MediaRule};
use style::stylesheets::{NamespaceRule, Origin, PageRule, Stylesheet, StyleRule, SupportsRule};
use style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
use style::stylist::RuleInclusion;
use style::supports::parse_condition_or_declaration;
Expand Down Expand Up @@ -801,6 +802,15 @@ pub extern "C" fn Servo_StyleSheet_Clone(raw_sheet: RawServoStyleSheetBorrowed)
Arc::new(sheet.as_ref().clone()).into_strong()
}

#[no_mangle]
pub extern "C" fn Servo_StyleSheet_SizeOfIncludingThis(malloc_size_of: MallocSizeOf,
sheet: RawServoStyleSheetBorrowed) -> usize {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let malloc_size_of = malloc_size_of.unwrap();
Stylesheet::as_arc(&sheet).malloc_size_of_children(&guard, malloc_size_of)
}

fn read_locked_arc<T, R, F>(raw: &<Locked<T> as HasFFI>::FFIType, func: F) -> R
where Locked<T>: HasArcFFI, F: FnOnce(&T) -> R
{
Expand Down

0 comments on commit aca0943

Please sign in to comment.