Grow old generation slower on low-memory devices.

Grow old generation slower on low-memory devices.

BUG=
R=mstarzinger@chromium.org, rmcilroy@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=20837

[Hannes Payer (out of office), 2014-04-15 11:32:50]

Fix for the low-memory devices.

[Michael Starzinger, 2014-04-15 12:11:50]

LGTM from my end. But it would be nice if Ross could also take a look.

https://codereview.chromium.org/236063015/diff/1/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/236063015/diff/1/src/heap.cc#newcode5958
src/heap.cc:5958: if (max_old_generation_size_ <= 256 * MB) {
I personally am fine with hard-coding this constant. But I am not sure how much
this duplicates "magic constants" between embedders and V8. I am deferring to
Ross' opinion about this.

https://codereview.chromium.org/236063015/diff/1/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/236063015/diff/1/src/heap.h#newcode1890
src/heap.h:1890: int old_generation_growing_factor_;
nit: Can we haz short comment of the semantics?

[rmcilroy, 2014-04-15 12:43:01]

+1 to Michael's comment too.

https://codereview.chromium.org/236063015/diff/1/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/236063015/diff/1/src/heap.cc#newcode5958
src/heap.cc:5958: if (max_old_generation_size_ <= 256 * MB) {
On 2014/04/15 12:11:50, Michael Starzinger wrote:
> I personally am fine with hard-coding this constant. But I am not sure how
much
> this duplicates "magic constants" between embedders and V8. I am deferring to
> Ross' opinion about this.

I'm fine with this being hard-coded as well, but could we extract a constant
somewhere for the 256MB and use it both here and in
ResourceConstraints::ConfigureDefaultS? That way, if we decide to change the
heap size of a low-mem device it will change both places.

[Hannes Payer (out of office), 2014-04-16 07:52:35]

- I added constants for all configuration parameters to heap.
- I ended up renaming some inconsistent variables and functions: young_space ->
new_space, old_generation -> old_space. There is more of that in heap.*, I will
do that in a separate cl.

[Hannes Payer (out of office), 2014-04-16 07:53:01]

https://codereview.chromium.org/236063015/diff/1/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/236063015/diff/1/src/heap.cc#newcode5958
src/heap.cc:5958: if (max_old_generation_size_ <= 256 * MB) {
On 2014/04/15 12:11:50, Michael Starzinger wrote:
> I personally am fine with hard-coding this constant. But I am not sure how
much
> this duplicates "magic constants" between embedders and V8. I am deferring to
> Ross' opinion about this.

Done.

https://codereview.chromium.org/236063015/diff/1/src/heap.cc#newcode5958
src/heap.cc:5958: if (max_old_generation_size_ <= 256 * MB) {
On 2014/04/15 12:43:01, rmcilroy wrote:
> On 2014/04/15 12:11:50, Michael Starzinger wrote:
> > I personally am fine with hard-coding this constant. But I am not sure how
> much
> > this duplicates "magic constants" between embedders and V8. I am deferring
to
> > Ross' opinion about this.
> 
> I'm fine with this being hard-coded as well, but could we extract a constant
> somewhere for the 256MB and use it both here and in
> ResourceConstraints::ConfigureDefaultS? That way, if we decide to change the
> heap size of a low-mem device it will change both places. 

Done.

[rmcilroy, 2014-04-16 08:28:15]

One suggestion, but otherwise lgtm.  Thanks!

https://codereview.chromium.org/236063015/diff/70001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/236063015/diff/70001/src/api.cc#newcode473
src/api.cc:473: const int lump_of_memory = (i::kPointerSize / 4) * i::MB;
nit - if you move lump_of_memory, to heap.h too, then the sizes could be simply
in MB (and the growth factor check would match the heap size even on 64bit
device).

[Hannes Payer (out of office), 2014-04-17 10:30:04]

https://codereview.chromium.org/236063015/diff/70001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/236063015/diff/70001/src/api.cc#newcode473
src/api.cc:473: const int lump_of_memory = (i::kPointerSize / 4) * i::MB;
On 2014/04/16 08:28:15, rmcilroy wrote:
> nit - if you move lump_of_memory, to heap.h too, then the sizes could be
simply
> in MB (and the growth factor check would match the heap size even on 64bit
> device).

Done.

[Hannes Payer (out of office), 2014-04-17 11:27:54]

Committed patchset #6 manually as r20837 (presubmit successful).