Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 604747 - TM: shell GC heap size should match browser heap size
: TM: shell GC heap size should match browser heap size
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla9
Assigned To: Gregor Wagner [:gwagner]
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: 604749
  Show dependency treegraph
Reported: 2010-10-15 13:10 PDT by Gregor Wagner [:gwagner]
Modified: 2011-09-20 07:46 PDT (History)
11 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (1.02 KB, patch)
2010-10-15 13:29 PDT, Gregor Wagner [:gwagner]
no flags Details | Diff | Splinter Review
patch (1.02 KB, patch)
2010-10-15 13:30 PDT, Gregor Wagner [:gwagner]
no flags Details | Diff | Splinter Review
patch to set maxheap to 4GB (699 bytes, patch)
2011-09-16 19:57 PDT, Bill McCloskey (:billm)
anygregor: review+
Details | Diff | Splinter Review

Description Gregor Wagner [:gwagner] 2010-10-15 13:10:12 PDT

Comment 1 Gregor Wagner [:gwagner] 2010-10-15 13:29:17 PDT
Created attachment 483580 [details] [diff] [review]

This patch sets the heap size of the shell to 512MB and gcMaxMallocBytes back to 64MB.
Comment 2 Gregor Wagner [:gwagner] 2010-10-15 13:30:02 PDT
Created attachment 483581 [details] [diff] [review]
Comment 3 Paul Wright 2010-10-22 16:00:27 PDT
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.

Comment 4 Gregor Wagner [:gwagner] 2010-10-23 00:10:49 PDT
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 2010-10-23 11:01:05 PDT
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 (:billm) 2011-09-16 19:57:07 PDT
Created attachment 560668 [details] [diff] [review]
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] 2011-09-19 11:32:13 PDT
Comment on attachment 560668 [details] [diff] [review]
patch to set maxheap to 4GB

lets try it!
Comment 8 Bill McCloskey (:billm) 2011-09-19 15:30:55 PDT

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 :Ehsan Akhgari (Away Oct 25 - Nov 9) 2011-09-20 07:46:37 PDT

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