1017141 - Make MAX_EMPTY_CHUNK_COUNT a GC tunable for B2G

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

This is currently an #ifdef, because it was needed urgently. We should make this a tunable to reduce our #ifdef load and to allow us to tailor the value to the memory size on a per-device basis.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1017141-chunk-count-params

Here's a patch to turn these into GC parameters.

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 8455327 [details] [diff] [review]
bug1017141-chunk-count-params

Review of attachment 8455327 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsgc.cpp
@@ +250,2 @@
>  #else
> +static const unsigned DEFAULT_MAX_EMPTY_CHUNK_COUNT = 30;

Actually making and using the pref is trivial from here, so let's go ahead and do that in this patch too.

First, add a new javascript.options.mem.gc_ in nsJSEnvironment.cpp (around like 3000 or so). Then you can add the pref in mobile/android/app/mobile.js and b2g/app/b2g.js. That should be pretty much all that's needed.

Comment 3

[Terrence Cole [:terrence]]

Ben, I expect you'll want to make use of this new preference in workers, at least until we can implement a shared per-process chunk pool.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Yes, definitely.

Adding to http://mxr.mozilla.org/mozilla-central/source/dom/workers/RuntimeService.cpp#454 shouldn't be too difficult I don't think (workers already watch a bunch of "javascript.options.mem" prefs), but if you'd rather have someone else do it in a separate bug that's fine too.

Comment 5

[Jon Coppeard (:jonco)]

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #4)
No problem, I can see how to handle adding new prefs.  But where are these prefs read from?  I couldn't find the config file.

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

They come from the same prefs file as the main thread DOM stuff (all.js, etc.)

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Oh, so if you want worker-specific overrides you'd add "dom.workers.options.mem.foo" and that would override whatever was in "javascript.options.mem.foo" automatically. Both would go in all.js.

Comment 8

[Jon Coppeard (:jonco)]

Attachment: 1 - add GC params

Just the js/src changes - add parameters for min/max empty chunk count.

Comment 9

[Jon Coppeard (:jonco)]

Attachment: 2 - Set the prefs from the browser

Set the preferences from the browser to their current values.

I haven't added any worker-specific ones, but could easily do this too.

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8456237 [details] [diff] [review]
2  - Set the prefs from the browser

Review of attachment 8456237 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

Comment 11

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1b29d99921e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/40e75ff09974

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1b29d99921e9
https://hg.mozilla.org/mozilla-central/rev/40e75ff09974