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-inl.h File src/ia32/macro-assembler-ia32-inl.h (right): http://codereview.chromium.org/7029030/diff/1/src/ia32/macro-assembler-ia32-inl.h#newcode37 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? http://codereview.chromium.org/7029030/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
