Remaining changes to fully support FastDoubleArray.

Remaining changes to fully support FastDoubleArray.

R=ager@chromium.org
BUG=none
TEST=cctests, unboxed-double-array.js

Committed: http://code.google.com/p/v8/source/detail?r=8718

[danno, 2011-07-22 06:57:43]

Please review.

[Mads Ager (chromium), 2011-07-22 08:29:19]

LGTM

We should deal with the double keys differently, but feel free to do that in a
separate changelist.

http://codereview.chromium.org/7473031/diff/3006/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7473031/diff/3006/src/objects.cc#newcode4810
src/objects.cc:4810: MaybeObject* maybe_obj =
heap->NumberFromDouble(other->get(y));
This is nasty. This can lead to out-of-memory crashes when we are in fact not
out of memory if the double array is large enough. That is probably very
unlikely to happen, but still. I wonder if we can restrict the API so that we
can never get doubles in key arrays (require integers and strings only)?
Alternatively, we should be able to create the key integer/string directly from
the double without going through a heap number allocation.

Feel free to deal with this in a separate changelist.

http://codereview.chromium.org/7473031/diff/3006/src/objects.cc#newcode4823
src/objects.cc:4823: AssertNoAllocation no_gc;
Maybe we should change the name to AssertNoGC at some point. There will be lots
of allocation below. Maybe limit the scope of the AssertNoAllocation? This is to
make sure that the write barrier mode stays valid, right?

http://codereview.chromium.org/7473031/diff/3006/src/objects.cc#newcode4835
src/objects.cc:4835: MaybeObject* maybe_obj =
heap->NumberFromDouble(other->get(y));
Here again. So we actually allocate two new heap numbers per entry. We should
create the integer/string key directly from the double.

http://codereview.chromium.org/7473031/diff/3006/test/mjsunit/unbox-double-ar...
File test/mjsunit/unbox-double-arrays.js (right):

http://codereview.chromium.org/7473031/diff/3006/test/mjsunit/unbox-double-ar...
test/mjsunit/unbox-double-arrays.js:473: for (x in names) {
property_name_count++};
++} -> ++; }

[danno, 2011-07-22 09:10:12]

Feedback addressed, landing.

http://codereview.chromium.org/7473031/diff/3006/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7473031/diff/3006/src/objects.cc#newcode4810
src/objects.cc:4810: MaybeObject* maybe_obj =
heap->NumberFromDouble(other->get(y));
Submitted as 1567.
On 2011/07/22 08:29:20, Mads Ager wrote:
> This is nasty. This can lead to out-of-memory crashes when we are in fact not
> out of memory if the double array is large enough. That is probably very
> unlikely to happen, but still. I wonder if we can restrict the API so that we
> can never get doubles in key arrays (require integers and strings only)?
> Alternatively, we should be able to create the key integer/string directly
from
> the double without going through a heap number allocation.
> 
> Feel free to deal with this in a separate changelist.

http://codereview.chromium.org/7473031/diff/3006/src/objects.cc#newcode4823
src/objects.cc:4823: AssertNoAllocation no_gc;
On 2011/07/22 08:29:20, Mads Ager wrote:
> Maybe we should change the name to AssertNoGC at some point. There will be
lots
> of allocation below. Maybe limit the scope of the AssertNoAllocation? This is
to
> make sure that the write barrier mode stays valid, right?

Done.

http://codereview.chromium.org/7473031/diff/3006/src/objects.cc#newcode4835
src/objects.cc:4835: MaybeObject* maybe_obj =
heap->NumberFromDouble(other->get(y));
On 2011/07/22 08:29:20, Mads Ager wrote:
> Here again. So we actually allocate two new heap numbers per entry. We should
> create the integer/string key directly from the double.
See above.