Make cycle-collection debugging features optional at compile time

RESOLVED FIXED in mozilla1.9alpha3

Status

()

Core
XPCOM
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: peterv, Assigned: peterv)

Tracking

Trunk
mozilla1.9alpha3
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

12 years ago
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.
Agreed.  I was actually thinking of filing this yesterday. :-)
(Assignee)

Updated

11 years ago
Assignee: nobody → peterv
Summary: Make cycle-collection graph drawing feature optional at compile time → Make cycle-collection debugging features optional at compile time
(Assignee)

Comment 3

11 years ago
Created attachment 262266 [details] [diff] [review]
v1.1
Attachment #262163 - Attachment is obsolete: true
Attachment #262266 - Flags: review?(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.
Attachment #262266 - Flags: review?(graydon) → review+
(Assignee)

Comment 5

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

Comment 6

11 years ago
Created attachment 262289 [details] [diff] [review]
v1.2

Actually, let's be safe and keep the null-check for now.
Attachment #262266 - Attachment is obsolete: true
Attachment #262289 - Flags: superreview?(jst)
Attachment #262289 - Flags: review+

Updated

11 years ago
Attachment #262289 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 7

11 years ago
Created attachment 262360 [details] [diff] [review]
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.
Attachment #262289 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Blocks: 377787
(Assignee)

Comment 8

11 years ago
This seems to have lowered Collect times by about 5-10% in my DEBUG build.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.