Last Comment Bug 799656 - Decrease b2g's javascript.options.mem.high_water_mark from 32mb to 6mb
: Decrease b2g's javascript.options.mem.high_water_mark from 32mb to 6mb
Status: RESOLVED FIXED
[MemShrink]
:
Product: Firefox OS
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Gregor Wagner [:gwagner]
:
Mentors:
Depends on:
Blocks: slim-fast
  Show dependency treegraph
 
Reported: 2012-10-09 13:20 PDT by Justin Lebar (not reading bugmail)
Modified: 2012-10-16 21:25 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
fixed


Attachments
Patch, v1 (1.01 KB, patch)
2012-10-09 13:22 PDT, Justin Lebar (not reading bugmail)
wmccloskey: review+
Details | Diff | Splinter Review
patch (1.71 KB, patch)
2012-10-09 18:18 PDT, Gregor Wagner [:gwagner]
wmccloskey: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2012-10-09 13:20:07 PDT
> pref("javascript.options.mem.high_water_mark", 32);

AIUI this means that 32mb of allocations trigger a GC.  In B2G, we should probably set this to something much smaller.

How about 4mb?
Comment 1 Justin Lebar (not reading bugmail) 2012-10-09 13:22:38 PDT
Created attachment 669707 [details] [diff] [review]
Patch, v1
Comment 2 Gregor Wagner [:gwagner] 2012-10-09 13:33:56 PDT
There are 2 javascript.options.mem.high_water_mark definitions in this file. Could you remove the 1st one? The current value should be 16.

I tried a smaller value but this resulted in too many GCs on my sgs2 when running some benchmarks but for the otoro that might be the right value.
Comment 3 Bill McCloskey (:billm) 2012-10-09 13:35:36 PDT
Comment on attachment 669707 [details] [diff] [review]
Patch, v1

