This looks great! A few more comments for you.
http://codereview.chromium.org/2101002/diff/2001/3009 File src/heap.cc (right): http://codereview.chromium.org/2101002/diff/2001/3009#newcode3572 src/heap.cc:3572: Address end = Remove extra spacing. http://codereview.chromium.org/2101002/diff/2001/3011 File src/ia32/builtins-ia32.cc (right): http://codereview.chromium.org/2101002/diff/2001/3011#newcode227 src/ia32/builtins-ia32.cc:227: __ mov(Operand(edi, HeapObject::kMapOffset), eax); // setup the map [Edit: I see now that the Array super class has been removed. Why?] This will work just fine, but it seems a bit strange to change this to the super class HeapObject when you know that it is a JSObject and then below change from super class Array to sub class FixedArray. Let's either use the most specific type or use the type in which the offsets are defined? http://codereview.chromium.org/2101002/diff/2001/3011#newcode760 src/ia32/builtins-ia32.cc:760: __ mov(FieldOperand(scratch1, HeapObject::kMapOffset), Ditto. http://codereview.chromium.org/2101002/diff/2001/3011#newcode854 src/ia32/builtins-ia32.cc:854: __ mov(FieldOperand(elements_array, HeapObject::kMapOffset), Ditto. http://codereview.chromium.org/2101002/diff/2001/3015 File src/ia32/macro-assembler-ia32.cc (right): http://codereview.chromium.org/2101002/diff/2001/3015#newcode96 src/ia32/macro-assembler-ia32.cc:96: // Set the remembered set bit for [object+offset]. We no longer use remembered sets so this comment should be revised. We should make sure to search for |remembered set| in the code base and update all comments. http://codereview.chromium.org/2101002/diff/2001/3018 File src/mark-compact.cc (right): http://codereview.chromium.org/2101002/diff/2001/3018#newcode1439 src/mark-compact.cc:1439: // Address end = start + size_in_bytes; Code in comments. http://codereview.chromium.org/2101002/diff/2001/3018#newcode1491 src/mark-compact.cc:1491: // Page::ClearRegionMarks(start, Code in comments. http://codereview.chromium.org/2101002/diff/2001/3018#newcode2056 src/mark-compact.cc:2056: // Find end of allocation of in the page of first_forwarded. allocation of in the -> allocation in the http://codereview.chromium.org/2101002/diff/2001/3021 File src/objects-inl.h (right): http://codereview.chromium.org/2101002/diff/2001/3021#newcode763 src/objects-inl.h:763: IsRegionDirty(object->address() + offset)); \ This line should be indented more. http://codereview.chromium.org/2101002/diff/2001/3023 File src/objects.h (left): http://codereview.chromium.org/2101002/diff/2001/3023#oldcode58 src/objects.h:58: // - Array Why remove the Array super class here? All the arrays need lengths and they all need to be smi-tagged, right? http://codereview.chromium.org/2101002/diff/2001/3026 File src/spaces.cc (right): http://codereview.chromium.org/2101002/diff/2001/3026#newcode2696 src/spaces.cc:2696: uint32_t marks = page->GetRegionMarks(); Remove extra indentation here and in the rest of this method. http://codereview.chromium.org/2101002/diff/2001/3027 File src/spaces.h (right): http://codereview.chromium.org/2101002/diff/2001/3027#newcode242 src/spaces.h:242: static const int kDirtyFlagOffset = 2*kPointerSize; Space around binary op. http://codereview.chromium.org/2101002/diff/2001/3027#newcode244 src/spaces.h:244: static const int kRegionSize = 1 << kRegionSizeLog2; I would remove the extra spacing here. http://codereview.chromium.org/2101002/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
