Add #define to always log the cycle collector graph

RESOLVED FIXED in mozilla9

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Trunk
mozilla9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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.
Assignee: nobody → continuation
(Assignee)

Comment 2

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

Comment 3

6 years ago
I'll need to update this Wiki page if I change this: https://wiki.mozilla.org/Performance:Leak_Tools
(Assignee)

Comment 4

6 years ago
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.
Attachment #553820 - Attachment is obsolete: true
(Assignee)

Comment 5

6 years ago
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.
Attachment #557589 - Attachment is obsolete: true
Attachment #557854 - Flags: review?(bent.mozilla)
(Assignee)

Updated

6 years ago
Component: XPConnect → XPCOM
QA Contact: xpconnect → xpcom
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.
Attachment #557854 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 7

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/7946546d5a6c
Target Milestone: --- → mozilla9
http://hg.mozilla.org/mozilla-central/rev/7946546d5a6c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.