1865383 - Initial GC slice often overrun its budget on youtube.com

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement)

Description

[Jon Coppeard (:jonco)]

While working on parallel marking I've been profiling browser GCs for a couple popular sites and I noticed that on youtube.com the initial slice often overruns the budget. The budget is 5ms and discarding JIT code often takes longer than that, but then we also spend a couple of ms marking as well.

The problem is that we don't check the slice budget until we've done some marking, but which time we are already well over it. This is compounded by the fact that one of the first things we mark for this site seems to be an enormous script that means we don't check the budget for a while.

We should check the budget as soon as we start marking to reduce the severity of overruns.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1865383 - Check slice budget when we start marking or sweeping r?sfink

The patch forces a budget check when we start marking or sweeping unless we
have just started a GC slice in the current state.

This turned out more complicated than expected because the change caused a test
that called gcslice(1) in a loop to not make any progress. This was because the
original stepAndForceCheck() method ate the single unit of work budget. The fix
was to remove the step part of this method. I don't think any of the callers
need it to step the budget as well.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4b1c9fc6509
Check slice budget when we start marking or sweeping r=sfink

Comment 3

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/b4b1c9fc6509

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1865383 - Ensure there is no more marking at the start of sweeping r?sfink

There are two issues here. The first is that there unexpected marking work at
the start of sweeping after entering from the mark phase without yielding. We
previously called assertNoMarkingWork() after markUntilBudgetExhausted() in the
marking phase so something since then must have added it.

As far as I can tell this must be the conditional call to
collectNurseryFromMajorGC(), where a post barrier for a pointer cleared during
finalization (e.g. for Maps in mapObject::sweepAfterMinorGC) ends up marking
something. I'm not sure such barriers are necessary, but for now the safest
thing to do is to move this nursery collection to the start of the slice so
that it happens before we drain the mark stack.

The second issue is that we check the budget and conditionally yield if we
enter from the marking state. The comment above this code states that this is
not safe since we have not yet started sweeping a sweep group. This check was
added in bug 1865383 but was not the main part of the fix. I think we should
remove this.

I wasn't able to come up with a test case to reproduce this.

Comment 5

[Phabricator Automation]

Comment on attachment 9411562 [details]
Bug 1865383 - Ensure there is no more marking at the start of sweeping r?sfink

Revision D215925 was moved to bug 1871303. Setting attachment 9411562 [details] to obsolete.