Bug 625305 (about:compartments)

about:compartments

RESOLVED WONTFIX

Status

()

Core
XPConnect
RESOLVED WONTFIX
7 years ago
6 years ago

People

(Reporter: shaver, Assigned: njn)

Tracking

({footprint})

Trunk
x86
Windows XP
footprint
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2], URL)

Attachments

(3 attachments, 5 obsolete attachments)

Created attachment 503431 [details] [diff] [review]
about:compartments v1

I present a first cut of about:compartments, which tells you how much method JIT memory and how many gcBytes are being used by each current compartment.  Lots more to do (JS heap accounting, figuring out what to do with the DOM and layout and imglib memory, trace-jit stuff).  Dunno if it's interesting for FF4 -- new code doesn't run unless you go to about:compartments, by risk-minimization design.

I don't want to talk about how long it took to write those ~500 new lines of code.
Created attachment 503432 [details]
screenshot

You don't have to tell me it's ugly.
it's beautiful.
did a quick skim through the code. This looks pretty sweet and could give us the basis for some real memory monitors(!).

There are some empty method stubs in here, and presumably a bit of work to do before it's ready for a blast through the try servers, but I'm pretty eager to get this kind of support into the platform so we can start creating memory monitors.

nice work!
I'm also curious if there are any platform limitations on this. Will this run everywhere? Just Windows?

Updated

7 years ago
Severity: normal → enhancement
OS: Windows 7 → Windows XP

Updated

7 years ago
Created attachment 509985 [details] [diff] [review]
updated: string-count, object-count, function-count, off-heap-object-data

Updated to include more stuff, and properly constrain to a compartment.
Assignee: nobody → shaver
Attachment #503431 - Attachment is obsolete: true
Severity: enhancement → normal
Rob: it'll run everywhere, it's all platform-agnostic code.
Created attachment 509986 [details]
updated screenshot

I took out gcLastBytes after I took the screenshot, and it's shrunk down a bit to show more interesting data -- 12MB of mjit code for facebook/plugins/recommendations!
Attachment #503432 - Attachment is obsolete: true
Alias: about:compartments
Created attachment 510040 [details] [diff] [review]
better atom accounting, gc-heap arena tracking

Next up: trace JIT memory, and then I move onto DOM structures!
Attachment #509985 - Attachment is obsolete: true
Created attachment 510054 [details] [diff] [review]
updated patch: mjit-code-net, script-count, gc-heap, string-data, string-count, object-count, function-count, off-heap-object-data, atom-count, atom-data

I'm still not very good at mq.
Attachment #510040 - Attachment is obsolete: true
Created attachment 510064 [details] [diff] [review]
classify heap kinds, add tjit data, classify scripts

tjit-code-net doesn't include everything I want it to yet, because some of the allocators don't expose their balance through public-enough interfaces.

http://webkit.org/perf/sunspider-0.9.1/sunspider-0.9.1/driver.html {
  "mjit-code-net": 1786695,
  "tjit-code-net": 52816,
  "script-count": 865,
  "script-count-mjit": 367,
  "script-count-tjit": 4,
  "string-count": 13,
  "string-data": 140,
  "object-count": 9602,
  "function-count": 8745,
  "off-heap-object-data": 210968,
  "gc-bytes": 39051264,
  "gc-bytes-objects": 18595840,
  "gc-bytes-functions": 6639616,
  "gc-bytes-xml": 0,
  "gc-bytes-strings": 13815808
}
Attachment #510054 - Attachment is obsolete: true
Created attachment 510436 [details] [diff] [review]
patch for tracking cumulative memory usage

This patch overlaps with the ones in this bug. It tracks net and gross usage, as well as cumulative usage (discounting frees). Cumulative numbers seems to be somewhat more stable than the current total, so it might be useful for tuning.

Eventually I'd like to integrate this stuff with the other patches here. For now I'm posting it so that dvander can maybe use it.
(Assignee)

Comment 12

