First round.
https://chromiumcodereview.appspot.com/10575032/diff/1/src/ic.cc File src/ic.cc (left): https://chromiumcodereview.appspot.com/10575032/diff/1/src/ic.cc#oldcode1443 src/ic.cc:1443: ASSERT(lookup->type() != NULL_DESCRIPTOR); We should actually keep this and assert against NONEXISTENT now. https://chromiumcodereview.appspot.com/10575032/diff/1/src/ic.cc#oldcode1945 src/ic.cc:1945: ASSERT(lookup->type() != NULL_DESCRIPTOR); Likewise. https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects-inl.h File src/objects-inl.h (right): https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects-inl.h#newcode3487 src/objects-inl.h:3487: void Map::SetOwnBitField3(int value) { That method name is mighty confusing! Is should actually be named ClearDescriptorArray I guess. https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc File src/objects.cc (right): https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc#newcode7346 src/objects.cc:7346: static void RightTrimFixedArray(Heap* heap, FixedArray* elms, int to_trim) { Add a comment that this function is only called from within the GC. Otherwise the live-bytes counter will be inconsistent. https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc#newcode7355 src/objects.cc:7355: STATIC_ASSERT(FixedArray::kHeaderSize == 2 * kPointerSize); These asserts are not needed for left-trimming! https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc#newcode7357 src/objects.cc:7357: const int len = elms->length(); Add an ASSERT(to_trim < len), other cases aren't handled correctly here. https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc#newcode7359 src/objects.cc:7359: Address new_end = elms->address() + FixedArray::kHeaderSize + Better use "elms->address() + FixedArray::SizeFor(len - to_trim)" here. https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc#newcode7361 src/objects.cc:7361: if (to_trim > FixedArray::kHeaderSize / kPointerSize && This condition doesn't make much sense to me. And as discussed offline, the zapping is probably only needed for debug mode with the new GC. https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc#newcode7364 src/objects.cc:7364: // formerly part of the array so that the GC (aided by the card-based Comment no longer applies. https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc#newcode7389 src/objects.cc:7389: static bool ClearDescriptor(Heap* heap, We should give this function the "ClearNonLive" prefix as well. How about "ClearNonLiveTransitionsFromDescriptor"? Also a comment describing the semantics of the return value would nice. https://chromiumcodereview.appspot.com/10575032/diff/1/src/objects.cc#newcode7454 src/objects.cc:7454: while (descriptor_index < d->number_of_descriptors() && Is there a strong reason why this loop is split into two parts and interleaved with the clearing of elements transitions? Is seems overly complicated. Can we merge these two loops into one? https://chromiumcodereview.appspot.com/10575032/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
