604747 - TM: shell GC heap size should match browser heap size

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: patch

This patch sets the heap size of the shell to 512MB and gcMaxMallocBytes back to 64MB.

Comment 2

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 3

[Paul Wright]

I think this is important to get implemented quickly, for multiple reasons:

1.  Test environment should mimic actual browser as closely as possible
2.  Using the data, the browser can be tuned for better real-world performance
3.  Many people, including (until recently) myself, took AWFY at face value.  If the shell is not truly representative of real-world browser behavior, aren't we doing ourselves an injustice by using it as the "benchmark"?

Are there any other areas and/or settings where the shell is not representative of the browser?  Are there plans (bugs filed) to make it so?

If I am out of line here, I apologize.  It just seems to me that we should be measuring progress based on what users will experience, not just in a test environment.

Paul

Comment 4

[Gregor Wagner [:gwagner]]

I was absolutely sure that increasing the JS heap size makes sense... Until I did the measurements for bug 606637.
There we see that the benchmarks for the shell and the browser have totally different memory requirements.
So you are absolutely right with your questions!

We can avoid all GC runs during the v8 benchmark in the shell version and make it faster but there is no way to avoid them in the browser version.

Fact is that the browser versions of the benchmarks are more important than the shell versions. 
Removing all GC runs in the shell version also means that we don't see GC regressions right away.

There is a tradeoff between:
1) Yes it makes sense to increase the heap size because we would see how the VM would handle a bigger heap. And yes we are faster on AWFY.

2) No it makes no sense because the working sets for all the shell programs is too small to mimic the browser benchmark environment. Furthermore we don't see the GC performance in the benchmarks that we run every day.

Another major problem is that the shell versions of the benchmarks don't mimic the actual browser versions. And even worse, the browser versions don't even mimic the actual web behavior... So what are we tuning for? :)

I could write a big section about how I think the compartmental GC will help  but lets wait for the real numbers.

Back to this bug: This patch is ready but I am not so sure if this is the way to go.

Comment 5

[Andreas Gal :gal]

We have 2 separate harnesses and issues here:

- The sunspider web harness runs short little benchmarks that don't do much. We should make sure we don't hit a last ditch while the benchmarks run. Instead, we should schedule preventive GCs at better times. This will help web content, too.

- The v8 web harness measures rates, and is mostly a GC benchmark. It creates tons of data and uses V8's generational GC to look fast. Yay! We can't win this competition by increasing the heap size. The compartmental GC will make us faster on last ditch GCing, and should help us with the v8 web harness.

Executive summary: lets make sure we don't GC during sunspider harness runs. The v8 harness is secondary and we will deal with it for b8.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch to set maxheap to 4GB

I recently found that the difference in maxheap between the shell and browser was causing massive differences in performance on splay. This patch sets maxheap to 4GB, the same as it is in the browser. With the patch, the shell and browser now give me the same score. (This is using the shell version of V8bench that is packaged with V8 as benchmarks/run.js, not the crappy SunSpider version.)

I think we should take this because it makes benchmarking in the shell more realistic.

Comment 7

[Gregor Wagner [:gwagner]]

Comment on attachment 560668 [details] [diff] [review]
patch to set maxheap to 4GB

lets try it!

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9208ee94b012

I ended up changing this to be closer to what the browser does. It allocates the runtime with a 32MB max heap size, and then increases it to 4GB. This way max malloc bytes gets set to 32MB rather than 4GB (which would be undesirable).

Comment 9

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/9208ee94b012