Last Comment Bug 747509 - nsDocument::DocSizeOfExcludingThis should include ID and styled links tables
: nsDocument::DocSizeOfExcludingThis should include ID and styled links tables
Status: RESOLVED FIXED
[MemShrink:P2]
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Nathan Froyd [:froydnj]
:
Mentors:
http://www.whatwg.org/specs/web-apps/...
Depends on: 762766
Blocks: DarkMatter
  Show dependency treegraph
 
Reported: 2012-04-20 12:57 PDT by Nathan Froyd [:froydnj]
Modified: 2012-06-26 01:59 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (759 bytes, patch)
2012-06-05 11:59 PDT, Nathan Froyd [:froydnj]
n.nethercote: feedback+
Details | Diff | Review
patch (1.22 KB, patch)
2012-06-07 17:33 PDT, Nathan Froyd [:froydnj]
n.nethercote: review+
Details | Diff | Review
followup for hash keys (2.73 KB, patch)
2012-06-18 09:42 PDT, Nathan Froyd [:froydnj]
n.nethercote: review+
Details | Diff | Review

Description Nathan Froyd [:froydnj] 2012-04-20 12:57:23 PDT
Running DMD on a browser viewing the HTML5 single-page spec turns up:

==28808== Unreported: 1 block(s) in record 7 of 15608
==28808==  1,052,672 bytes (1,048,580 requested / 4,092 slop)
==28808==  0.47% of the heap (17.11% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A1C9: moz_malloc (mozalloc.cpp:97)
==28808==    by 0x9C4FCED: PL_DHashAllocTable (pldhash.cpp:117)
==28808==    by 0x9C50710: ChangeTable(PLDHashTable*, int) (pldhash.cpp:560)
==28808==    by 0x9C50AB5: PL_DHashTableOperate (pldhash.cpp:645)
==28808==    by 0x8B62E3C: nsTHashtable<nsIdentifierMapEntry>::PutEntry(nsAString_internal const&) (nsTHashtable.h:198)
==28808==    by 0x8B46FDE: nsDocument::AddToIdTable(mozilla::dom::Element*, nsIAtom*) (nsDocument.cpp:2646)
==28808==    by 0x8B9A6D5: nsGenericElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) (nsGenericElement.cpp:3227)
==28808==    by 0x8D1864D: nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) (nsGenericHTMLElement.cpp:1130)
==28808==    by 0x8B9C093: nsINode::doInsertChildAt(nsIContent*, unsigned int, bool, nsAttrAndChildArray&) (nsGenericElement.cpp:3802)
==28808==    by 0x8B9B987: nsGenericElement::InsertChildAt(nsIContent*, unsigned int, bool) (nsGenericElement.cpp:3735)
==28808==    by 0x875A73E: nsINode::AppendChildTo(nsIContent*, bool) (nsINode.h:563)
==28808== 
==28808== Unreported: 1 block(s) in record 8 of 15608
==28808==  1,052,672 bytes (1,048,580 requested / 4,092 slop)
==28808==  0.47% of the heap (17.59% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A1C9: moz_malloc (mozalloc.cpp:97)
==28808==    by 0x9C4FCED: PL_DHashAllocTable (pldhash.cpp:117)
==28808==    by 0x9C50710: ChangeTable(PLDHashTable*, int) (pldhash.cpp:560)
==28808==    by 0x9C50AB5: PL_DHashTableOperate (pldhash.cpp:645)
==28808==    by 0x8B64D80: nsTHashtable<nsPtrHashKey<mozilla::dom::Link> >::PutEntry(mozilla::dom::Link*) (nsTHashtable.h:198)
==28808==    by 0x8B57F04: nsDocument::AddStyleRelevantLink(mozilla::dom::Link*) (nsDocument.cpp:7550)
==28808==    by 0x8C19402: mozilla::dom::Link::LinkState() const (Link.cpp:137)
==28808==    by 0x8B59CBA: EnumeratePendingLinkUpdates(nsPtrHashKey<mozilla::dom::Link>*, void*) (nsDocument.cpp:8087)
==28808==    by 0x8B691F5: nsTHashtable<nsPtrHashKey<mozilla::dom::Link> >::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, 
unsigned int, void*) (nsTHashtable.h:500)
==28808==    by 0x9C50F88: PL_DHashTableEnumerate (pldhash.cpp:750)
==28808==    by 0x8B64F62: nsTHashtable<nsPtrHashKey<mozilla::dom::Link> >::EnumerateEntries(PLDHashOperator (*)(nsPtrHashKey<mozilla::dom::Link>*, void*), void*) (nsTHashtable.h:251)

So those might be useful to keep track of, possibly along with the name table.
Comment 1 Nathan Froyd [:froydnj] 2012-06-05 11:59:15 PDT
Created attachment 630255 [details] [diff] [review]
patch

These came up again when viewing a fairly small merge to m-c:

