Decoupling MarkDescriptorArray as much as possible from the ContentArray.

Decoupling MarkDescriptorArray as much as possible from the ContentArray.

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

[Michael Starzinger, 2012-05-24 14:28:54]

I like comments.

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

https://chromiumcodereview.appspot.com/10417030/diff/1/src/mark-compact.cc#ne...
src/mark-compact.cc:1878: // Contents contains (value, details) pairs.  If the
descriptor contains a
Since you are decoupling this block of code form the contents array, I would
suggest also removing the first sentences of this comment.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects-inl.h
File src/objects-inl.h (right):

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects-inl.h#newc...
src/objects-inl.h:1930: 
Two lines between top-level declarations.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects-inl.h#newc...
src/objects-inl.h:1935: 
Likewise.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.cc#newcode...
src/objects.cc:5843: // potentially swap a black and a white member. This would
resulting in
Reasoning is off.

[...] were to be black, the shuffling would move a slot that was already
recorded as pointing into an evacuation candidate. This would result in missing
updates upon evacuation.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.h#newcode2493
src/objects.h:2493: // (unmarked), so potentially unique heap objects can be
freely assigned
The reasoning is slightly off, I would replace it with something like the
following:

[...] descriptor array is white, so incremental write barriers can be skipped
because the marking invariant cannot be broken and slots pointing into
evacuation candidates will be discovered when the object is scanned. [...]

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.h#newcode2495
src/objects.h:2495: // stack-allocated right after creating a descriptor array.
By allocating a witness,
More than 80 characters.

[Toon Verwaest, 2012-05-25 12:27:26]

Addressed comments, please take a look.

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

https://chromiumcodereview.appspot.com/10417030/diff/1/src/mark-compact.cc#ne...
src/mark-compact.cc:1878: // Contents contains (value, details) pairs.  If the
descriptor contains a
On 2012/05/24 14:28:54, Michael Starzinger wrote:
> Since you are decoupling this block of code form the contents array, I would
> suggest also removing the first sentences of this comment.

Done.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects-inl.h
File src/objects-inl.h (right):

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects-inl.h#newc...
src/objects-inl.h:1930: 
On 2012/05/24 14:28:54, Michael Starzinger wrote:
> Two lines between top-level declarations.

Done.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects-inl.h#newc...
src/objects-inl.h:1935: 
On 2012/05/24 14:28:54, Michael Starzinger wrote:
> Likewise.

Done.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.cc#newcode...
src/objects.cc:5843: // potentially swap a black and a white member. This would
resulting in
On 2012/05/24 14:28:54, Michael Starzinger wrote:
> Reasoning is off.
> 
> [...] were to be black, the shuffling would move a slot that was already
> recorded as pointing into an evacuation candidate. This would result in
missing
> updates upon evacuation.

Done.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.h#newcode2493
src/objects.h:2493: // (unmarked), so potentially unique heap objects can be
freely assigned
On 2012/05/24 14:28:54, Michael Starzinger wrote:
> The reasoning is slightly off, I would replace it with something like the
> following:
> 
> [...] descriptor array is white, so incremental write barriers can be skipped
> because the marking invariant cannot be broken and slots pointing into
> evacuation candidates will be discovered when the object is scanned. [...]

Done.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects.h#newcode2495
src/objects.h:2495: // stack-allocated right after creating a descriptor array.
By allocating a witness,
On 2012/05/24 14:28:54, Michael Starzinger wrote:
> More than 80 characters.

Done.

[Michael Starzinger, 2012-05-30 15:09:34]

LGTM (with one nit).

https://chromiumcodereview.appspot.com/10417030/diff/7001/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10417030/diff/7001/src/objects.cc#newc...
src/objects.cc:5842: // descriptor array. If the descriptor array were to be
were to be black, the
Remove one "were to be".