372110 - Make cycle-collection debugging features optional at compile time

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Peter Van der Beken [:peterv]]

I don't think we should ship the graph drawing code in released builds, in that case we also don't need the objname argument for DescribeNode.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Agreed.  I was actually thinking of filing this yesterday. :-)

Comment 2

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 3

[Peter Van der Beken [:peterv]]

Attachment: v1.1

Comment 4

[Graydon Hoare :graydon]

Comment on attachment 262266 [details] [diff] [review]
v1.1

I'm a little nervous about the disabled check for null in Walk, since we'll crash rather than just go dead in that case. Ask whoever's super-reviewing to tie-break on that, I don't know what the right choice is to be "well-behaved". Otherwise looks good.

Comment 5

[Peter Van der Beken [:peterv]]

(In reply to comment #4)
> (From update of attachment 262266 [details] [diff] [review])
> I'm a little nervous about the disabled check for null in Walk, since we'll
> crash rather than just go dead in that case. Ask whoever's super-reviewing to
> tie-break on that, I don't know what the right choice is to be "well-behaved".

Well, my thinking was that because we make sure that the pointer is added to mGraph before pushing it in the queue (in NoteXPCOMChild/NoteScriptChild), we're sure that it is in mGraph and lookup shouldn't fail.

Comment 6

[Peter Van der Beken [:peterv]]

Attachment: v1.2

Actually, let's be safe and keep the null-check for now.

Comment 7

[Peter Van der Beken [:peterv]]

Attachment: v1.3

Turns out we need mScanDelay to force collection during shutdown, so I undid the changes to remove it and nsCycleCollectorParams, otherwise we'll leak more.

Comment 8

[Peter Van der Beken [:peterv]]

This seems to have lowered Collect times by about 5-10% in my DEBUG build.