This is looking good. Once Slava's comments are addressed I think you can
land
this under the flag.
My main comment is that I think there is stuff missing for double array
elements
at this point. Off the top of my head I can think of AddKeysFromJSArray,
HasElementPostInterceptor, EstimateElementsCount... I think we need to go
through all the places where FAST_ELEMENTS are used, add a test and make it
pass.
Since this is under a flag, you can do this in a separate change list to
keep
this one small.
http://codereview.chromium.org/7089002/diff/17001/src/factory.h
File src/factory.h (right):
http://codereview.chromium.org/7089002/diff/17001/src/factory.h#newcode52
src/factory.h:52: // Allocate a new fixed double array with undefined
entries.
Nit: Could you update the comment to use the wording: uninitialized
fixed double array? And do the same for NewFixedArray above? I get
confused by the use of undefined here (not sure if it means the
'undefined' value).
http://codereview.chromium.org/7089002/diff/17001/src/objects-inl.h
File src/objects-inl.h (right):
http://codereview.chromium.org/7089002/diff/17001/src/objects-inl.h#newcode1627
src/objects-inl.h:1627: ASSERT(map() != HEAP->fixed_cow_array_map());
If you assert for fixed_cow_array_map should you assert for normal
fixed_array_map too?
http://codereview.chromium.org/7089002/diff/17001/src/objects-inl.h#newcode1635
src/objects-inl.h:1635: ASSERT(map() != HEAP->fixed_cow_array_map());
Ditto?
http://codereview.chromium.org/7089002/diff/17001/src/objects.cc
File src/objects.cc (right):
http://codereview.chromium.org/7089002/diff/17001/src/objects.cc#newcode8051
src/objects.cc:8051: // Check whether there is extra space in the fixed
array..
.. -> .
http://codereview.chromium.org/7089002/diff/17001/src/objects.cc#newcode8452
src/objects.cc:8452: return GetHeap()->AllocateHeapNumber(double_value);
Use NumberFromDouble?
http://codereview.chromium.org/7089002/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
