Closed Bug 654041 Opened 13 years ago Closed 13 years ago

Add buttons that trigger GC and CC from about:memory

Categories

(Toolkit :: about:memory, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla6

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(3 files, 2 obsolete files)

When using about:memory to hunt down memory leaks, sometimes you really want to run a GC and/or CC to ensure that all garbage has been collected.

We probably want three buttons: "GC", "CC", and "GC + CC".  I've heard that sometimes you need multiple runs of both to get all the garbage, so the "GC + CC" button could do this.
Depends on: 651273
Yes, the CC only frees DOM objects, and the GC only frees JS objects, so if there's a garbage cycle made up of both, it could take something like GC -> CC -> GC to clear it out.
OS: Linux → All
Hardware: x86_64 → All
Oops, I guess you are just running a CC here, not dumping a graph, so never mind about that dependency I added.
No longer depends on: 651273
BTW, to do this I need to find out how to trigger GC and CC from JavaScript.  I thought gc() did GC but now I see that's only in the shell.  Any suggestions are welcome.
There is nsIDOMWindowUtils.garbageCollect and Components.utils.forceGC. You can figure out which does what because I don't really know.
I'd like to also have a button that fires a memory-pressure notification (like RAMBack). It could be labeled "GC, CC, and flush caches".

This code fires a notification:

  Components.classes["@mozilla.org/observer-service;1"]
            .getService(Components.interfaces.nsIObserverService)
            .notifyObservers(null, "memory-pressure", "heap-minimize");

Caches are expected to listen for memory-pressure notifications and clear themselves [1]. For example, it causes all bfcache pages to be evicted [2]. It also triggers a GC and a CC [3].

[1] https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIMemory#Low_memory_notifications

[2] http://hg.mozilla.org/mozilla-central/annotate/84af3e2c7ac3/docshell/shistory/src/nsSHistory.cpp#l120

[3] http://hg.mozilla.org/mozilla-central/annotate/84af3e2c7ac3/dom/base/nsJSEnvironment.cpp#l195
The code in this add-on should be helpful: https://addons.mozilla.org/en-US/firefox/addon/ramback/ :)
Attached patch patch (obsolete) — Splinter Review
This patch adds three buttons: "GC", "GC + CC", and "Minimize memory usage".  (Screenshot coming shortly.)  The 2nd button is "GC + CC" instead of just "CC" because of bug 625302.

This works great on desktop;  I used to console to check that the GCs/CCs were occurring.  On Fennec it's less useful as the events only apply to the main (chrome) process.  I imagine triggering the events in the content proces would require some complicated IPC.  I guess the question is whether this patch is good enough for now?
Attached image screenshot
Comment on attachment 530548 [details] [diff] [review]
patch

Asking Vlad for the review because I can't think of anyone better.
Attachment #530548 - Flags: review?(vladimir)
Awesome stuff. shaver can review as well. He hacked on in quite a bit.
> +  // Do it three times, just to be sure.
> +  os.notifyObservers(null, "memory-pressure", "heap-minimize");
> +  os.notifyObservers(null, "memory-pressure", "heap-minimize");
> +  os.notifyObservers(null, "memory-pressure", "heap-minimize");

For maximum effect, you want to return to the event loop between these notifications.  See bug 610166 comment 12 for an example of why. http://pastebin.mozilla.org/1219590 shows the code I use to return to the event loop.

> "triggers a global garbage collection followed by a cycle collection, and 
> may cause the process to reduce other memory usage, e.g. by flushing caches."

The use of "may" here makes it seem like the cache-flushing effect is less certain than the garbage-collection effect, or less certain to reduce memory use.

Would "... followed by a cycle collection, and flushes many caches" be accurate enough?
Just in case it disappears from pastebin some time in the future.
(In reply to comment #11)
> 
> For maximum effect, you want to return to the event loop between these
> notifications.

Good to know, thanks.

> > "triggers a global garbage collection followed by a cycle collection, and 
> > may cause the process to reduce other memory usage, e.g. by flushing caches."
> 
> The use of "may" here makes it seem like the cache-flushing effect is less
> certain than the garbage-collection effect, or less certain to reduce memory
> use.
> 
> Would "... followed by a cycle collection, and flushes many caches" be
> accurate enough?

Sure, I'll change it.
Comment on attachment 530548 [details] [diff] [review]
patch

Looks reasonable.
Attachment #530548 - Flags: review?(vladimir) → review+
Attachment #530558 - Attachment mime type: text/x-matlab → text/plain
I filed bug 655647 as a follow-up for working with child processes;  I want to get this in quickly (ie. for Firefox 6) because it's fully functional for desktop Firefox.
Blocks: 655647
(In reply to comment #11)
> > +  // Do it three times, just to be sure.
> > +  os.notifyObservers(null, "memory-pressure", "heap-minimize");
> > +  os.notifyObservers(null, "memory-pressure", "heap-minimize");
> > +  os.notifyObservers(null, "memory-pressure", "heap-minimize");
> 
> For maximum effect, you want to return to the event loop between these
> notifications.  See bug 610166 comment 12 for an example of why.
> http://pastebin.mozilla.org/1219590 shows the code I use to return to the
> event loop.

I'm not sure that we want to do that.  If we need multiple GCs or CCs or event loop trips that's (at least IMO) a bug that should be fixed.
Attached patch patch v2 (obsolete) — Splinter Review
This version goes back to the event loop for each notification, as per Jesse's suggestion.
Attachment #530548 - Attachment is obsolete: true
Attachment #530976 - Flags: review?(jruderman)
Looks like this patch runs 3 memory-pressure notifications, but calls update() after the first one.  I think you want to update() after the last one instead.
Attached patch patch v3Splinter Review
This version uses runSoon() to call update() after the notifications have occurred.
Attachment #530976 - Attachment is obsolete: true
Attachment #530976 - Flags: review?(jruderman)
Attachment #530981 - Flags: review?(jruderman)
Comment on attachment 530981 [details] [diff] [review]
patch v3

Looks good to me.
Attachment #530981 - Flags: review?(jruderman) → review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Verified on the latest Nightly:
Mozilla/5.0 (Windows NT 6.1; rv:7.0a1) Gecko/20110703 Firefox/7.0a1
Status: RESOLVED → VERIFIED
Component: General → about:memory
QA Contact: general → about.memory
Target Milestone: --- → mozilla6
Blocks: 677358
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: