Swapped transition array and descriptor array.

Swapped transition array and descriptor array.

Now a map points to a transition array which contains the descriptor array. The descriptor array is now immutable. The next step is to share the descriptor array with all back-pointed maps as long as there is a single line of extension. Maps that require a descriptor array but don't need transitions will still need a pseudo-empty transition array to contain the descriptor array.

Committed: https://code.google.com/p/v8/source/detail?r=12298

[Toon Verwaest, 2012-07-20 15:10:08]

PTAL.

Setting of descriptor array over map now has to ensure there is a transition
array, just like for prototype transitions and elements transitions.

On the other hand, setting transition arrays is now immediate via the map, and
does not need to ensure the availability of a descriptor array.

[Michael Starzinger, 2012-08-06 14:15:52]

One real comment.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assem...
File src/arm/macro-assembler-arm.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assem...
src/arm/macro-assembler-arm.cc:3704: Register empty_descriptor_array_value = r7;
Drop the "empty_descriptor_array_value".

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assem...
src/arm/macro-assembler-arm.cc:3726: LoadRoot(empty_descriptor_array_value,
Better use r7 directly here, so that it's clear that CheckMap above will destroy
it.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/factory.cc
File src/factory.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/factory.cc#newc...
src/factory.cc:123: void Factory::SetDescriptors(Handle<Map> map,
This method should definitely be moved into the "Map" class and be made static.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/mark-compact.cc
File src/mark-compact.cc (left):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/mark-compact.cc...
src/mark-compact.cc:1884: void Marker<T>::MarkDescriptorArray(DescriptorArray*
descriptors) {
Oh yeah! I like that!

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/objects.cc#newc...
src/objects.cc:5253: if (!(*HeapObject::RawField(transition_array,
The HeapObject::map() method should still work here. Also can we turn this into
one single condition ...

if (!transition_array->map()->IsSmi() &&
    !transition_array->IsTransitionArray()) {
  return NULL;
}

[Toon Verwaest, 2012-08-07 08:33:04]

Addressed comments, PTAL.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assem...
File src/arm/macro-assembler-arm.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assem...
src/arm/macro-assembler-arm.cc:3704: Register empty_descriptor_array_value = r7;
On 2012/08/06 14:15:52, Michael Starzinger wrote:
> Drop the "empty_descriptor_array_value".

Done.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assem...
src/arm/macro-assembler-arm.cc:3726: LoadRoot(empty_descriptor_array_value,
On 2012/08/06 14:15:52, Michael Starzinger wrote:
> Better use r7 directly here, so that it's clear that CheckMap above will
destroy
> it.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/factory.cc
File src/factory.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/factory.cc#newc...
src/factory.cc:123: void Factory::SetDescriptors(Handle<Map> map,
On 2012/08/06 14:15:52, Michael Starzinger wrote:
> This method should definitely be moved into the "Map" class and be made
static.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/objects.cc#newc...
src/objects.cc:5253: if (!(*HeapObject::RawField(transition_array,
On 2012/08/06 14:15:52, Michael Starzinger wrote:
> The HeapObject::map() method should still work here. Also can we turn this
into
> one single condition ...
> 
> if (!transition_array->map()->IsSmi() &&
>     !transition_array->IsTransitionArray()) {
>   return NULL;
> }

Done.

[Michael Starzinger, 2012-08-07 13:55:04]

Second round.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/bootstrapper.c...
src/bootstrapper.cc:402: 
Drop the empty newline.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/bootstrapper.c...
src/bootstrapper.cc:544: 
Drop the empty newline.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1853: // array as weak and also mark that array to prevent
visiting it later.
These comments are completely outdated.

// Make sure that the back pointer stored either in the map itself or inside
// its transitions array is marked. Treat pointers in the transitions array
// as weak and also mark that array to prevent visiting it later.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1857: HeapObject::RawField(map,
Map::kPointerFieldsBeginOffset);
Move this down to the main marking loop again.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1863: mark_compact_collector()->RecordSlot(
This slot is actually recorded in the main marking loop below. We can drop it.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1873: // to it. But make sure to skip back pointer.
These comments are completely outdated.

// Mark the Object* fields of the Map. Since the transitions array has been
// marked already, it is fine that one of these fields contains a pointer
// to it.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1875: Map::kBitField3Offset + kPointerSize);
This assert is no longer necessary, because we no longer shared bit_field3 and
the backpointer.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1876: Object** end_slot = HeapObject::RawField(map,
Map::kBitField3Offset);
We can use Map::kPointerFieldsEndOffset here.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/objects.h#newc...
src/objects.h:5039: // Storage for instance descriptors is overloaded to also
contain additional
These comments are completely outdated.

https://chromiumcodereview.appspot.com/10816005/diff/22001/test/cctest/test-a...
File test/cctest/test-alloc.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/22001/test/cctest/test-a...
test/cctest/test-alloc.cc:161: v8::internal::Map::SetDescriptors(map,
new_descriptors);
We can drop the "v8::internal::" prefix for both of these lines, that's already
the current namespace.

[Toon Verwaest, 2012-08-10 12:32:52]

PTAL.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/bootstrapper.c...
src/bootstrapper.cc:544: 
On 2012/08/07 13:55:04, Michael Starzinger wrote:
> Drop the empty newline.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1853: // array as weak and also mark that array to prevent
visiting it later.
On 2012/08/07 13:55:04, Michael Starzinger wrote:
> These comments are completely outdated.
> 
> // Make sure that the back pointer stored either in the map itself or inside
> // its transitions array is marked. Treat pointers in the transitions array
> // as weak and also mark that array to prevent visiting it later.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1857: HeapObject::RawField(map,
Map::kPointerFieldsBeginOffset);
On 2012/08/07 13:55:04, Michael Starzinger wrote:
> Move this down to the main marking loop again.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1863: mark_compact_collector()->RecordSlot(
On 2012/08/07 13:55:05, Michael Starzinger wrote:
> This slot is actually recorded in the main marking loop below. We can drop it.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1873: // to it. But make sure to skip back pointer.
On 2012/08/07 13:55:05, Michael Starzinger wrote:
> These comments are completely outdated.
> 
> // Mark the Object* fields of the Map. Since the transitions array has been
> // marked already, it is fine that one of these fields contains a pointer
> // to it.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1875: Map::kBitField3Offset + kPointerSize);
On 2012/08/07 13:55:05, Michael Starzinger wrote:
> This assert is no longer necessary, because we no longer shared bit_field3 and
> the backpointer.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/mark-compact.c...
src/mark-compact.cc:1876: Object** end_slot = HeapObject::RawField(map,
Map::kBitField3Offset);
On 2012/08/07 13:55:05, Michael Starzinger wrote:
> We can use Map::kPointerFieldsEndOffset here.

Done.

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10816005/diff/22001/src/objects.h#newc...
src/objects.h:5039: // Storage for instance descriptors is overloaded to also
contain additional
On 2012/08/07 13:55:05, Michael Starzinger wrote:
> These comments are completely outdated.

Done.

[Michael Starzinger, 2012-08-12 18:18:07]

LGTM (with one nit).

https://chromiumcodereview.appspot.com/10816005/diff/12023/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/12023/src/bootstrapper.c...
src/bootstrapper.cc:543: 
Drop the empty newline.