Right now we may end up calling CCRunnerFired quite a few times before deciding to stop it or call CC - meaning there are quite some wake-ups even if the browser is otherwise rather idle.
I was thinking that maybe we could finish sooner, just try to find some slice which would do some work. Or the recursive call will just kill the CCRunner if there isn't anything to do.
Note, the earlier 'else if' still forces the most important forgetSkippable calls and such.

Calling the method recursively is just the easiest way to implement the approach.
The return value of the method is true, if some work was done.

(still away from my main machine, so moz-phab doesn't work. Misusing data-review)

yes. That is the whole point. Ending up to late timer fires, and skipping some of the earlier possible forget skippables.

I thought the comment was quite clear.
Any ideas how you'd improve this?

I'm not worried about max recursion depth here.

And to clarify still, isLateTimerFire is when interesting stuff happens. Either we decide to stop, or do still another run which starts CC. I'm not sure how to make the comment better, and calling the method recursively is rather clean way to achieve the behavior.

I still don't really understand what this is doing. Isn't this the same as just setting sCCRunnerFireCount to numEarlyTimerFires + 1? The only other conditions I see involve sPreviousSuspectedCount and sCleanupsSinceLastGC, and those aren't going to change when we recurse, or the passage of time, and I assume those aren't intended to fire either. I would think that if the CC isn't doing much of anything because there's nothing to do, we'd be better off delaying the next check, rather than doing more work immediately.

Setting sCCRunnerFireCount to sCCRunnerFireCount + 1 and calling the stuff again, until we end up to the
late fires where we actually do something.
And once we're at the late firing case, we call ShouldTriggerCC(), which may lead to still another forget skippable, and then possibly stop the forget skippable cycle, or continue to CC.

The issue is that we have too many CCRunnerFired calls which don't do anything (return false). That causes tons of wake ups.
If we do have idle time, better to just try to run CC if possible, or stop.

And I did verify that we often do end up running
if (ShouldTriggerCC(nsCycleCollector_suspectedCount())) {
... this
}

since there can be enough suspected objects to trigger CC. After that there would another asynchronous
call to CCRunnerFired to possibly trigger CC.
But we wouldn't have so many CCRunnerFired calls which don't do anything.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c233ebdc8906d5e14b7b38a9a0945efa8f132eeb

CYCLE_COLLECTOR_TIME_BETWEEN should regress if the patch behaves badly.

https://hg.mozilla.org/mozilla-central/rev/5c868e3f1c12

== Change summary for alert #20880 (as of Thu, 09 May 2019 11:49:18 GMT) ==

Improvements:

7% raptor-tp6-docs-firefox loadtime linux64-shippable opt 1,035.48 -> 965.33

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20880