679779 - Add #define to always log the cycle collector graph

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

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]]

Attachment: 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]]

azakai (I think) was asking how to do this a few weeks ago.  This patch makes it easier.

Comment 3

[Andrew McCreight [:mccr8]]

I'll need to update this Wiki page if I change this: https://wiki.mozilla.org/Performance:Leak_Tools

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: 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]]

Attachment: 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!)]

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]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/7946546d5a6c

Comment 8

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/7946546d5a6c