7 years ago
I reckon ultimately about:memory and about:compartments should be merged, though this wouldn't have to happen immediately.  It'd also be nice if the measurements were less ad hoc -- for example, about:memory's "mapped"/"in use" measurements are just for the heap and exclude executable memory allocated by the JITs.  And it'd be nice if all the sub-listings added up to the totals in an understandable fashion.

As for this bug:  something I'd love to see in about:compartments would be a "Do a GC" button.  Actually, you could have one per compartment to force a compartment GC, plus one to force a global GC.  This would be a huge help for a bug like bug 630932 where I was trying to work out if there's a leak, but I could never quite tell if there was a GC pending that would get rid of a lot of junk.  Would that be difficult?  

(Hmm, "javascript: gc()" suffices?  Though that's presumably a global GC?  Can a compartment GC be done that way?  Also, can the cycle collector be manually triggered?)
There is garbageCollect in nsIDOMWindowUtils.
the "includeInTotal" and units will combine to give us the totals you want.  I may come up with a prefix model too, to make some of the hierarchy more straightforward as well.  Probably not until this weekend, though, and I'm not sure this'll make it in 4.
(Assignee)

Comment 15

7 years ago
(In reply to comment #12)
> I reckon ultimately about:memory and about:compartments should be merged,

I blogged about this and some follow-on thoughts:  http://blog.mozilla.com/nnethercote/2011/02/09/a-vision-for-better-memory-profiling-with-aboutmemory/
(Assignee)

Comment 16

7 years ago
Comment on attachment 510064 [details] [diff] [review]
classify heap kinds, add tjit data, classify scripts

>+    size_t grossSize() {
>+        size_t gross = 0;
>+        for (ExecutablePool **ep = m_smallAllocationPools.begin();
>+             ep != m_smallAllocationPools.end();
>+             ++ep) {
>+            gross += (*ep)->grossAllocationSize();
>+        }
>+        return gross;
>+    }

Oh, this is insufficient.  m_smallAllocationPools holds up to four ExecutablePools;  it does this just to minimize fragmentation (see bug 616310).  If more than four pools have been created, the ExecutableAllocator will have lost track of them.  (References to the pools will be stored in other places like JITScripts and IC structures so they haven't been lost altogether.)

So ExecutableAllocator will need to have a vector added to it that hang onto all of the pools it has created.  I was planning to do this anyway, because I want to get rid of the reference counting of executable pools and instead just deallocate them all when the compartment is destroyed, and that requires exactly this vector.
yeah, I know that's wrong.  I don't use it anywhere yet, IIRC.  I was thinking about fixing it to count correctly, and then I saw your cleanup bug, so I figured I'd just wait it out. :)
(Assignee)

Updated

7 years ago
Blocks: 640791
(Assignee)

Comment 18

7 years ago
Will this help identify leaks?  Eg. if I open foo.com and bar.com in two tabs, then close the bar.com tab, if the bar.com compartment stays around that means there's a leak of some kind?

Comment 19

7 years ago
(In reply to comment #15)
Maybe not merge about:memory and about:compartment memory information?
The compartment resource presentation has significant user relevance and could potentially ascend to be a menu selection and therefore its display requirements may become different -- much more user friendly/interactive.

For now, would simple cross references suffice?  (see "about:compartments" ...)
(Assignee)

Comment 20

6 years ago
This patch does a GC-style trace of the live heap.  But that has a couple of problems:

- you can overflow the stack

- it only counts live objects

Bug 571249 will add code to do traversals of the GC arenas (ie. both live and dead objects).  That will be done on a global basis, but will provide a good bases for per-compartment traversals required for this bug.
Assignee: shaver → nnethercote
Depends on: 571249
(Assignee)

Updated

6 years ago
Whiteboard: [MemShrink:P2]
(Assignee)

Comment 21

6 years ago
I'm going to WONTFIX this in favour of bug 661474, which is my preferred way of presenting this information.  Parts of the code from Shaver's patch will live on in bug 666075, however.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.