Use page header information to test for InNewSpace.

Use page header information to test for InNewSpace.

Use page lookup to test for InNewSpace-ness in assembler macro (IA32 only).
Add function InNewSpacePage to test any address for being in 
new space. Use where it's known that the address is in a paged
space.
Retains the old InNewSpace for the cases where the address might
not be in a paged space.
Change InNewSpace(Object*) to use page header.

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

[Erik Corry, 2011-05-19 07:22:31]

LGTM

http://codereview.chromium.org/7029030/diff/1/src/heap-inl.h
File src/heap-inl.h (right):

http://codereview.chromium.org/7029030/diff/1/src/heap-inl.h#newcode257
src/heap-inl.h:257: bool result = new_space_.PageContains(object);
Not mad keen on this name, but I guess it will go away when the transition to
page-based new-space-contains is done.

http://codereview.chromium.org/7029030/diff/1/src/ia32/macro-assembler-ia32-i...
File src/ia32/macro-assembler-ia32-inl.h (right):

http://codereview.chromium.org/7029030/diff/1/src/ia32/macro-assembler-ia32-i...
src/ia32/macro-assembler-ia32-inl.h:37: void MacroAssembler::InNewSpace(Register
object,
Seems like you can just use CheckPageFlag instead of this having an
implementation.

http://codereview.chromium.org/7029030/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/7029030/diff/1/src/objects.h#newcode634
src/objects.h:634: class Failure;
It would perhaps be neater to group this with the other class forward
declarations above.

http://codereview.chromium.org/7029030/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7029030/diff/1/src/runtime.cc#newcode595
src/runtime.cc:595: if (value->IsFailure()) { OS::DebugBreak(); }
This looks like an accidental edit.

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

http://codereview.chromium.org/7029030/diff/1/src/spaces.h#newcode1698
src/spaces.h:1698: // page, it must be a pointer into the beginning of it.
Comment should be moved down to the function to which it applies.

http://codereview.chromium.org/7029030/diff/1/src/spaces.h#newcode1708
src/spaces.h:1708: return false;
Can we add a TODO to remove this?

http://codereview.chromium.org/7029030/diff/1/src/spaces.h#newcode1719
src/spaces.h:1719: if ((reinterpret_cast<uintptr_t>(o) & ~kHeapObjectTagMask) ==
And this

http://codereview.chromium.org/7029030/diff/1/src/store-buffer.cc
File src/store-buffer.cc (right):

http://codereview.chromium.org/7029030/diff/1/src/store-buffer.cc#newcode420
src/store-buffer.cc:420: (owner == page->heap()->map_space() ?
page->heap() could just be heap_

http://codereview.chromium.org/7029030/diff/1/src/top.cc
File src/top.cc (right):

http://codereview.chromium.org/7029030/diff/1/src/top.cc#newcode575
src/top.cc:575: if (exception->IsFailure()) return
exception->ToFailureUnchecked();
Does this need to be ported to bleeding edge and branches?

[Lasse Reichstein, 2011-05-19 08:40:40]

http://codereview.chromium.org/7029030/diff/1/src/heap-inl.h
File src/heap-inl.h (right):

http://codereview.chromium.org/7029030/diff/1/src/heap-inl.h#newcode257
src/heap-inl.h:257: bool result = new_space_.PageContains(object);
That is the idea. 
If anything, this name is a little too nice. It should stand out like a sore
thumb, kindof like C++ casts.

http://codereview.chromium.org/7029030/diff/1/src/ia32/macro-assembler-ia32-i...
File src/ia32/macro-assembler-ia32-inl.h (right):

http://codereview.chromium.org/7029030/diff/1/src/ia32/macro-assembler-ia32-i...
src/ia32/macro-assembler-ia32-inl.h:37: void MacroAssembler::InNewSpace(Register
object,
Merged this with CheckPageFlag (accept object and scratch being the same
register, use byte test).

http://codereview.chromium.org/7029030/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/7029030/diff/1/src/objects.h#newcode634
src/objects.h:634: class Failure;
Done.

http://codereview.chromium.org/7029030/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7029030/diff/1/src/runtime.cc#newcode595
src/runtime.cc:595: if (value->IsFailure()) { OS::DebugBreak(); }
Ack, yes. Shouldn't happen any more.
I'll keep the assert (it's on bleeding edge too).

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

http://codereview.chromium.org/7029030/diff/1/src/spaces.h#newcode1698
src/spaces.h:1698: // page, it must be a pointer into the beginning of it.
Moved.

http://codereview.chromium.org/7029030/diff/1/src/spaces.h#newcode1708
src/spaces.h:1708: return false;
Done.

http://codereview.chromium.org/7029030/diff/1/src/spaces.h#newcode1719
src/spaces.h:1719: if ((reinterpret_cast<uintptr_t>(o) & ~kHeapObjectTagMask) ==
And done.

http://codereview.chromium.org/7029030/diff/1/src/store-buffer.cc
File src/store-buffer.cc (right):

http://codereview.chromium.org/7029030/diff/1/src/store-buffer.cc#newcode420
src/store-buffer.cc:420: (owner == page->heap()->map_space() ?
On 2011/05/19 07:22:32, Erik Corry wrote:
> page->heap() could just be heap_

Done.

http://codereview.chromium.org/7029030/diff/1/src/top.cc
File src/top.cc (right):

http://codereview.chromium.org/7029030/diff/1/src/top.cc#newcode575
src/top.cc:575: if (exception->IsFailure()) return
exception->ToFailureUnchecked();
Already ported to bleeding edge (or actually, from bleeding_edge).
Will check branches.