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#newcode1878
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#newcode1930
src/objects-inl.h:1930:
Two lines between top-level declarations.

https://chromiumcodereview.appspot.com/10417030/diff/1/src/objects-inl.h#newcode1935
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#newcode5843
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.

https://chromiumcodereview.appspot.com/10417030/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to