Closed
Bug 663271
Opened 13 years ago
Closed 12 years ago
Add memory reporters for the DOM
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: n.nethercote, Assigned: mounir)
References
(Depends on 2 open bugs, Blocks 1 open bug)
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file)
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).
Reporter | ||
Updated•13 years ago
|
Comment 1•13 years ago
|
||
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.
Reporter | ||
Updated•13 years ago
|
Whiteboard: MemShrink:P1 → [MemShrink:P1]
Assignee | ||
Updated•13 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
Comment 2•13 years ago
|
||
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.
Comment 3•13 years ago
|
||
That's missing most of the memory (e.g. the memory needed by attributes, child lists, text in textnodes, etc, etc)...
Comment 4•13 years ago
|
||
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.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → mounir.lamouri
Reporter | ||
Comment 5•13 years ago
|
||
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.
Assignee | ||
Comment 7•13 years ago
|
||
For the first step, do we want to mix content and chrome or just have content?
Reporter | ||
Comment 8•13 years ago
|
||
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.
Assignee | ||
Comment 9•13 years ago
|
||
(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...
Reporter | ||
Comment 10•13 years ago
|
||
(In reply to comment #9) > And it looks like the bug number your pasted isn't correct... Sorry, I meant bug 666075.
Assignee | ||
Comment 11•13 years ago
|
||
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
Comment 12•13 years ago
|
||
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...
Reporter | ||
Comment 13•13 years ago
|
||
(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.
Reporter | ||
Comment 14•13 years ago
|
||
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.
Reporter | ||
Comment 15•13 years ago
|
||
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.
Comment 16•13 years ago
|
||
> 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.
Reporter | ||
Comment 17•13 years ago
|
||
(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 [4] │ └──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.
Comment 18•13 years ago
|
||
> 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.
Reporter | ||
Comment 19•13 years ago
|
||
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.)
Reporter | ||
Comment 20•13 years ago
|
||
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]
Comment 21•12 years ago
|
||
This is pretty fixed. We can file new bugs for followups that we think are important as needed.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 22•12 years ago
|
||
(and by "think are important", I mean, where we have some specific evidence they would account for enough memory to be useful)
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•