The default value of LIFO_ALLOC_PRIMARY_CHUNK_SIZE is too big (128K) that almost all compartments do not use it up. It wastes a lot of memory (~10%). By cutting down LIFO_ALLOC_PRIMARY_CHUNK_SIZE for compartments, it can save a lot of memory. By setting LIFO_ALLOC_PRIMARY_CHUNK_SIZE to 32K, the memory size for analysis-temporary comes down to 1.8MB from original ~5MB.

The tradeoff here is that smaller chunks cause longer GC pauses, but maybe that is only a real problem on OSX. Pretty amazing that it is using up that much memory though!

75% of compartments do not use more than 32K for analysis-temporary.

Good catch, Thinker! The size was set to 128 KiB in bug 699668 after lots of discussion. However, that was before compartment-per-global. (Indeed, see the last sentence in bug 699668 comment 26.) 32 KiB seems a good starting point for re-evaluation. billm, are you able to do some experiments like the ones you did in bug 699668 comment 25 to determine if 32 KiB is reasonable, and whether 16 KiB would be ok?

We may incrementally discard these structures now, come to think of it.

Yes, we free this stuff on the background thread now. So I think 16K should be fine.

> Yes, we free this stuff on the background thread now. So I think 16K should > be fine. Great! Do you want to do experiments before landing a change?

Nice! :)

I talked to billm. The original problem with all this was that free() is really slow on Mac. But because of the background freeing he thinks it shouldn't be a problem anymore. Nonetheless, he'd like to land a patch for this on mozilla-central, and then wait a week to see what the telemetry data looks like. Assuming it doesn't cause problems, we can then backport it to Aurora.

I compared 128, 32 and 16 KiB arenas on Mac64 desktop. My process: 1. start browser, open about:memory, get measurement 2. hit "minimize memory usage", get measurement 3. open techcrunch.com in a second tab, hit "update" in about:memory, get measurement 4. hit "minimize memory usage", get measurement Results: 128 KiB arenas range -------------- ----- 1. 8.5 6.6 19.6 19.6 19.6 6.6--19.6 2. 0.4 0.4 0.4 0.4 0.4 0.4 3. 3.4 3.4 10.4 15.2 2.2 2.2--15.2 4. 2.0 1.9 1.8 1.8 2.3 1.8-- 2.3 32 KiB arenas range ------------- ----- 1. 4.8 4.0 11.4 11.5 11.5 4.0--11.5 2. 0.2 0.2 0.2 0.2 0.2 0.2 3. 1.2 1.4 4.8 4.4 4.5 1.2--4.8 4. 0.6 0.6 0.6 0.7 0.5 0.5--0.7 16 KiB arenas range ------------- ----- 1. 4.3 3.8 10.7 10.7 10.7 3.8--10.7 2. 0.2 0.2 0.2 0.2 0.2 0.2 3. 3.7 0.9 4.7 1.8 2.4 0.9--4.7 4. 0.4 0.4 0.4 0.5 0.4 0.4-- 0.5 The improvement between 32 and 16 is small enough that I suggest we change it to 32, to minimize the chance of GC time regressions.

Shrink it from 128 KiB to 32 KiB.

> 128 KiB arenas range > -------------- ----- > 1. 8.5 6.6 19.6 19.6 19.6 6.6--19.6 > 2. 0.4 0.4 0.4 0.4 0.4 0.4 > 3. 3.4 3.4 10.4 15.2 2.2 2.2--15.2 > 4. 2.0 1.9 1.8 1.8 2.3 1.8-- 2.3 I failed to mention that measurements 1 and 3 are volatile, which is why I did all the measurements five times.

\o/

Setting flags so we hopefully don't forget to uplift this one.

billm, can you look at the telemetry to see if any bad effects have occurred? Thanks.

It looks good to me. I think it's ready for aurora.

This needs either blocking-basecamp or aurora approval, as far as I understand.

Comment on attachment 674842 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): N/A. User impact if declined: higher memory usage, esp. on B2G. Testing completed (on m-c, etc.): has been on m-c for several days without problem; billm has checked telemetry data to confirm that an unlikely-but-possible GC time regression didn't occur. Risk to taking this patch (and alternatives if risky): negligible. The patch changes a single constant that controls the size of chunks of memory used for type inference analysis. String or UUID changes made by this patch: none.

Comment on attachment 674842 [details] [diff] [review] patch [Triage Comment] Thanks for looking into whether or not this caused a new performance regression for desktop. Given that analysis, and the risk evaluation here, approving for Aurora 18.

This was backed out while investigating Android timeouts and re-landed after it was deemed not at fault. https://hg.mozilla.org/releases/mozilla-aurora/rev/50cee6bb0eea