I'm a little worried about the effect this patch will have on GC performance. GCs triggered by allocating too much malloc memory are always non-incremental. Also, I'd be afraid that we might just GC constantly on some sites. Could you do some light testing with this patch before checking it in, Justin?
Comment 4 Gregor Wagner [:gwagner] 2012-10-09 13:40:31 PDT
(In reply to Bill McCloskey (:billm) from comment #3)
> Comment on attachment 669707 [details] [diff] [review]
> Patch, v1
> 
> I'm a little worried about the effect this patch will have on GC
> performance. GCs triggered by allocating too much malloc memory are always
> non-incremental. Also, I'd be afraid that we might just GC constantly on
> some sites. Could you do some light testing with this patch before checking
> it in, Justin?

We really have to be careful here. Many graphic related allocations influence the dynamic malloc trigger and we could end up triggering many non-incremental GCs during scrolling or small animations.
Comment 5 Justin Lebar (not reading bugmail) 2012-10-09 15:07:57 PDT
Gregor, since you've looked at this in the past, can you do some testing on the Otoro to see whether 16 is the right value to use?  I clearly have no idea what to look for.  :)
Comment 6 Gregor Wagner [:gwagner] 2012-10-09 15:24:08 PDT
(In reply to Justin Lebar [:jlebar] from comment #5)
> Gregor, since you've looked at this in the past, can you do some testing on
> the Otoro to see whether 16 is the right value to use?  I clearly have no
> idea what to look for.  :)

We currently use 16MB. I don't know why we have a duplicated line in the file. But I can try with 4 and 8.
Comment 7 Gregor Wagner [:gwagner] 2012-10-09 16:54:29 PDT
(In reply to Justin Lebar [:jlebar] from comment #0)
> > pref("javascript.options.mem.high_water_mark", 32);
> 
> AIUI this means that 32mb of allocations trigger a GC.  In B2G, we should
> probably set this to something much smaller.
> 
> How about 4mb?

Just to make sure, this value defines the trigger for dynamically allocated memory and allocations in the JS-heap don't modify this trigger.
Comment 8 Gregor Wagner [:gwagner] 2012-10-09 17:09:49 PDT
4MB triggers non-incremental GCs during app startup (browser, sms, contacts).
I am trying 6MB now.
Comment 9 Gregor Wagner [:gwagner] 2012-10-09 18:18:01 PDT
Created attachment 669827 [details] [diff] [review]
patch

I think we should change it to 6MB.

With 6MB I see:
- a TOO_MUCH_MALLOC GC during phone startup with 90msec.
- sometimes a TOO_MUCH_MALLOC (50msec) and sometimes a MAYBE_GC during browser startup.
(MaybeGC is incremental)
- some TOO_MUCH_MALLOC GCs during surfing on nytimes. Sometimes we hit it in the parent and sometimes in the child. 



I found some interesting data during looking at the GC logs:
There is a global GC that takes 34 msec and afterwards an incremental GC that takes 217msec with a max pause time of 90 msec. We should look into that!


E/GeckoConsole(  106): GC(T+544.7) Total Time: 33.4ms, Compartments Collected: 1, Total Compartments: 132, MMU (20ms): 0%, MMU (50ms): 33%, SCC Sweep Total: 4.9ms, SCC Sweep Max Pause: 1.5ms, Reason: TOO_MUCH_MALLOC, Nonincremental Reason: malloc bytes trigger, Allocated: 15MB, +Chunks: 0, -Chunks: 0
E/GeckoConsole(  106):     Totals: Purge: 0.1ms, Mark: 13.5ms, Mark Roots: 8.3ms, Mark Weak: 0.2ms, Mark Gray: 0.5ms, Mark Gray and Weak: 0.1ms, Finalize Start Callback: 0.6ms, Sweep: 17.5ms, Sweep Compartments: 4.5ms, Sweep Tables: 3.4ms, Sweep Object: 0.4ms, Sweep Shape: 0.9ms, Sweep Discard Code: 0.2ms, Discard Analysis: 0.5ms, Discard TI: 0.3ms, Sweep Types: 0.2ms, Finalize End Callback: 9.2ms, End Callback: 0.2ms



E/GeckoConsole(  106): GC(T+547.6) Total Time: 217.5ms, Compartments Collected: 132, Total Compartments: 132, MMU (20ms): 0%, MMU (50ms): 0%, SCC Sweep Total: 44.9ms, SCC Sweep Max Pause: 9.8ms, Max Pause: 97.4ms, Allocated: 15MB, +Chunks: 0, -Chunks: 0
E/GeckoConsole(  106):     Slice: 0, Pause: 31.5 (When: 0.0ms, Reason: PAGE_HIDE): Purge: 0.2ms, Mark: 26.3ms, Mark Discard Code: 4.1ms, Mark Roots: 15.6ms
E/GeckoConsole(  106):     Slice: 3, Pause: 97.4 (When: 384.7ms, Reason: INTER_SLICE_GC): Mark: 40.3ms, Mark Weak: 0.3ms, Mark Gray: 39.8ms, Mark Gray and Weak: 0.3ms, Finalize Start Callback: 0.8ms, Sweep: 54.0ms, Sweep Compartments: 20.1ms, Sweep Tables: 10.4ms, Sweep Object: 19.7ms, Sweep String: 3.1ms, Sweep Script: 1.0ms, Sweep Shape: 0.2ms, Sweep Discard Code: 2.2ms, Discard Analysis: 6.3ms, Discard TI: 1.5ms, Free TI Arena: 0.4ms, Sweep Types: 2.4ms, Clear Script Analysis: 1.3ms, Finalize End Callback: 5.1ms
Comment 10 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-10-09 18:26:23 PDT
Do we have to use the same value for the main process and the child processes? I don't really know, but it seems like it would be better to have a higher limit in the parent than in the children. Maybe then we wouldn't have the slower full GCs as often in the parent?
Comment 11 Gregor Wagner [:gwagner] 2012-10-10 11:07:46 PDT
(In reply to ben turner [:bent] from comment #10)
> Do we have to use the same value for the main process and the child
> processes? I don't really know, but it seems like it would be better to have
> a higher limit in the parent than in the children. Maybe then we wouldn't
> have the slower full GCs as often in the parent?

GCs in the browser child are mostly triggered by SET_NEW_DOCUMENT and CC_WAITING afterwards. Sometimes also TOO_MUCH_MALLOC now with this patch. That seems about right.

We have a new situation in the parent. In our desktop browser we try to rely on PAGE_HIDE, CC_WAITING or SET_NEW_DOCUMENT GCs and not on allocation based triggers because they mostly occur during page load.
So our allocation based triggers are not too strict but it seems the parent has to rely on them now.
I see some TOO_MUCH_MALLOC GCs but never MAYBE_GCs in the parent. I guess we should also tighten the heap based allocation triggers.
Comment 12 Justin Lebar (not reading bugmail) 2012-10-10 11:29:33 PDT
Did you mean to unset the blocking flag?  (Bugzilla may be silently unsetting the flag when you comment on a bug which has been in the meantime marked blocking+.)
Comment 13 Gregor Wagner [:gwagner] 2012-10-10 17:24:50 PDT
Assigning to me so I can take the blame if something goes wrong :)
Comment 15 Gregor Wagner [:gwagner] 2012-10-10 22:01:43 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/e3f7fdf150a4
Comment 16 Ed Morley [:emorley] 2012-10-11 12:08:23 PDT
https://hg.mozilla.org/mozilla-central/rev/07edb74f85cd

Note You need to log in before you can comment on or make changes to this bug.