Last Comment Bug 729008 - Add memory reporter for FramePropertyTable
: Add memory reporter for FramePropertyTable
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla13
Assigned To: Nicholas Nethercote [:njn] (on vacation until July 11)
:
Mentors:
Depends on: DMD
Blocks: DarkMatter
  Show dependency treegraph
 
Reported: 2012-02-20 21:39 PST by Nicholas Nethercote [:njn] (on vacation until July 11)
Modified: 2012-03-07 02:52 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (11.65 KB, patch)
2012-02-20 21:39 PST, Nicholas Nethercote [:njn] (on vacation until July 11)
bzbarsky: review+
Details | Diff | Review

Description Nicholas Nethercote [:njn] (on vacation until July 11) 2012-02-20 21:39:32 PST
Created attachment 599048 [details] [diff] [review]
patch

DMD pointed me at FramePropertyTable.  On a workload of 14 wikipedia tabs, the this patch improves coverage of layout memory by about 1.3MB of additional layout memory.

Other minor changes:
- Renamed "layout/styledata" reports as "layout/style-sets".
- |const|-ifies nsTHashTable::SizeOfIncludingThis
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-02-21 09:01:16 PST
Comment on attachment 599048 [details] [diff] [review]
patch

r=me

It's interesting that we're getting 1.3MB of proptable storage but not that much for the values... Might be worth seeing whether we should do some reporting for the values too at some point.
Comment 2 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-02-21 16:26:09 PST
> It's interesting that we're getting 1.3MB of proptable storage but not that
> much for the values... Might be worth seeing whether we should do some
> reporting for the values too at some point.

That's what this comment is about:

+      // We don't need to measure mProperty because it always points to static
+      // memory.  As for mValue:  if it's a single value we can't measure it,
+      // because the type is opaque;  if it's an array, we measure the array
+      // storage, but we can't measure the individual values, again because
+      // their types are opaque.

However, we have mProperty->mDestructor and mProperty->mDestructorWithFrame;  I could add mProperty->mSizeOfIncludingthis...
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-02-21 17:27:45 PST
Yeah, perhaps as a followup if we see those allocations.
Comment 4 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-03-06 17:28:21 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/878c5c8b8b21
Comment 5 Marco Bonardo [::mak] 2012-03-07 02:52:32 PST
https://hg.mozilla.org/mozilla-central/rev/878c5c8b8b21

Note You need to log in before you can comment on or make changes to this bug.