Continued from bug 1034910. This is just a variant of my patch in bug 1034611. This probably isn't going to be enough for B2G, but it should at least help with desktop GGC stability. I'm going to reduce the nursery size to 4MB, from 16MB.

I suggest this be pref-controlled so that we can tweak for different devices.

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #1) > I suggest this be pref-controlled so that we can tweak for different devices. Yeah, definitely. It may be a followup bug depending on whether I can quickly figure out how to get a mainthread pref over to the worker.

try run: https://tbpl.mozilla.org/?tree=Try&rev=f136c8fe434e This first patch is just a refactoring. It shouldn't alter the behavior. I split out the actual change in worker nursery size to a separate patch to make it easier to back out if something goes wrong.

Comment on attachment 8454653 [details] [diff] [review] part 1 - Hoist JS runtime creation out of CycleCollectedJSRuntime. Review of attachment 8454653 [details] [diff] [review]: ----------------------------------------------------------------- I would strongly prefer that we just added another parameter to the ctor. Why do you want to do it this way?

Pulling JS_NewRuntime() out of the ctor will make it easier deal with it failing in a fallible way. But I guess I could always bail early from the ctor in the case of failure, and then add some kind of method to check if the ctor succeeded. So I can just add another parameter if you prefer it.

Ah, so the goal is to sneak bug 1034611 in here too?

I was just moving some of my planned refactoring for that in here, not actually making it fallible yet. But if you don't think that's the way to go for that, I can do this differently.

I'm perfectly happy to kick the can down the road on refactoring for fallibility. This just adds a new argument for the nursery size. The next patch will reduce the size.

Kyle: review ping. I could also get Olli to review it if you are busy. I assume you are okay with this approach because I basically just did what you suggested.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/40e39eea4621 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/cd748cd23efc

https://hg.mozilla.org/mozilla-central/rev/40e39eea4621 https://hg.mozilla.org/mozilla-central/rev/cd748cd23efc

It looks like this improved various TP5 memory stats by 8% to 18%, which I guess makes sense if there's a worker anywhere in there.

There's the OS.File worker if nothing else. Can you nominate this for approvals?

Yeah, I was going to wait a day or two in case there are any horrible regressions. I'll needinfo myself so I don't forget.

> There's the OS.File worker if nothing else. And SessionWorker.js. And PageThumbsWorker.js comes and goes.

Comment on attachment 8455475 [details] [diff] [review] part 1 - Add nursery size as a parameter of CycleCollectedJSRuntime. (this request is for part 1 and 2) There are no crashes of at least one signature of bug 991845 yesterday, so that's something. Approval Request Comment [Feature/regressing bug #]: generational GC [User impact if declined]: OOM crashes when creating workers, like in bug 991845 (top crash) [Describe test coverage new/current, TBPL]: workers are heavily tested [Risks and why]: This could hurt worker performance somewhat, but that's probably not a huge deal. Part 2 is the only part that changes behavior, and it would be easy to back out. [String/UUID change made/needed]: none

Comment on attachment 8455477 [details] [diff] [review] part 2 - Reduce GGC nursery size to 1MB on workers. cf comment #18

https://hg.mozilla.org/releases/mozilla-aurora/rev/e646f8f8e027 https://hg.mozilla.org/releases/mozilla-aurora/rev/3cb4eb9b4d3a https://hg.mozilla.org/releases/mozilla-beta/rev/84bf8e0aaecb https://hg.mozilla.org/releases/mozilla-beta/rev/e61371be0f38

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/84bf8e0aaecb https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e61371be0f38