623072 - Reduce JSGC_MAX_MALLOC_BYTES usage

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Doug Turner (:dougt)]

Attachment: rss usages reported at .25s over ss

Recently, JSGC_MAX_MALLOC_BYTES was increased to 80mb.  I added support to make this configurable.  The original value was 32mb.  The difference between 40 and 20 makes little difference in terms of memory usage, but does slow down ss.  The following patch sets JSGC_MAX_MALLOC_BYTES to 40mbs.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 501194 [details] [diff] [review]
patch v.1

this patch will cause perf regressions over benchmarks due to more frequent gc as memory rises.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 501194 [details] [diff] [review]
patch v.1

So if I understand correctly, we are currently running with a high_water_mark = 32 and I don't see any negative effect on the Tsunspider talos test. Why not just keep it at 32?

Comment 4

[Doug Turner (:dougt)]

default  is 80.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #4)
> default  is 80.

high_water_mark? What about http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/init/all.js#599

Comment 6

[Doug Turner (:dougt)]

mfinkle, the existing code, when 32 was set, would force the watermark to actually be 80 and the max allocation to be -1 (inf).

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

show should we just set this to 32 and try it?

Comment 8

[Doug Turner (:dougt)]

Sounds fine.  r+ on that change?

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 501194 [details] [diff] [review]
patch v.1

r+ for setting it to 32 to start with

Comment 10

[Doug Turner (:dougt)]

http://hg.mozilla.org/mobile-browser/rev/8d55cbf030a4

Comment 11

[Tony Chung [:tchung]]

doug, anything we need to look out for when testing this fix?   stability? oom? perf?

Comment 12

[Doug Turner (:dougt)]

memory usage should be lower when compared to builds without this change.

Comment 13

[Tony Chung [:tchung]]

Ran a handful of startup tests, open tabs, and running extensions on on the 1/10 nightly vs fennec beta 3.  The nightly seemed to consistently display lower memory usage (using process manager app).   

marking verified on Mozilla/5.0 (Android; Linux armv71; rv:2.0b9pre) Gecko/20110110 Firefox/4.0b9pre Fennec/4.0b4pre	

Can someone verify this against maemo builds?

Comment 14

[Andreea Pod]

How can be verified this on maemo?