This is going into the right direction. Most of my comments are just nits.


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

https://chromiumcodereview.appspot.com/10411067/diff/1/src/objects-inl.h#newcode1947
src/objects-inl.h:1947:
Two empty lines between top-level methods.

https://chromiumcodereview.appspot.com/10411067/diff/1/src/objects-inl.h#newcode1955
src/objects-inl.h:1955: Object* details =
RawGetContentArray()->get(ToDetailsIndex(descriptor_number));
More than 80 characters.

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

https://chromiumcodereview.appspot.com/10411067/diff/1/src/objects.cc#newcode4901
src/objects.cc:4901: // Attention, tricky index manipulation ahead: Two
two consecutive indices
Only one "two".

https://chromiumcodereview.appspot.com/10411067/diff/1/src/objects.cc#newcode4907
src/objects.cc:4907: while ((index / 2) <
descriptor_array_->number_of_descriptors()) {
Can we hoist the division by two into a local variable? That would
improve readability. Like this:

int index = Smi::cast(*DescriptorArrayHeader())->value();
int descriptor_number = raw_index / 2;

https://chromiumcodereview.appspot.com/10411067/diff/1/src/objects.cc#newcode4919
src/objects.cc:4919:
static_cast<AccessorPair*>(descriptor_array_->RawGetValue(index / 2));
More than 80 characters.

https://chromiumcodereview.appspot.com/10411067/diff/1/src/objects.cc#newcode5082
src/objects.cc:5082: IntrusiveMapTransitionIterator
descriptor_iterator(MutatedInstanceDescriptors());
Could we just pass the map itself to the constructor of the child
iterator and do the raw access of the instance descriptor field in
there?

More than 80 characters.

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

https://chromiumcodereview.appspot.com/10411067/diff/1/src/objects.h#newcode2479
src/objects.h:2479: inline Object* RawGetValue(int descriptor_number);
These methods are only needed by the IntrusiveMapTransitionIterator, can
we move them into that class (into the private section)? That prevents
others from accidentally using these methods.

Also I would rename them to GetRawValue and GetEawDetails which is more
natural sounding.

https://chromiumcodereview.appspot.com/10411067/diff/1/src/objects.h#newcode2657
src/objects.h:2657: inline FixedArray* RawGetContentArray();
Likewise.

https://chromiumcodereview.appspot.com/10411067/

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

Reply via email to