688225 - add preference variables to control cycle collector graph dumping

Status: RESOLVED INCOMPLETE

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8] (PTO until 12-2)]

We're almost there right now, so we might as well make it easy as possible to do.  This could be a DEBUG-only option or not.  The code would be similar to the current DEBUG_CC-only option.  I'd call the CC option something like XPCOM_CC_DUMP_GRAPHS.

Comment 1

[Andrew McCreight [:mccr8] (PTO until 12-2)]

Attachment: add debug-only env var to always dump CC graph

Needs to be unbitrotted for the PRBool->bool change, but that doesn't affect the patch proper.  I used "false" instead of "PR_FALSE" for this reason.  I can switch it back if that is better.

I made this DEBUG-only because it requires an additional call to PR_GetEnv during startup, when the cycle collector object is created.  Johnny said that could be slow.  I could instead defer this PR_GetEnv to the first cycle collection, but I think that would require checking every time we CC if we've done the GetEnv yet.  Which would just be a single bool check, but still...

Comment 2

[Andrew McCreight [:mccr8] (PTO until 12-2)]

Comment on attachment 564215 [details] [diff] [review]
add debug-only env var to always dump CC graph

sfink suggested that this could be a preference, and in non-debug builds, and that sounds niftier, so I'll look into doing that instead.

Comment 3

[Andrew McCreight [:mccr8] (PTO until 12-2)]

This is getting past the original goal for the bug, but I also want to dump the JS graph into the same file as the CC graph as Peter suggested.  There could be a pref for each.  Once that is available, I think it makes some amount of sense to not have WANT_ALL_TRACES as the default behavior for log dumping when there is a listener, because we can get the full JS heap anyways.

Johnny was also wondering what the performance impact of the CC dumping is, so I should look into that a bit, at least informally.  What is the cost of dumping the CC graph vs. WANT_ALL_TRACES vs. dumping the GC graph?

Comment 4

[Andrew McCreight [:mccr8] (PTO until 12-2)]

jst pointed out that a preference would be good because then you could set it after startup, and that the danger of somebody accidentally leaving it on is not a huge one.

Comment 5

[Andrew McCreight [:mccr8] (PTO until 12-2)]

Looks like the cycle collector initializes before Preferences...

Comment 6

[Andrew McCreight [:mccr8] (PTO until 12-2)]

Attachment: add 2 prefs for controlling CC dumping, crashes immediately