Closed Bug 1017141 Opened 8 years ago Closed 8 years ago

Make MAX_EMPTY_CHUNK_COUNT a GC tunable for B2G


(Core :: JavaScript: GC, defect)

Not set





(Reporter: terrence, Assigned: jonco)


(Blocks 1 open bug)



(2 files, 1 obsolete file)

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.
Blocks: GC.size
Attached patch bug1017141-chunk-count-params (obsolete) — Splinter Review
Here's a patch to turn these into GC parameters.
Assignee: nobody → jcoppeard
Attachment #8455327 - Flags: review?(terrence)
Comment on attachment 8455327 [details] [diff] [review]

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.
Attachment #8455327 - Flags: review?(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.
Flags: needinfo?(bent.mozilla)
Yes, definitely.

Adding to 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.
Flags: needinfo?(bent.mozilla)
(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.
Flags: needinfo?(bent.mozilla)
They come from the same prefs file as the main thread DOM stuff (all.js, etc.)
Flags: needinfo?(bent.mozilla)
Oh, so if you want worker-specific overrides you'd add "" and that would override whatever was in "" automatically. Both would go in all.js.
Just the js/src changes - add parameters for min/max empty chunk count.
Attachment #8455327 - Attachment is obsolete: true
Attachment #8456236 - Flags: review?(terrence)
Set the preferences from the browser to their current values.

I haven't added any worker-specific ones, but could easily do this too.
Attachment #8456237 - Flags: review?(bent.mozilla)
Comment on attachment 8456237 [details] [diff] [review]
2  - Set the prefs from the browser

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

Looks good!
Attachment #8456237 - Flags: review?(bent.mozilla) → review+
Attachment #8456236 - Flags: review?(terrence) → review+
Blocks: 1118560
You need to log in before you can comment on or make changes to this bug.