The default bug view has changed. See this FAQ.

Add memory reporters for the DOM

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: njn, Assigned: mounir)

Tracking

(Depends on: 3 bugs, Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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

6 years ago
Blocks: 563700
No longer blocks: 640791
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

6 years ago
Whiteboard: MemShrink:P1 → [MemShrink:P1]
(Assignee)

Updated

6 years ago
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
Created attachment 539700 [details] [diff] [review]
estimate DOM memory usage using cycle collector infrastructure

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.
(Assignee)

Updated

6 years ago
Assignee: nobody → mounir.lamouri
(Reporter)

Comment 5

6 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.
(Reporter)

Updated

6 years ago
Duplicate of this bug: 340372
(Assignee)

Comment 7

6 years ago
For the first step, do we want to mix content and chrome or just have content?
(Reporter)

Comment 8

6 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

6 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

6 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)

Updated

6 years ago
Depends on: 667468
(Assignee)

Comment 11

6 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
(Assignee)

Updated

6 years ago
Depends on: 667470
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...
(Assignee)

Updated

6 years ago
Depends on: 667474
(Assignee)

Updated

6 years ago
Depends on: 667485
(Assignee)

Updated

6 years ago
Depends on: 667497
(Reporter)

Comment 13

6 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.
(Assignee)

Updated

6 years ago
Depends on: 667883
(Assignee)

Updated

6 years ago
Depends on: 667887
(Assignee)

Updated

6 years ago
Depends on: 668013
(Reporter)

Comment 14

6 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.
(Assignee)

Updated

6 years ago
Depends on: 669308
(Assignee)

Updated

6 years ago
Depends on: 669330
(Assignee)

Updated

6 years ago
Depends on: 669886
(Assignee)

Updated

6 years ago
Depends on: 669904
(Assignee)

Updated

6 years ago
Depends on: 670962
(Assignee)

Updated

6 years ago
Depends on: 670965
(Assignee)

Updated

6 years ago
Depends on: 672539
(Reporter)

Comment 15

6 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.
> 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.
(Assignee)

Updated

6 years ago
Depends on: 674112
(Assignee)

Updated

6 years ago
Depends on: 674113
(Assignee)

Updated

6 years ago
Depends on: 674114
(Assignee)

Updated

6 years ago
Depends on: 674115
(Assignee)

Updated

6 years ago
Depends on: 674116
(Assignee)

Updated

6 years ago
Depends on: 677503
(Assignee)

Updated

6 years ago
Depends on: 677504
(Assignee)

Updated

6 years ago
Depends on: 677506
(Reporter)

Comment 17

6 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.
> 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.
(Assignee)

Updated

6 years ago
Depends on: 679499
(Reporter)

Comment 19

6 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.)
Depends on: 681199
Depends on: 682264
Depends on: 682432
Depends on: 682435
(Reporter)

Comment 20

6 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]
(Assignee)

Updated

6 years ago
Depends on: 684698
This is pretty fixed.  We can file new bugs for followups that we think are important as needed.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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.