Moving prototype transitions into the header of the transition array.

Moving prototype transitions into the header of the transition array.

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

[Toon Verwaest, 2012-06-28 15:07:40]

PTAL.

[Michael Starzinger, 2012-07-06 13:53:18]

First round. Mainly nits, and one real comment in objects-inl.h

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

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/mark-compact.cc...
src/mark-compact.cc:1840: // to it. But make sure to skip back pointer and
prototype transitions.
Comment no longer applies, drop the "and prototype transitions".

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

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects-inl.h#n...
src/objects-inl.h:3554: FixedArray* Map::prototype_transitions() {
Since these methods are far more than generic accessors we should use camel-case
naming for them.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects-inl.h#n...
src/objects-inl.h:3563: MaybeObject* Map::set_prototype_transitions(FixedArray*
proto_transitions) {
Likewise.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects-inl.h#n...
src/objects-inl.h:3568: prototype_transitions() == proto_transitions) {
I think this should actually be "!=" here. Maybe we should turn that into an
assertion if it can actually never be "==" here.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects-inl.h#n...
src/objects-inl.h:3626: CONDITIONAL_WRITE_BARRIER(
This should fit into one line now.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.cc#newc...
src/objects.cc:5138: *Header() = Smi::FromInt(0);
Oh yeah, I like that change!

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.h#newco...
src/objects.h:4810: inline void init_back_pointer(Object* undefined);
Lets make that a camel-case method (i.e. InitializeBackPointer), since it will
soon become more complex. Also drop the empty newline above.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.h#newco...
src/objects.h:5002: static const int kBackPointerOffset =
This should fit in one line now?

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.h#newco...
src/objects.h:5004: static const int kPadStart =
Likewise.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/transitions.h
File src/transitions.h (right):

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/transitions.h#n...
src/transitions.h:71: inline Object** GetPrototypeTransitionsSlot();
Can we group these accessors somehow, this is becoming a rather large block.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/transitions.h#n...
src/transitions.h:115: static const int kPrototypeTransitionsOffset =
kElementsTransitionOffset +
Better break that after the "=", to be consistent with objects.h

[Toon Verwaest, 2012-07-09 12:45:27]

Addressed comments, PTAL.

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

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/mark-compact.cc...
src/mark-compact.cc:1840: // to it. But make sure to skip back pointer and
prototype transitions.
On 2012/07/06 13:53:18, Michael Starzinger wrote:
> Comment no longer applies, drop the "and prototype transitions".

Done.

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

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects-inl.h#n...
src/objects-inl.h:3563: MaybeObject* Map::set_prototype_transitions(FixedArray*
proto_transitions) {
On 2012/07/06 13:53:18, Michael Starzinger wrote:
> Likewise.

Done.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects-inl.h#n...
src/objects-inl.h:3568: prototype_transitions() == proto_transitions) {
On 2012/07/06 13:53:18, Michael Starzinger wrote:
> I think this should actually be "!=" here. Maybe we should turn that into an
> assertion if it can actually never be "==" here.

Done.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects-inl.h#n...
src/objects-inl.h:3626: CONDITIONAL_WRITE_BARRIER(
On 2012/07/06 13:53:18, Michael Starzinger wrote:
> This should fit into one line now.

Done.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.h#newco...
src/objects.h:4810: inline void init_back_pointer(Object* undefined);
As discussed offline, will leave as is. Not complex since only used while
bootstrapping.

On 2012/07/06 13:53:18, Michael Starzinger wrote:
> Lets make that a camel-case method (i.e. InitializeBackPointer), since it will
> soon become more complex. Also drop the empty newline above.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.h#newco...
src/objects.h:5002: static const int kBackPointerOffset =
On 2012/07/06 13:53:18, Michael Starzinger wrote:
> This should fit in one line now?

Done.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/objects.h#newco...
src/objects.h:5004: static const int kPadStart =
On 2012/07/06 13:53:18, Michael Starzinger wrote:
> Likewise.

Done.

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/transitions.h
File src/transitions.h (right):

https://chromiumcodereview.appspot.com/10692026/diff/9011/src/transitions.h#n...
src/transitions.h:71: inline Object** GetPrototypeTransitionsSlot();
On 2012/07/06 13:53:18, Michael Starzinger wrote:
> Can we group these accessors somehow, this is becoming a rather large block.

Done.

[Michael Starzinger, 2012-07-09 16:40:55]

LGTM (with one final nit).

https://chromiumcodereview.appspot.com/10692026/diff/12001/src/transitions.h
File src/transitions.h (right):

https://chromiumcodereview.appspot.com/10692026/diff/12001/src/transitions.h#...
src/transitions.h:65: inline Object** GetElementsSlot();
Let's name that GetElementsTransitionSlot() instead.