Last Comment Bug 654041 - Add buttons that trigger GC and CC from about:memory
: Add buttons that trigger GC and CC from about:memory
Status: VERIFIED FIXED
:
Product: Toolkit
Classification: Components
Component: about:memory (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla6
Assigned To: Nicholas Nethercote [:njn] (on vacation until July 11)
:
Mentors:
Depends on: revampaboutmemory
Blocks: 655647 677358
  Show dependency treegraph
 
Reported: 2011-05-01 21:54 PDT by Nicholas Nethercote [:njn] (on vacation until July 11)
Modified: 2011-08-12 12:15 PDT (History)
19 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.88 KB, patch)
2011-05-05 23:21 PDT, Nicholas Nethercote [:njn] (on vacation until July 11)
vladimir: review+
Details | Diff | Review
screenshot (17.08 KB, image/png)
2011-05-05 23:23 PDT, Nicholas Nethercote [:njn] (on vacation until July 11)
no flags Details
Jesse's code from comment 11 (540 bytes, text/plain)
2011-05-06 00:20 PDT, Nicholas Nethercote [:njn] (on vacation until July 11)
no flags Details
patch v2 (3.67 KB, patch)
2011-05-08 21:09 PDT, Nicholas Nethercote [:njn] (on vacation until July 11)
no flags Details | Diff | Review
patch v3 (3.69 KB, patch)
2011-05-08 21:47 PDT, Nicholas Nethercote [:njn] (on vacation until July 11)
jruderman: review+
Details | Diff | Review

Description Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-01 21:54:21 PDT
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.
Comment 1 Andrew McCreight [:mccr8] 2011-05-02 07:50:37 PDT
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.
Comment 2 Andrew McCreight [:mccr8] 2011-05-02 08:17:13 PDT
Oops, I guess you are just running a CC here, not dumping a graph, so never mind about that dependency I added.
Comment 3 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-02 15:38:34 PDT
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.
Comment 4 Dave Townsend [:mossop] 2011-05-02 15:41:19 PDT
There is nsIDOMWindowUtils.garbageCollect and Components.utils.forceGC. You can figure out which does what because I don't really know.
Comment 5 Jesse Ruderman 2011-05-03 14:01:46 PDT
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
Comment 6 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-05 19:26:27 PDT
The code in this add-on should be helpful: https://addons.mozilla.org/en-US/firefox/addon/ramback/ :)
Comment 7 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-05 23:21:43 PDT
Created attachment 530548 [details] [diff] [review]
patch

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?
Comment 8 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-05 23:23:24 PDT
Created attachment 530549 [details]
screenshot
Comment 9 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-05 23:25:13 PDT
Comment on attachment 530548 [details] [diff] [review]
patch

Asking Vlad for the review because I can't think of anyone better.
Comment 10 Andreas Gal :gal 2011-05-05 23:32:53 PDT
Awesome stuff. shaver can review as well. He hacked on in quite a bit.
Comment 11 Jesse Ruderman 2011-05-05 23:47:15 PDT
> +  // 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?
Comment 12 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-06 00:20:10 PDT
Created attachment 530558 [details]
Jesse's code from comment 11

Just in case it disappears from pastebin some time in the future.
Comment 13 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-06 00:23:14 PDT
(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 14 Vladimir Vukicevic [:vlad] [:vladv] 2011-05-06 15:03:11 PDT
Comment on attachment 530548 [details] [diff] [review]
patch

Looks reasonable.
Comment 15 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-08 20:31:33 PDT
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.
Comment 16 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-05-08 20:44:32 PDT
(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.
Comment 17 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-08 21:09:37 PDT
Created attachment 530976 [details] [diff] [review]
patch v2

This version goes back to the event loop for each notification, as per Jesse's suggestion.
Comment 18 Jesse Ruderman 2011-05-08 21:22:39 PDT
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.
Comment 19 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-08 21:47:28 PDT
Created attachment 530981 [details] [diff] [review]
patch v3

This version uses runSoon() to call update() after the notifications have occurred.
Comment 20 Jesse Ruderman 2011-05-08 22:59:36 PDT
Comment on attachment 530981 [details] [diff] [review]
patch v3

Looks good to me.
Comment 21 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-11 21:04:20 PDT
http://hg.mozilla.org/mozilla-central/rev/28bd3e0ad1fb
Comment 22 AndreiD[QA] 2011-07-04 06:58:15 PDT
Verified on the latest Nightly:
Mozilla/5.0 (Windows NT 6.1; rv:7.0a1) Gecko/20110703 Firefox/7.0a1

Note You need to log in before you can comment on or make changes to this bug.