Last Comment Bug 679779 - Add #define to always log the cycle collector graph
: Add #define to always log the cycle collector graph
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Andrew McCreight [:mccr8]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-17 10:33 PDT by Andrew McCreight [:mccr8]
Modified: 2011-09-08 10:27 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add define to always log the CC graph (2.70 KB, patch)
2011-08-17 10:38 PDT, Andrew McCreight [:mccr8]
no flags Details | Diff | Splinter Review
add #define to always log the CC graph (3.93 KB, patch)
2011-09-01 11:50 PDT, Andrew McCreight [:mccr8]
no flags Details | Diff | Splinter Review
add a const to always log the CC graph (3.68 KB, patch)
2011-09-02 09:11 PDT, Andrew McCreight [:mccr8]
bent.mozilla: review+
Details | Diff | Splinter Review

Description Andrew McCreight [:mccr8] 2011-08-17 10:33:59 PDT
For leak finding purposes it is often useful to set the cycle collector to always produce a cycle collector log.  Thanks to all of the listener code, this capacity is present even in a non-DEBUG_CC build, but getting it to actually trigger all of the time requires a little bit of fussing with the code in nsCycleCollector.cpp.

This patch adds a new macro LOG_CC_GRAPH that says whether we should log the graph or not.  If this macro evaluates to true, a CC graph will be logged.

If ALWAYS_LOG_CC_GRAPH is defined, then this is true.  If it isn't defined, then if DEBUG_CC is defined, then it looks up mDrawGraphs in the CC parameters, as usual.  If neither of these is defined, this is false.
Comment 1 Andrew McCreight [:mccr8] 2011-08-17 10:38:30 PDT
Created attachment 553820 [details] [diff] [review]
add define to always log the CC graph

This removes the #ifdef DEBUG_CC in a few places, but it turns the branch into an "if (false)" so it should not change a normal optimized build.
Comment 2 Andrew McCreight [:mccr8] 2011-08-17 10:39:30 PDT
azakai (I think) was asking how to do this a few weeks ago.  This patch makes it easier.
Comment 3 Andrew McCreight [:mccr8] 2011-08-29 09:14:03 PDT
I'll need to update this Wiki page if I change this: https://wiki.mozilla.org/Performance:Leak_Tools
Comment 4 Andrew McCreight [:mccr8] 2011-09-01 11:50:32 PDT
Created attachment 557589 [details] [diff] [review]
add #define to always log the CC graph

This is less #define magic-y than the other patch. I make mDrawGraphs into a parameter that is always present (and rename it mLogGraphs, as that is more accurate), setting it as appropriate.

To make it always log you only have to change
  #define ALWAYS_LOG_CC_GRAPHS PR_FALSE
to
  #define ALWAYS_LOG_CC_GRAPHS PR_TRUE

This patch also gets rid of two uses of #DEBUG_CC.  I haven't tested it yet.
Comment 5 Andrew McCreight [:mccr8] 2011-09-02 09:11:02 PDT
Created attachment 557854 [details] [diff] [review]
add a const to always log the CC graph

Mostly the same as the last version, but I use a const global instead of a #define.  I tested this with and without DEBUG_CC, and with DEBUG_CC and XPCOM_CC_DRAW_GRAPHS.  I had to comment out ExplainLiveExpectedGarbage due to Bug 684098, but that shouldn't be a problem.
Comment 6 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-09-06 15:34:31 PDT
Comment on attachment 557854 [details] [diff] [review]
add a const to always log the CC graph

Review of attachment 557854 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/nsCycleCollector.cpp
@@ +2909,5 @@
>      // disable the collector because the program is shutting down.
>  
> +    nsCOMPtr<nsCycleCollectorLogger> listener;
> +    if (mParams.mLogGraphs)
> +        listener = new nsCycleCollectorLogger();

Nit: Brace this.
Comment 7 Andrew McCreight [:mccr8] 2011-09-07 10:50:01 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/7946546d5a6c
Comment 8 :Ehsan Akhgari (away Aug 1-5) 2011-09-08 10:27:00 PDT
http://hg.mozilla.org/mozilla-central/rev/7946546d5a6c

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