Decrease b2g's javascript.options.mem.high_water_mark from 32mb to 6mb

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: gwagner)

Tracking

(Blocks: 1 bug)

unspecified

Firefox Tracking Flags

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
> 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?
(Reporter)

Updated

5 years ago
Blocks: 797189
(Reporter)

Updated

5 years ago
blocking-basecamp: --- → ?
(Reporter)

Comment 1

5 years ago
Created attachment 669707 [details] [diff] [review]
Patch, v1
Attachment #669707 - Flags: review?(wmccloskey)
(Reporter)

Updated

5 years ago
Assignee: nobody → justin.lebar+bug
Whiteboard: [MemShrink]
(Assignee)

Comment 2

5 years ago
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 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?
Attachment #669707 - Flags: review?(wmccloskey) → review+
(Assignee)

Comment 4

5 years ago
(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.
(Reporter)

Comment 5

5 years ago
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.  :)
(Assignee)

Comment 6

5 years ago
(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.
(Assignee)

Comment 7

5 years ago
(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.
(Assignee)

Comment 8

5 years ago
4MB triggers non-incremental GCs during app startup (browser, sms, contacts).
I am trying 6MB now.
(Assignee)

Comment 9

5 years ago
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
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?
blocking-basecamp: ? → +
(Assignee)

Comment 11

5 years ago
(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.
blocking-basecamp: + → ?
(Reporter)

Comment 12

5 years ago
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+.)
blocking-basecamp: ? → +
(Assignee)

Updated

5 years ago
Attachment #669827 - Flags: review?(wmccloskey)
Attachment #669827 - Flags: review?(wmccloskey) → review+
(Assignee)

Updated

5 years ago
Attachment #669707 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Assigning to me so I can take the blame if something goes wrong :)
Assignee: justin.lebar+bug → anygregor
Summary: Decrease b2g's javascript.options.mem.high_water_mark from 32mb to something smaller → Decrease b2g's javascript.options.mem.high_water_mark from 32mb to 6mb
(Assignee)

Comment 14

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/07edb74f85cd
(Assignee)

Comment 15

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/e3f7fdf150a4
https://hg.mozilla.org/mozilla-central/rev/07edb74f85cd
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
status-firefox18: --- → fixed
status-firefox19: --- → fixed
You need to log in before you can comment on or make changes to this bug.