Reviewers: Toon Verwaest,
Message:
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-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: /*
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-natives-elements.js
File test/mjsunit/array-natives-elements.js (right):
https://codereview.chromium.org/349073002/diff/40001/test/mjsunit/array-natives-elements.js#newcode298
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-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]));
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: /*
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: /*
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.
Description:
Remove SmartMove from array.js.
This patch is progress towards having a single, spec-compliant array.js
path for
all objects.
Please review this at https://codereview.chromium.org/349073002/
SVN Base: git://github.com/v8/v8.git@bleeding_edge
Affected files (+19, -84 lines):
M src/array.js
M test/mjsunit/array-functions-prototype-misc.js
M test/mjsunit/array-natives-elements.js
M test/mjsunit/array-shift2.js
M test/mjsunit/array-unshift.js
M test/mjsunit/bugs/bug-2615.js
--
--
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.
