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-functions-prototype-misc.js
File test/mjsunit/array-functions-prototype-misc.js (right):
https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-functions-prototype-misc.js#newcode34
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-functions-prototype-misc.js#newcode91
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-shift2.js
File test/mjsunit/array-shift2.js (right):
https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-shift2.js#newcode15
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-unshift.js
File test/mjsunit/array-unshift.js (right):
https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-unshift.js#newcode193
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-2615.js
File test/mjsunit/bugs/bug-2615.js (right):
https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/bugs/bug-2615.js#newcode27
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.
https://codereview.chromium.org/349073002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
