nsIDocument's PropertyTables should be accounted for in about:memory

RESOLVED FIXED in mozilla16

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla16
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
Whiteboard: [MemShrink]
Whiteboard: [MemShrink] → [MemShrink:P2]
(Assignee)

Comment 1

5 years ago
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?
Attachment #633390 - Flags: review?(n.nethercote)
Attachment #633390 - Flags: feedback?(bugs)
(Assignee)

Comment 2

5 years ago
Created attachment 633392 [details] [diff] [review]
patch

Here, let me not screw up the paths...
Attachment #633390 - Attachment is obsolete: true
Attachment #633390 - Flags: review?(n.nethercote)
Attachment #633390 - Flags: feedback?(bugs)
Attachment #633392 - Flags: review?(n.nethercote)
Attachment #633392 - Flags: feedback?(bugs)
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.
Attachment #633392 - Flags: review?(n.nethercote) → review+

Updated

5 years ago
Attachment #633392 - Flags: feedback?(bugs) → feedback+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c09642d4cd0
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/9c09642d4cd0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16

Updated

5 years ago
Blocks: 810712
You need to log in before you can comment on or make changes to this bug.