Fix new space shrinking to compute correct capacity.

Fix new space shrinking to compute correct capacity.

R=vegorov@chromium.org
BUG=v8:1702
TEST=cctest/test-heap/GrowAndShrinkNewSpace

Committed: http://code.google.com/p/v8/source/detail?r=9348

[Michael Starzinger, 2011-09-20 14:12:54]

PTAL.

[Vyacheslav Egorov (Chromium), 2011-09-20 14:26:40]

LGTM

http://codereview.chromium.org/7983001/diff/1/src/spaces.cc
File src/spaces.cc (right):

http://codereview.chromium.org/7983001/diff/1/src/spaces.cc#newcode929
src/spaces.cc:929: ASSERT(static_cast<size_t>(Page::kPageSize) >
OS::AllocateAlignment());
I don't think we really need this assert here. There are checks in routines that
deal with virtual memory.

http://codereview.chromium.org/7983001/diff/1/src/spaces.cc#newcode930
src/spaces.cc:930: int new_capacity = Min(MaximumCapacity(), 2 *
static_cast<int>(Capacity()));
We should ensure that MaxCapacity & Capacity is _always_ kPageSize aligned. 

That would clean the code (e.g. Rounding would not be needed here).

And I really would expect that capacities are always aligned as we are managing
new space with kPageSize granularity.

http://codereview.chromium.org/7983001/diff/1/src/spaces.cc#newcode949
src/spaces.cc:949: ASSERT(static_cast<size_t>(Page::kPageSize) >
OS::AllocateAlignment());
I don't think we really need this assert here. There are checks in routines that
deal with virtual memory.

[Michael Starzinger, 2011-09-20 15:29:03]

Added new patch set. Also fixed assertion as discussed offline with Vyacheslav
Egorov.

http://codereview.chromium.org/7983001/diff/1/src/spaces.cc
File src/spaces.cc (right):

http://codereview.chromium.org/7983001/diff/1/src/spaces.cc#newcode929
src/spaces.cc:929: ASSERT(static_cast<size_t>(Page::kPageSize) >
OS::AllocateAlignment());
On 2011/09/20 14:26:40, Vyacheslav Egorov wrote:
> I don't think we really need this assert here. There are checks in routines
that
> deal with virtual memory.

Done.

http://codereview.chromium.org/7983001/diff/1/src/spaces.cc#newcode930
src/spaces.cc:930: int new_capacity = Min(MaximumCapacity(), 2 *
static_cast<int>(Capacity()));
On 2011/09/20 14:26:40, Vyacheslav Egorov wrote:
> We should ensure that MaxCapacity & Capacity is _always_ kPageSize aligned. 
> 
> That would clean the code (e.g. Rounding would not be needed here).
> 
> And I really would expect that capacities are always aligned as we are
managing
> new space with kPageSize granularity.

Done. I just checked that it is only modified in a way that keeps it aligned to
kPageSize. The assertion which checks that is exactly the one being triggered
(in SemiSpace::ShrinkTo and SemiSpace::GrowTo).

http://codereview.chromium.org/7983001/diff/1/src/spaces.cc#newcode949
src/spaces.cc:949: ASSERT(static_cast<size_t>(Page::kPageSize) >
OS::AllocateAlignment());
On 2011/09/20 14:26:40, Vyacheslav Egorov wrote:
> I don't think we really need this assert here. There are checks in routines
that
> deal with virtual memory. 

Done.

[Vyacheslav Egorov (Chromium), 2011-09-20 15:32:24]

LGTM