Remove SmartMove from array.js

Remove SmartMove from array.js.

This patch is progress towards having a single, spec-compliant array.js path for all objects.

[Superseded by https://codereview.chromium.org/656423004]

[rafaelw, 2014-06-22 23:59:19]

PTAL. What do you think should be done with some of these tests which now
timeout because SmartMove is gone.

https://codereview.chromium.org/349073002/diff/40001/src/array.js
File src/array.js (right):

https://codereview.chromium.org/349073002/diff/40001/src/array.js#newcode243
src/array.js:243: if (num_additional_args !== del_count || force) {
Note that |force| is required here because the spec for unshift requires that
read & assignment from each index position still take place even if there are no
arugments (observable if there are data properties on the proto in element
positions < length, but none on the object.

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-funct...
File test/mjsunit/array-functions-prototype-misc.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-funct...
test/mjsunit/array-functions-prototype-misc.js:34: /*
This is currently testing the existing SmartMove code path in two dimensions:

1) By applying array methods to array-likes (line 94)
2) By putting a index property on the prototype (line 142)

These two are problematic only because of the size of the array(like)s used. My
suggestion is to alter this test to use a sufficiently small size for these two
paths.

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-nativ...
File test/mjsunit/array-natives-elements.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-nativ...
test/mjsunit/array-natives-elements.js:298:
assertTrue(%HasFastDoubleElements(a4));
It looks like this is actually the preferred backing store at this point.

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-shift...
File test/mjsunit/array-shift2.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-shift...
test/mjsunit/array-shift2.js:15: assertEquals(["element 1","element 1"],
test(["0",,2]));
This test was actually wrong. Given that the property on the proto is an
accessor, assignment should invoke setter, NOT write to the object.

It's not clear to me which of these three cases should remain. Maybe put the
"odd" object at element position 3.

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-unshi...
File test/mjsunit/array-unshift.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-unshi...
test/mjsunit/array-unshift.js:193: /*
So I assume the RangeError gets thrown here the first time an attempt is made to
assign to length a value which is outside of UInt32. Is this going through the
JS path because of the size of the Array?

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/bugs/bug-26...
File test/mjsunit/bugs/bug-2615.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/bugs/bug-26...
test/mjsunit/bugs/bug-2615.js:27: /*
What's happening with this test is somewhat mysterious to me. For some reason,
when you run bleeding_edge check, this test doesn't fail -- unless I run it
directly.

However, it's clear the problem is performing array operations when the size is
very large (e.g. SimpleMove vs SmartMove) and the method gets pushed onto the JS
Path.

[Toon Verwaest, 2014-06-23 08:42:36]

https://codereview.chromium.org/349073002/diff/40001/src/array.js
File src/array.js (right):

https://codereview.chromium.org/349073002/diff/40001/src/array.js#newcode732
src/array.js:732: SimpleSlice(array, start_i, del_count, len, deleted_elements);
I guess now you can remove the "Simple" prefixes

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-funct...
File test/mjsunit/array-functions-prototype-misc.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-funct...
test/mjsunit/array-functions-prototype-misc.js:34: /*
Sounds good for now.

We'll probably need to build a 4th optimization layer for dictionary mode
objects for splice, move, shift and unshift. But lets get it correct first. This
may cause regressions though, so we'll need to keep an eye on it.

On 2014/06/22 23:59:18, rafaelw wrote:
> This is currently testing the existing SmartMove code path in two dimensions:
> 
> 1) By applying array methods to array-likes (line 94)
> 2) By putting a index property on the prototype (line 142)
> 
> These two are problematic only because of the size of the array(like)s used.
My
> suggestion is to alter this test to use a sufficiently small size for these
two
> paths.

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-funct...
test/mjsunit/array-functions-prototype-misc.js:91: // Don't run largest size on
non-arrays or we'll be here for ever.
.. ok :)

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-shift...
File test/mjsunit/array-shift2.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-shift...
test/mjsunit/array-shift2.js:15: assertEquals(["element 1","element 1"],
test(["0",,2]));
Sounds good and correct.

On 2014/06/22 23:59:19, rafaelw wrote:
> This test was actually wrong. Given that the property on the proto is an
> accessor, assignment should invoke setter, NOT write to the object.
> 
> It's not clear to me which of these three cases should remain. Maybe put the
> "odd" object at element position 3.

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-unshi...
File test/mjsunit/array-unshift.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-unshi...
test/mjsunit/array-unshift.js:193: /*
Yeah. There's a limit on fast-mode objects. They always have a smi-based length,
but iirc there's even a lower limit. Dictionary mode arrays go to JS code.

On 2014/06/22 23:59:19, rafaelw wrote:
> So I assume the RangeError gets thrown here the first time an attempt is made
to
> assign to length a value which is outside of UInt32. Is this going through the
> JS path because of the size of the Array?

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/bugs/bug-26...
File test/mjsunit/bugs/bug-2615.js (right):

https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/bugs/bug-26...
test/mjsunit/bugs/bug-2615.js:27: /*
I wrote this test at some point to have a list of all known JSArray bugs. Some
of these tests should now be fixed I presume (and some may run too slow as you
indicate). We should move the now working parts of this bug from bug to test.

On 2014/06/22 23:59:19, rafaelw wrote:
> What's happening with this test is somewhat mysterious to me. For some reason,
> when you run bleeding_edge check, this test doesn't fail -- unless I run it
> directly.
> 
> However, it's clear the problem is performing array operations when the size
is
> very large (e.g. SimpleMove vs SmartMove) and the method gets pushed onto the
JS
> Path.