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#newcode1840
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#newcode3554
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#newcode3563
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#newcode3568
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#newcode3626
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#newcode5138
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#newcode4810
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#newcode5002
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#newcode5004
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#newcode71
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#newcode115
src/transitions.h:115: static const int kPrototypeTransitionsOffset =
kElementsTransitionOffset +
Better break that after the "=", to be consistent with objects.h

https://chromiumcodereview.appspot.com/10692026/

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

Reply via email to