Addressed comments. Now porting to x64 and arm.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/handles.cc
File src/handles.cc (right):
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/handles.cc#newcode794
src/handles.cc:794: if (cache_result)
object->map()->SetEnumLength(enum_size);
On 2012/09/11 14:24:25, Jakob wrote:
I don't feel strongly about it, but for the record: I don't think we
should
convert if-statements to their one-line form.
Done.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/heap.cc
File src/heap.cc (right):
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/heap.cc#newcode2067
src/heap.cc:2067: int bit_field3 = 0;
On 2012/09/11 14:37:09, Michael Starzinger wrote:
Can we use the following style here as well?
int bit_field3 = Map::EnumLengthBits::encode(Map::kInvalidEnumCache) |
Map::OwnsDescriptors::encode(true);
Done.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/mark-compact.cc
File src/mark-compact.cc (right):
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/mark-compact.cc#newcode1943
src/mark-compact.cc:1943: // We don't have to record the
descriptors_pointer slot since the cellspace is
On 2012/09/11 14:37:09, Michael Starzinger wrote:
Use s/cellspace/cell space/ in the comment, so we can grep for it.
Done.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/objects-inl.h
File src/objects-inl.h (right):
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/objects-inl.h#newcode1968
src/objects-inl.h:1968: // name->PrintLn();
On 2012/09/11 14:24:25, Jakob wrote:
nit: leftover?
Done.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/objects-inl.h#newcode3032
src/objects-inl.h:3032: void Map::SetOwnsDescriptors(bool is_shared) {
On 2012/09/11 14:24:25, Jakob wrote:
nit: for consistency, these two methods should be in
unix_hacker_style.
Done.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/objects.cc
File src/objects.cc (right):
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/objects.cc#newcode5116
src/objects.cc:5116: // name->PrintLn();
On 2012/09/11 14:24:25, Jakob wrote:
leftover?
Done.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/objects.cc#newcode5169
src/objects.cc:5169: // if (enumeration_index ==
number_of_descriptors()) return this;
This is leftover code. Removed.
It would have been nice to actually share descriptor arrays if no
descriptors were added yet. This would introduce sharing without
transferring ownership, which is not possible given that descriptor
arrays aren't fully immutable: they potentially get reduced in
ClearNonLiveTransitions.
This is problematic if the new map is not linked in the transitions of
the old map. They might have been removed when we changed the nature of
a transition from accessor to property, or not have been inserted in the
first place.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/objects.cc#newcode5220
src/objects.cc:5220: // Resort if descriptors were removed.
On 2012/09/11 14:24:25, Jakob wrote:
nit: s/Resort/Re-sort/
Done.
https://chromiumcodereview.appspot.com/10909007/diff/18001/src/objects.cc#newcode6025
src/objects.cc:6025: ASSERT(!IsEmpty()); // Do nothing for empty
descriptor array.
On 2012/09/11 14:24:25, Jakob wrote:
outdated comment (just remove it)
Done.
https://chromiumcodereview.appspot.com/10909007/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
