Closed Bug 913527 Opened 6 years ago Closed 6 years ago

Simplify top level invocation of cycle collector phases

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(3 files)

No description provided.
Depends on: 913130
I'm simplifying the top level invocation of CC phases in these patches to make it easier to resume an ICC.
Attachment #801180 - Flags: review?(bugs)
Blocks: 913881
Comment on attachment 801180 [details] [diff] [review]
part 1 - Get rid of FinishCollection. r=smaug

Odd naming after this patch.
BeginCollection, CollectWhite and CleanupAfterCollection.

CollectWhite doesn't quite fit in.

Could you explain a bit.
Attachment #801180 - Flags: review?(bugs) → review-
Attachment #801181 - Flags: review?(bugs) → review+
Attachment #801182 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #4)
> Odd naming after this patch.
> BeginCollection, CollectWhite and CleanupAfterCollection.
> 
> CollectWhite doesn't quite fit in.
> 
> Could you explain a bit.

Sure. So, what happens in later patches is that I move MarkRoots and ScanRoots out of BeginCollection, and split up CollectWhite into its three subphases.  That leaves the main part of Collect like this:

    BeginCollection(aCCType, aManualListener);
    MarkRoots();
    ScanRoots();
    RootGarbage();
    UnlinkGarbage();
    bool collectedAny = UnrootGarbage();
    CleanupAfterCollection();

Where BeginCollection sets up the graph and adds the roots to the graph.  At that point, I suppose BeginCollection() is really more like PrepareForCollection().  I don't know if you think that is better or not. :)

I want to have all of these at the top level because in ICC there are 3 separate ways to trigger a CC:
1 - synchronously, like we do now, which runs through all of the phases
2 - incrementally, which runs a single one of these phases (eg RootGarbage)
3 - finish the current ICC, which says something like "hey, we're working on RootGarbage right now, so finish that off, then run the phases after it"

Having the top level phase thing just be a simple list of phases makes it easier to write those three pieces of code.
Attachment #801180 - Flags: review- → review+
You need to log in before you can comment on or make changes to this bug.