Fix Heap::Shrink to ensure that it does not free pages that are still in use.

Fix Heap::Shrink to ensure that it does not free pages that are still in use.

Heap::Shrink is called from EnsureFromSpaceIsCommitted at the very start of the GC. At this moment live bytes counts on pages are in inconsistent states. Some pages might have been already swept but have not been yet reached by an incremental marker (or incremental marker is not in progress) and have live bytes count set to 0. Thus we can't rely only on LiveBytes to determine which pages can be released to the OS.


R=mstarzinger@chromium.org
BUG=100414
Committed: http://code.google.com/p/v8/source/detail?r=9953

[Michael Starzinger, 2011-11-10 12:35:40]

LGTM (with nits).

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

http://codereview.chromium.org/8507038/diff/1/src/spaces.cc#newcode758
src/spaces.cc:758: // Adjust list of unswept pages if the page is it's head or
tail.
This block assumes that last_unswept_page_ is either NULL or refers to the
page's anchor. Can we add an assertion checking that?

http://codereview.chromium.org/8507038/diff/1/src/spaces.cc#newcode797
src/spaces.cc:797: if (sizes.Total() == Page::kObjectAreaSize) {
Currently the free-list count at this point should always sum up to
Page::kObjectAreaSize so can we turn this into an assertion instead? This would
save us the counting in release builds.

[Michael Starzinger, 2011-11-10 12:52:45]

LGTM.

http://codereview.chromium.org/8507038/diff/5001/src/spaces.cc
File src/spaces.cc (right):

http://codereview.chromium.org/8507038/diff/5001/src/spaces.cc#newcode801
src/spaces.cc:801: // by free list items.
Wow, that's tricky. Nice catch!