==18974== Unreported: 1 block(s) in record 1 of 13741
==18974==  4,198,400 bytes (4,194,308 requested / 4,092 slop)
==18974==  3.29% of the heap (3.29% cumulative unreported)
==18974==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==18974==    by 0x6A5A1C9: moz_malloc (mozalloc.cpp:64)
==18974==    by 0x9D95553: PL_DHashAllocTable (pldhash.cpp:82)
==18974==    by 0x9D95F76: ChangeTable(PLDHashTable*, int) (pldhash.cpp:525)
==18974==    by 0x9D9631B: PL_DHashTableOperate (pldhash.cpp:610)
==18974==    by 0x8BAC66E: nsTHashtable<nsIdentifierMapEntry>::PutEntry(nsAString_internal const&, mozilla::fallible_t const&) (nsTHashtable.h:184)
==18974==    by 0x8BA7B42: nsTHashtable<nsIdentifierMapEntry>::PutEntry(nsAString_internal const&) (nsTHashtable.h:170)
==18974==    by 0x8B8B302: nsDocument::AddToIdTable(mozilla::dom::Element*, nsIAtom*) (nsDocument.cpp:2595)
==18974==    by 0x8BE0831: nsGenericElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) (nsGenericElement.cpp:3186)
==18974==    by 0x8D904FD: nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) (nsGenericHTMLElement.cpp:1700)
==18974==    by 0x8DAC277: nsHTMLAnchorElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) (nsHTMLAnchorElement.cpp:229)
==18974==    by 0x8BE21EF: nsINode::doInsertChildAt(nsIContent*, unsigned int, bool, nsAttrAndChildArray&) (nsGenericElement.cpp:3761)
==18974== 
==18974== Unreported: 3 block(s) in record 5 of 13741
==18974==  1,073,152 bytes (1,060,876 requested / 12,276 slop)
==18974==  0.84% of the heap (5.92% cumulative unreported)
==18974==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==18974==    by 0x6A5A1C9: moz_malloc (mozalloc.cpp:64)
==18974==    by 0x9D95553: PL_DHashAllocTable (pldhash.cpp:82)
==18974==    by 0x9D95F76: ChangeTable(PLDHashTable*, int) (pldhash.cpp:525)
==18974==    by 0x9D9631B: PL_DHashTableOperate (pldhash.cpp:610)
==18974==    by 0x8BADD16: nsTHashtable<nsPtrHashKey<mozilla::dom::Link> >::PutEntry(mozilla::dom::Link*, mozilla::fallible_t const&) (nsTHashtable.h:184)
==18974==    by 0x8BA9A7C: nsTHashtable<nsPtrHashKey<mozilla::dom::Link> >::PutEntry(mozilla::dom::Link*) (nsTHashtable.h:170)
==18974==    by 0x8B9BF6A: nsDocument::AddStyleRelevantLink(mozilla::dom::Link*) (nsDocument.cpp:7480)
==18974==    by 0x8C5FECB: mozilla::dom::Link::LinkState() const (Link.cpp:104)
==18974==    by 0x8B9DD20: EnumeratePendingLinkUpdates(nsPtrHashKey<mozilla::dom::Link>*, void*) (nsDocument.cpp:8017)
==18974==    by 0x8BADE35: nsTHashtable<nsPtrHashKey<mozilla::dom::Link> >::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) (nsTHashtable.h:486)
==18974==    by 0x9D967EE: PL_DHashTableEnumerate (pldhash.cpp:715)

(4% of heap!) so I thought it'd be worth fixing that.

I wasn't sure whether to assign them their own subnodes in about:memory, or lump them into the DOM.  Is there a guideline for when things get their own subtree?
Comment 2 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-06-06 00:06:34 PDT
Comment on attachment 630255 [details] [diff] [review]
patch

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

f=me.  Please run DMD to check there's no double-counting.  As for splitting off to separate sub-trees, there are no rules.  Do it if you think it's worthwhile.  But I think these entries will usually be small.
Comment 3 Nathan Froyd [:froydnj] 2012-06-07 17:33:14 PDT
Created attachment 631227 [details] [diff] [review]
patch

Yeah, probably small enough.  We'll just let them sit in dom/other.
Comment 4 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-06-07 20:39:41 PDT
Comment on attachment 631227 [details] [diff] [review]
patch

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

Looks good, 

BTW, I hope/assume you're doing DMD runs each time you improve the covereage to (a) make sure the memory you think is covered really *is* covered, and (b) there's no double-counting.
Comment 5 Nathan Froyd [:froydnj] 2012-06-08 06:48:00 PDT
(In reply to Nicholas Nethercote [:njn] from comment #4)
> BTW, I hope/assume you're doing DMD runs each time you improve the covereage
> to (a) make sure the memory you think is covered really *is* covered, and
> (b) there's no double-counting.

Yup, did that.  Pushed as:

https://hg.mozilla.org/integration/mozilla-inbound/rev/885395287478

Going to leave open, as bug 762766 needs to get fixed before we can properly account for all the id table memory.
Comment 6 Ed Morley [:emorley] 2012-06-08 13:54:45 PDT
https://hg.mozilla.org/mozilla-central/rev/885395287478
Comment 7 Nathan Froyd [:froydnj] 2012-06-18 09:42:21 PDT
Created attachment 634077 [details] [diff] [review]
followup for hash keys

After jlebar pointed out my mental block in bug 762766, writing this patch became much easier.
Comment 9 Ed Morley [:emorley] 2012-06-26 01:59:02 PDT
https://hg.mozilla.org/mozilla-central/rev/542074e589eb

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