Update the table of RAM vs number of renderers...

Update the table of RAM vs number of renderers
- revised my guesstimate of 25 MB per renderer to 40MB, which I think is more appropiate for long lived tabs, since short lived tabs don't really matter here.
- the lower end (< 1GB) has less total renderers
- the middle has about the same
- the upper end (> 2GB) has more renderers
- added test to open 34 tabs.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10024

[M-A Ruel, 2009-02-19 01:14:54]

http://codereview.chromium.org/21484/diff/7/10
File chrome/browser/browser_uitest.cc (right):

http://codereview.chromium.org/21484/diff/7/10#newcode93
Line 93: if (in_process_renderer())
I think the test should run with in_process_renderer(). The number of child
processes should be 0.

http://codereview.chromium.org/21484/diff/7/10#newcode112
Line 112: EXPECT_LE(process_count, 22);
It will fail on Pawel's machine (he has 512 megs of RAM?) Make the test more
complete.

http://codereview.chromium.org/21484/diff/7/8
File chrome/browser/renderer_host/render_process_host.cc (right):

http://codereview.chromium.org/21484/diff/7/8#newcode24
Line 24: static const int kMaxRenderersByRamTier[] = {
To help the test, you could declare this constant in render_process_host.h and
not make it static.

[cpu_(ooo_6.6-7.5), 2009-02-19 02:43:49]

http://codereview.chromium.org/21484/diff/7/10
File chrome/browser/browser_uitest.cc (right):

http://codereview.chromium.org/21484/diff/7/10#newcode93
Line 93: if (in_process_renderer())
On 2009/02/19 01:14:55, M-A wrote:
> I think the test should run with in_process_renderer(). The number of child
> processes should be 0.

Done.

http://codereview.chromium.org/21484/diff/7/10#newcode112
Line 112: EXPECT_LE(process_count, 22);
On 2009/02/19 01:14:55, M-A wrote:
> It will fail on Pawel's machine (he has 512 megs of RAM?) Make the test more
> complete.

I don't think it will fail because I am testing that process_count < 22 , or at
least that is what I think I am testing.

The test was aiming mostly at asserting that more than 31 rendereres (our old
limit) did not cause a catastrophic (read crash) situation. Not trying to probe
each entry on the table.

http://codereview.chromium.org/21484/diff/7/8
File chrome/browser/renderer_host/render_process_host.cc (right):

http://codereview.chromium.org/21484/diff/7/8#newcode24
Line 24: static const int kMaxRenderersByRamTier[] = {
On 2009/02/19 01:14:55, M-A wrote:
> To help the test, you could declare this constant in render_process_host.h and
> not make it static.

In light of the comments on the test, I don't see the need to make this public.
Or at least I was not trying to make the test exhaustive.

[M-A Ruel, 2009-02-19 12:33:56]

lgtm with an optional nit

http://codereview.chromium.org/21484/diff/1007/16
File chrome/browser/browser_uitest.cc (right):

http://codereview.chromium.org/21484/diff/1007/16#newcode107
Line 107: return;
nit: Why not EXPECT_EQ(process_count, 0); ?

[darin (slow to review), 2009-02-19 18:40:01]

http://codereview.chromium.org/21484/diff/1007/14
File chrome/browser/renderer_host/render_process_host.cc (right):

http://codereview.chromium.org/21484/diff/1007/14#newcode39
Line 39: 40                        // 3584MB
40 seems too high to me.  can we just double our previous limits for a start?

[darin (slow to review), 2009-02-19 18:41:20]

i see... 20 x 2 == 40 :)  i meant, increase by 50%, so max of 30 instead of 20
in the high memory case.