Last Comment Bug 752952 - nsIDocument's PropertyTables should be accounted for in about:memory
: nsIDocument's PropertyTables should be accounted for in about:memory
Status: RESOLVED FIXED
[MemShrink:P2]
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Nathan Froyd [:froydnj]
:
Mentors:
http://www.whatwg.org/specs/web-apps/...
Depends on:
Blocks: DarkMatter 810712
  Show dependency treegraph
 
Reported: 2012-05-08 09:46 PDT by Nathan Froyd [:froydnj]
Modified: 2012-11-11 14:09 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (7.36 KB, patch)
2012-06-14 21:15 PDT, Nathan Froyd [:froydnj]
no flags Details | Diff | Splinter Review
patch (7.36 KB, patch)
2012-06-14 21:18 PDT, Nathan Froyd [:froydnj]
n.nethercote: review+
bugs: feedback+
Details | Diff | Splinter Review

Description Nathan Froyd [:froydnj] 2012-05-08 09:46:13 PDT
On the HTML5 single-page spec, post-bug 747508, the single biggest offender is:

==24328== Unreported: 1 block(s) in record 1 of 15374
==24328==  3,149,824 bytes (3,145,732 requested / 4,092 slop)
==24328==  2.77% of the heap (2.77% cumulative unreported)
==24328==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==24328==    by 0x68521B9: moz_malloc (mozalloc.cpp:97)
==24328==    by 0x9A4407B: PL_DHashAllocTable (pldhash.cpp:117)
==24328==    by 0x9A44A9E: ChangeTable(PLDHashTable*, int) (pldhash.cpp:560)
==24328==    by 0x9A44E43: PL_DHashTableOperate (pldhash.cpp:645)
==24328==    by 0x89AD7AA: nsPropertyTable::SetPropertyInternal(nsPropertyOwner, nsIAtom*, void*, void (*)(void*, nsIAtom*, void*, void*), void*, bool, void**) (nsPropertyTable.cpp:260)
==24328==    by 0x8968FB5: nsPropertyTable::SetProperty(nsPropertyOwner, nsIAtom*, void*, void (*)(void*, nsIAtom*, void*, void*), void*, bool, void**) (nsPropertyTable.h:137)
==24328==    by 0x896CC09: nsINode::SetProperty(unsigned short, nsIAtom*, void*, void (*)(void*, nsIAtom*, void*, void*), bool, void**) (nsGenericElement.cpp:231)
==24328==    by 0x854C598: nsINode::SetProperty(nsIAtom*, void*, void (*)(void*, nsIAtom*, void*, void*), bool) (nsINode.h:636)
==24328==    by 0x86EE60F: nsTextFrame::GetInFlowContentLength() (nsTextFrameThebes.cpp:635)
==24328==    by 0x870280E: nsTextFrame::ReflowText(nsLineLayout&, int, nsRenderingContext*, bool, nsHTMLReflowMetrics&, unsigned int&) (nsTextFrameThebes.cpp:7261)
==24328==    by 0x86C2FA5: nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) (nsLineLayout.cpp:872)

Fixing this is just a matter of modifying nsIDocument::DocSizeOfExcludingThis and adding in mPropertyTable and friends to the appropriate places.
Comment 1 Nathan Froyd [:froydnj] 2012-06-14 21:15:25 PDT
Created attachment 633390 [details] [diff] [review]
patch

DMD-checked, etc.

Ambivalent if this should get its own node or be rolled into the dom/ blob.  I lean towards splitting it out because the DMD stacks make it appear as though layout is using it quite a bit, so it doesn't make much sense putting it in one or the other.  Olli, do you have an opinion on this?
Comment 2 Nathan Froyd [:froydnj] 2012-06-14 21:18:00 PDT
Created attachment 633392 [details] [diff] [review]
patch

Here, let me not screw up the paths...
Comment 3 Nicholas Nethercote [:njn] 2012-06-14 22:08:37 PDT
Comment on attachment 633392 [details] [diff] [review]
patch

Review of attachment 633392 [details] [diff] [review]:
-----------------------------------------------------------------

Splitting it out seems fine;  we can revisit later if necessary.

::: dom/base/nsWindowMemoryReporter.cpp
@@ +332,5 @@
>    REPORT("window-objects/dom/comment-nodes", windowTotalSizes.mDOMCommentNodes,
>           "Memory used for DOM comment nodes within windows. "
>           "This is the sum of all windows' 'dom/comment-nodes' numbers.");
>  
> +  REPORT("window-objects//property-tables",

Double slash.  Did you attach the same patch again?  r=me with that fixed.
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-06-16 06:52:45 PDT
https://hg.mozilla.org/mozilla-central/rev/9c09642d4cd0

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