1529306 - Fix frequent jsreftest OOMs on Win32

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1529306 - Check sFullGCTimer when JS code calls WindowUtils.runNextCollectorTimer() explicitly. r?mccr8!

This fixes frequent jsreftest OOMs on Win32 after bug 1527905 landed. We have a
lot more zones now but we weren't firing full GCs for a long time in some cases,
resulting in OOMs.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1529306 - Trigger a full GC if PokeGC was called on a large number of zones since the last full GC. r?mccr8!

This fixes frequent jsreftest OOMs on Win32 after bug 1527905 landed. We have a
lot more zones now and we weren't collecting old zones fast enough. Full GCs
ensure we collect them.

Comment 3

[Jan de Mooij [:jandem]]

The last patch doesn't help actually on Win32 :/

However I realized something: in nsJSContext::RunNextCollectorTimer we run timers in this order:

1. sGCTimer
2. sICCRunner
3. sCCRunner
4. sInterSliceGCRunner

Now what often happens is that we have a CC runner and that keeps us from running sInterSliceGCRunner more frequently.

The comments in that function actually mention this:

// Check the CC timers after the GC timers, because the CC timers won't do
// anything if a GC is in progress.

That's not what happens with sInterSliceGCRunner though. Moving sInterSliceGCRunner after sGCTimer in the list above fixes the oranges on Try on Win32 (I still have to make sure it doesn't time-out in debug builds!).

Olli, you changed this a while ago. Any thoughts?

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1529306 - Trigger GC timers before CC timers in nsJSContext::RunNextCollectorTimer. r?mccr8!

This fixes frequent jsreftest OOMs on Win32 because we weren't triggering GC
frequently enough. Bug 1377131 changed the timer ordering in this code and might
have regressed this.

Comment 5

[Jan de Mooij [:jandem]]

I'd ask smaug to review but he's not available.

Bug 1377131 refactored this code and changed the order of the timers we check here:

https://hg.mozilla.org/integration/mozilla-inbound/rev/10405ae76f20#l3.92

It looks like this might have been an unintentional side-effect from the refactoring. It also reversed the CC vs ICC timer checks but I left those alone in this patch.

Comment 6

[Jan de Mooij [:jandem]]

Oh and this looks green on Try, Talos/Raptor seems unaffected.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64dd671a7a60
Trigger GC timers before CC timers in nsJSContext::RunNextCollectorTimer. r=mccr8

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/64dd671a7a60