LGTM

It looks like you rely on the fact that pages are sorted in increasing address order. If you don't you will when you get the water mark to work right. But I
can't see you assert it anywhere.


http://codereview.chromium.org/7124006/diff/1/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/7124006/diff/1/src/heap.cc#newcode4316
src/heap.cc:4316: void Heap::ZapFromSpace() {
I wonder why this isn't called NukeFromOrbit?

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

http://codereview.chromium.org/7124006/diff/1/src/spaces.cc#newcode1020
src/spaces.cc:1020: return false;
Why did AddFreshPage get called if the current page is empty?

http://codereview.chromium.org/7124006/diff/1/src/spaces.cc#newcode1023
src/spaces.cc:1023: // Failed to use a new page in to-space.
Failed to get?

http://codereview.chromium.org/7124006/diff/1/src/spaces.cc#newcode1026
src/spaces.cc:1026: // Clear remainder of current pag.
pag -> page

http://codereview.chromium.org/7124006/diff/1/src/spaces.cc#newcode1079
src/spaces.cc:1079: limit = (page == end_page) ? top() :
page->body_limit();
This ternary operator together with the if above confuse me...

http://codereview.chromium.org/7124006/diff/1/src/spaces.h
File src/spaces.h (right):

http://codereview.chromium.org/7124006/diff/1/src/spaces.h#newcode1637
src/spaces.h:1637: // The offset of an address from the beginning of the
space.
We can't delete this function?

http://codereview.chromium.org/7124006/diff/1/src/spaces.h#newcode1964
src/spaces.h:1964: Address ToSpaceLow() { return to_space_.space_low();
}
This does not really make sense, at least for the purpose that the
comment claims it has.

http://codereview.chromium.org/7124006/diff/1/src/spaces.h#newcode1999
src/spaces.h:1999:
Comment on what the return value means.

http://codereview.chromium.org/7124006/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to