Consider memory-reporting more things on DocumentOrShadowRoot

RESOLVED FIXED in Firefox 63

Status

()

defect
RESOLVED FIXED
Last year
5 months ago

People

(Reporter: bzbarsky, Assigned: emilio)

Tracking

(Blocks 1 bug)

unspecified
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

With the memory reporting DocumentOrShadowRoot does after bug 1486623, we should consider moving more things into DocumentOrShadowRoot::SizeOfExcludingThis.

As a specific example, nsDocument::DocAddSizeOfExcludingThis has:

  aWindowSizes.mDOMOtherSize +=
    mIdentifierMap.SizeOfExcludingThis(aWindowSizes.mState.mMallocSizeOf);

and we should move that up into DocumentOrShadowRoot.  I am looking at a DMD log shows this (due to elements with ids in an <svg:use> subtree):

Unreported {
  2,201 blocks in heap block record 11 of 6,988
  2,253,824 bytes (1,408,640 requested / 845,184 slop)
  Individual block sizes: 1,024 x 2,201
  0.47% of the heap (20.64% cumulative)
  1.27% of unreported (56.08% cumulative)
  Allocated at {
    #01: replace_calloc(unsigned long, unsigned long) (DMD.cpp:1282, in libmozglue.dylib)
    #02: PLDHashTable::Add(void const*, std::nothrow_t const&) (PLDHashTable.h:245, in XUL)
    #03: PLDHashTable::Add(void const*) (PLDHashTable.cpp:619, in XUL)
    #04: mozilla::dom::ShadowRoot::AddToIdTable(mozilla::dom::Element*, nsAtom*) (nsTHashtable.h:156, in XUL)
    #05: mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*) (Element.cpp:0, in XUL)
    #06: nsSVGElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*) (nsSVGElement.cpp:240, in XUL)
    #07: nsINode::InsertChildBefore(nsIContent*, nsIContent*, bool) (nsINode.cpp:1412, in XUL)
    #08: mozilla::dom::SVGUseElement::UpdateShadowTree() (RefPtr.h:42, in XUL)
  }
}

Of course it's worth figuring out whether this hashtable ever gets used in the <svg:use> case...
I guess mDOMStyleSheets is the only other thing on DocumentOrShadowRoot that might need memory reporting...
Yeah, can get to this tomorrow.

The identifier map is actually used, it's needed to efficiently lookup idrefs in Shadow DOM subtrees... Though for <svg:use> we jump over them so we could potentially avoid doing it.
Flags: needinfo?(emilio)
Given it doesn't have it in document, I'll just move the identifier map for now.
Assignee: nobody → emilio
Flags: needinfo?(emilio)
Comment on attachment 9005060 [details]
Measure memory usage of the identifier map in ShadowRoot as well.

Olli Pettay [:smaug] has approved the revision.
Attachment #9005060 - Flags: review+
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/48f88e237b4e
Measure memory usage of the identifier map in ShadowRoot as well. r=smaug
https://hg.mozilla.org/mozilla-central/rev/48f88e237b4e
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.