Closed Bug 663271 Opened 9 years ago Closed 8 years ago
Add memory reporters for the DOM
There are no memory reporters for the DOM. There should be, because DOM structures can (presumably) grow quite large. jst told me there used to be a SizeOf method on DOM objects, but it was never very reliable. Hopefully now that about:memory is getting more usage, any feature like that will get more use and so be less likely to rot. FWIW, there are two basic models for implementing memory reporters: - The first is to have some global counter, and to increment/decrement it every time an allocation/deallocation of the appropriate kind happens. - The second is to have some way of computing the size from scratch on demand, eg. by iterating over all relevant structures. I've been tending towards using the second kind in the JS engine, even though it make about:memory a little slower, because they seem less error-prone -- we had some counters of the first kind that ended up reporting ridiculous numbers because the code changed over time and the increment/decrement points weren't added/removed/changed as necessary (see bug 648490).
We want to do the second, I think. One issue with SizeOf is that it tended to not get updated when members got added... Maybe we'll do better this time.
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
The cycle collector actually gets the size of the object corresponding to each object in its graph (well, more or less...). We can just get the total size of all of the ref counted objects it sees as a very rough estimate of the memory used by the DOM. I haven't tried this patch out yet.
That's missing most of the memory (e.g. the memory needed by attributes, child lists, text in textnodes, etc, etc)...
Yeah, I guess that is true. Ah well. Anyways, when I ran this, it was reporting 2.5megs to 5.5megs for browsing around. This sounds like the cycle collector problem, where fields have to be added to some magic function, but people forget to update it.
FWIW: just a single "explicit/dom" reporter would be a great start here. That would make it obvious whether we need sub-reporters to give more information. If we do need sub-reporters, the multi-reporter interface (bug 666075) might make them a lot easier.
For the first step, do we want to mix content and chrome or just have content?
My first suggestion is to get as much as possible included in the measurement. So chrome and content should be measured. But then, if it's easy to measure them separately (and take a look again at bug 666705) that's probably worth trying.
(In reply to comment #8) > My first suggestion is to get as much as possible included in the > measurement. So chrome and content should be measured. But then, if it's > easy to measure them separately (and take a look again at bug 666705) that's > probably worth trying. I splitting them should not be a problem. And it looks like the bug number your pasted isn't correct...
(In reply to comment #9) > And it looks like the bug number your pasted isn't correct... Sorry, I meant bug 666075.
If no one object, I'm going to use this bug as a tracker. Splitting the patches should make everything easier (from writing the code to the reviews) and hopefully, will let me push different patches without everything being finished (behind a compilation flag or a preference option).
Status: NEW → ASSIGNED
I think making this a tracker is fine. I'd just push things as they come online; the reporter will just end up being more and more accurate...
(In reply to comment #12) > I think making this a tracker is fine. I'd just push things as they come > online; the reporter will just end up being more and more accurate... bz++. That's what I've been doing with the about:memory changes in general.
In bug 669117 comment 5 bz suggested this: > There's a network-related one that's really DOM: XHR. The data retrieved by > XHR is stored in memory in many cases; we should add an XHR memory reporter.
A commenter on my blog (Ahmad) noticed that this URL: http://www.w3.org/2000/10/swap/test/plist/iTunesMusicLibrary.xml results in heap-unclassified going over 60%, and I can reproduce it. Is this something that the DOM reporters will cover? Or is it due to bug 291643? I'm not sure if it deserves its own bug.
> Is this something that the DOM reporters will cover? Or is it due to > bug 291643? "Yes." We have a pretty good idea of what's up there -- lots of DOM nodes.
(In reply to Boris Zbarsky (:bz) from comment #16) > > "Yes." We have a pretty good idea of what's up there -- lots of DOM nodes. With DOM reporters enabled, here's what I get: 293.10 MB (100.0%) -- explicit ├──160.33 MB (54.70%) -- heap-unclassified ├───81.16 MB (27.69%) -- layout │ ├──80.55 MB (27.48%) -- arenas │ └───0.61 MB (00.21%) -- (1 omitted) ├───23.96 MB (08.18%) -- js │ ├──11.14 MB (03.80%) -- compartment([System Principal], 0x7fbc24d5a000) │ │ ├───6.29 MB (02.15%) -- gc-heap │ │ │ ├──2.97 MB (01.01%) -- objects │ │ │ ├──2.09 MB (00.71%) -- shapes │ │ │ └──1.23 MB (00.42%) -- (5 omitted) │ │ ├───2.69 MB (00.92%) -- (7 omitted) │ │ └───2.16 MB (00.74%) -- scripts │ ├───8.00 MB (02.73%) -- stack │ ├───2.64 MB (00.90%) -- (7 omitted) │ └───2.18 MB (00.74%) -- gc-heap-chunk-dirty-unused ├───21.50 MB (07.34%) -- dom ├────4.75 MB (01.62%) -- storage │ └──4.75 MB (01.62%) -- sqlite │ ├──2.42 MB (00.83%) -- (10 omitted) │ └──2.33 MB (00.79%) -- places.sqlite │ ├──2.02 MB (00.69%) -- cache-used  │ └──0.31 MB (00.10%) -- (2 omitted) └────1.39 MB (00.48%) -- (3 omitted) So, DOM isn't much of it -- only 21.5MB. And that's after I fixed the nsPresArena waste in bug 678422. I'll have to run DMD on this one, but I'm out of time this week. It may end up being similar to the issues in bug 678376 comment 13.
> So, DOM isn't much of it -- only 21.5MB. The DOM reporter only reports explicit (non-anonymous) DOM elements at the moment. Most of the DOM in the XML-viewer case is anonymous XBL-bound stuff. We need to add that to the DOM reporter.... In particular, we should consider using nsChildIterator in nsIDocument::SizeOf. And even then, I'm not sure it would hit all the nodes.
DMD shows this DOM memory as important: ==31189== Unreported: 9,644,518 (cumulative: 32,915,270) bytes in 3,609 heap block(s) in record 3 of 11524: ==31189== Requested bytes unreported: 8,798,046 / 8,798,046 ==31189== Slop bytes unreported: 846,472 / 846,472 ==31189== at 0x4029F90: malloc (vg_replace_malloc.c:235) ==31189== by 0x403B090: moz_malloc (mozalloc.cpp:120) ==31189== by 0x785A5C6: NS_Alloc_P (nsMemoryImpl.cpp:198) ==31189== by 0x662FA60: nsMemory::Alloc(unsigned long) (nsMemory.h:68) ==31189== by 0x6BAA629: nsTextFragment::SetTo(unsigned short const*, int) (nsTextFragment.cpp:282) ==31189== by 0x6B695F3: nsGenericDOMDataNode::SetTextInternal(unsigned int, unsigned int, unsigned short const*, unsigned int, int, CharacterDataChangeInfo::Details*) (nsGenericDOMDataNode.cpp:342) volkmar, is that covered by existing reporters? (I haven't taught DMD about DOM reporters yet.)
I'm downgrading this to MemShrink:P2, because a decent chunk of the DOM is covered now. CSS stuff is a bigger fraction of the dark matter now.
Whiteboard: [MemShrink:P1] → [MemShrink:P2]
This is pretty fixed. We can file new bugs for followups that we think are important as needed.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
(and by "think are important", I mean, where we have some specific evidence they would account for enough memory to be useful)
You need to log in before you can comment on or make changes to this bug.