913527 - Simplify top level invocation of cycle collector phases

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: part 1 - Get rid of FinishCollection. r=smaug

I'm simplifying the top level invocation of CC phases in these patches to make it easier to resume an ICC.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: part 2 - Remove empty CC graph optimization. r=smaug

Try run for these patches: https://tbpl.mozilla.org/?tree=Try&rev=cd687180be3a

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: part 3 - Check for collection in progress in Collect. r=smaug

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 5

[Andrew McCreight [:mccr8]]

(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.

Comment 6

[Andrew McCreight [:mccr8]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c65a5025b50a
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/41888e2e355d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/33e3cbca0437

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c65a5025b50a
https://hg.mozilla.org/mozilla-central/rev/41888e2e355d
https://hg.mozilla.org/mozilla-central/rev/33e3cbca0437