Closed
Bug 913130
Opened 11 years ago
Closed 11 years ago
Make ShutdownCollect() call Collect()
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: mccr8, Assigned: mccr8)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
3.54 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
2.10 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
1.66 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
1.35 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
The body of the ShutdownCollect() loop is almost the same as Collect(), so we should just make them the same.
Assignee | ||
Comment 1•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=fdc08dad20b1
Assignee | ||
Comment 2•11 years ago
|
||
This is the most complicated part. Pass in white nodes as an argument, force a GC if the type is ShutdownCC, return if we collected anything. No particular hurry on the reviews for this.
Attachment #800830 -
Flags: review?(bugs)
Assignee | ||
Comment 3•11 years ago
|
||
A few of the things in these two methods are different for no reason I can see, so make them consistent. I figure it is better to do PrepareForCollection as early as possible.
Attachment #800831 -
Flags: review?(bugs)
Assignee | ||
Comment 4•11 years ago
|
||
PrepareForCollection just initializes some variables and calls the runtime's PrepareForCollection, so doing it repeatedly doesn't seem like a big deal. Doing CleanupAfterCollection on every iteration will fire telemetry repeatedly, but we don't run shutdown CCs in opt builds so it shouldn't matter.
Attachment #800832 -
Flags: review?(bugs)
Assignee | ||
Comment 5•11 years ago
|
||
The inner loop in ShutdownCollect is now the same as Collect, so just call Collect.
Attachment #800835 -
Flags: review?(bugs)
Comment 6•11 years ago
|
||
Comment on attachment 800830 [details] [diff] [review] part 1 - Modify nsCycleCollector::Collect to allow it to be used at shutdown. >- void Collect(ccType aCCType, >+ bool Collect(ccType aCCType, >+ nsTArray<PtrInfo*> *aWhiteNodes, Could this be nsTArray<PtrInfo*> &aWhiteNodes Not sure I understand yet the setup... reading other patches.
Attachment #800830 -
Flags: review?(bugs) → review+
Updated•11 years ago
|
Attachment #800831 -
Flags: review?(bugs) → review+
Updated•11 years ago
|
Attachment #800832 -
Flags: review?(bugs) → review+
Updated•11 years ago
|
Attachment #800835 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 7•11 years ago
|
||
> Could this be nsTArray<PtrInfo*> &aWhiteNodes
Sort of, but it just gets passed through a few functions and stuck into a field mWhiteNodes that has to be a pointer because it is NULL when the CC isn't running. I'm also going to get rid of that argument for ICC anyways, so I'm just going to leave it as a * for now.
Assignee | ||
Comment 8•11 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/9d23a05bd9d6 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ed3d4d7d678c remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/0896b8c0f616 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/5e861b43ebf1
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9d23a05bd9d6 https://hg.mozilla.org/mozilla-central/rev/ed3d4d7d678c https://hg.mozilla.org/mozilla-central/rev/0896b8c0f616 https://hg.mozilla.org/mozilla-central/rev/5e861b43ebf1
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in
before you can comment on or make changes to this bug.
Description
•