Use ES2015-style TypedArray prototype chain

Use ES2015-style TypedArray prototype chain

This patch switches TypedArrays to use the prototype chain described
in the ES2015 specification, which adds a %TypedArray% superclass above
all individual TypedArray types. Most methods are defined on the
superclass rather than the subclasses.

In order to prevent a performance regression, a few methods are
marked as inline. Inlining might prevent code which was previously
monomorphic from becoming polymorphic, and it was specifically
applied in places where methods became more polymorphic than before.
Tests with realistic workloads would be nice to do before this
ships in stable.

This patch does not bring TypedArrays up to full spec compliance. In
particular, @@species is not yet supported.

R=cbruni
BUG=v8:4085
LOG=Y

Committed: https://crrev.com/07c91dccbe55c7be3ec75857dee5ad59873330b7
Cr-Commit-Position: refs/heads/master@{#33050}

[Dan Ehrenberg, 2015-12-22 20:46:08]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-22 20:46:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541233002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541233002/1

[commit-bot: I haz the power, 2015-12-22 20:48:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-22 20:48:16]

Dry run: Try jobs failed on following builders:
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/13514)

[Dan Ehrenberg, 2015-12-22 21:28:38]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-22 21:28:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541233002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541233002/20001

[commit-bot: I haz the power, 2015-12-22 21:41:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-22 21:41:08]

Dry run: Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[Dan Ehrenberg, 2015-12-22 21:55:10]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-22 21:55:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541233002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541233002/40001

[commit-bot: I haz the power, 2015-12-22 22:13:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-22 22:13:31]

Dry run: This issue passed the CQ dry run.

[Dan Ehrenberg, 2015-12-22 22:48:20]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-22 22:48:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541233002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541233002/60001

[Dan Ehrenberg, 2015-12-22 22:49:01]

Description was changed from

==========
Use ES2015-style TypedArray prototype chain

This patch switches TypedArrays to use the prototype chain described
in the ES2015 specification, which adds a %TypedArray% superclass above
all individual TypedArray types. Most methods are defined on the
superclass rather than the subclasses.

In order to prevent a performance regression, a few methods are
marked as inline. Inlining might prevent code which was previously
monomorphic from becoming polymorphic, and it was specifically
applied in places where methods became more polymorphic than before.
Tests with realistic workloads would be nice to do before this
ships in stable.

This patch does not bring TypedArrays up to full spec compliance. In
particular, more work is still needed for subclassing and @@species.

R=cbruni
BUG=v8:4085
LOG=Y
==========

to

==========
Use ES2015-style TypedArray prototype chain

This patch switches TypedArrays to use the prototype chain described
in the ES2015 specification, which adds a %TypedArray% superclass above
all individual TypedArray types. Most methods are defined on the
superclass rather than the subclasses.

In order to prevent a performance regression, a few methods are
marked as inline. Inlining might prevent code which was previously
monomorphic from becoming polymorphic, and it was specifically
applied in places where methods became more polymorphic than before.
Tests with realistic workloads would be nice to do before this
ships in stable.

This patch does not bring TypedArrays up to full spec compliance. In
particular, @@species is not yet supported.

R=cbruni
BUG=v8:4085
LOG=Y
==========

[commit-bot: I haz the power, 2015-12-22 23:11:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-22 23:11:03]

Dry run: This issue passed the CQ dry run.

[caitp (gmail), 2015-12-22 23:47:45]

caitpotter88@gmail.com changed reviewers:
+ caitpotter88@gmail.com

[caitp (gmail), 2015-12-22 23:47:46]

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js
File src/js/typedarray.js (right):

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:745: if (new.target === TypedArray) {
question: how is this supposed to work if new.target is TypedArray from a
different context, with __proto__ set to this context's TypedArray? It doesn't
look like it would throw in that case, and could possibly cause some assertions
to fail when methods are invoked on it. I'm not sure

[Dan Ehrenberg, 2015-12-22 23:53:08]

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js
File src/js/typedarray.js (right):

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:745: if (new.target === TypedArray) {
On 2015/12/22 at 23:47:45, caitp wrote:
> question: how is this supposed to work if new.target is TypedArray from a
different context, with __proto__ set to this context's TypedArray? It doesn't
look like it would throw in that case, and could possibly cause some assertions
to fail when methods are invoked on it. I'm not sure

Well, this definitely implements the spec in this particular respect. TypedArray
will refer to that same function object that is currently executing. I argued
that this test is not meaningful to have on GitHub, but I ended up going with
Allen's suggestion to include the check.

[caitp (gmail), 2015-12-22 23:56:28]

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js
File src/js/typedarray.js (right):

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:745: if (new.target === TypedArray) {
On 2015/12/22 23:53:08, Dan Ehrenberg wrote:
> On 2015/12/22 at 23:47:45, caitp wrote:
> > question: how is this supposed to work if new.target is TypedArray from a
> different context, with __proto__ set to this context's TypedArray? It doesn't
> look like it would throw in that case, and could possibly cause some
assertions
> to fail when methods are invoked on it. I'm not sure
> 
> Well, this definitely implements the spec in this particular respect.
TypedArray
> will refer to that same function object that is currently executing. I argued
> that this test is not meaningful to have on GitHub, but I ended up going with
> Allen's suggestion to include the check.

It looks spec compliant --- but I'm not sure if it necessarily interacts well
with the rest of the TypedArray implementation in V8. I might be overthinking
it, but maybe a cctest with mixesd contexts + TypedArray method invocations
might be helpful

[Dan Ehrenberg, 2015-12-23 00:02:04]

On 2015/12/22 at 23:56:28, caitpotter88 wrote:
> https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js
> File src/js/typedarray.js (right):
> 
>
https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
> src/js/typedarray.js:745: if (new.target === TypedArray) {
> On 2015/12/22 23:53:08, Dan Ehrenberg wrote:
> > On 2015/12/22 at 23:47:45, caitp wrote:
> > > question: how is this supposed to work if new.target is TypedArray from a
> > different context, with __proto__ set to this context's TypedArray? It
doesn't
> > look like it would throw in that case, and could possibly cause some
assertions
> > to fail when methods are invoked on it. I'm not sure
> > 
> > Well, this definitely implements the spec in this particular respect.
TypedArray
> > will refer to that same function object that is currently executing. I
argued
> > that this test is not meaningful to have on GitHub, but I ended up going
with
> > Allen's suggestion to include the check.
> 
> It looks spec compliant --- but I'm not sure if it necessarily interacts well
with the rest of the TypedArray implementation in V8. I might be overthinking
it, but maybe a cctest with mixesd contexts + TypedArray method invocations
might be helpful

I really can't picture how this part of it could cause something bad to happen.
I'd be more worried about the interaction with bootstrapping, if anything (but I
don't have any really specific worry). If you have a more concrete test case
(idea), then I'd be happy to look/think closer at/about it.

[Camillo Bruni, 2015-12-23 15:56:02]

I've got some nits here and there.
I'm not sure about the SetForceInlineFlag since that's rarely used in the
system, I'd have to refer to somebody else. You could probably land it with the
inlining set and we discuss later once more people are around. 
WDYT?

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js
File src/js/typedarray.js (right):

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:269: TYPED_ARRAYS(TYPED_ARRAY_SUBARRAY_CASE)
Now that we have a common prototype we could implement the SubArray helper
directly here (the ELEMENT_SIZE and GlobalNAME are the two different args that
could be stored in a local variable here.

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:745: if (new.target === TypedArray) {
> It looks spec compliant --- but I'm not sure if it necessarily interacts well
with the rest of the TypedArray implementation in V8. I might be overthinking
it, but maybe a cctest with mixesd contexts + TypedArray method invocations
might be helpful

Just use Realms, it's quite easy (see for instance
proxies-cross-realm-exception.js).

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:752: %FunctionSetPrototype(TypedArray, new GlobalObject());
According to the Spec 22.2.2.3 the prototype should be read-only, but I think in
your case it's configurable (probably you have to create TypedArray in
bootstrapper.cc)

https://codereview.chromium.org/1541233002/diff/60001/test/mjsunit/es6/typed-...
File test/mjsunit/es6/typed-array-iterator.js (right):

https://codereview.chromium.org/1541233002/diff/60001/test/mjsunit/es6/typed-...
test/mjsunit/es6/typed-array-iterator.js:17:
assertTrue(TypedArrayPrototype.hasOwnProperty(Symbol.iterator));
Could you check that all the other TypedArray constructor.prototypes don't have
these properties directly?

[Dan Ehrenberg, 2015-12-23 23:14:46]

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js
File src/js/typedarray.js (right):

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:269: TYPED_ARRAYS(TYPED_ARRAY_SUBARRAY_CASE)
On 2015/12/23 at 15:56:02, cbruni wrote:
> Now that we have a common prototype we could implement the SubArray helper
directly here (the ELEMENT_SIZE and GlobalNAME are the two different args that
could be stored in a local variable here.

I was initially doing that, but I actually wanted the switch statement to be
inlined and the things it calls out to to be out-of-line. Of course, decisions
like these should be validated with benchmarks. I reached out to a member of the
Tactile team for this, but haven't heard back yet. I can wait to land this until
I there's more of that kind of evidence, if you prefer.

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:745: if (new.target === TypedArray) {
On 2015/12/23 at 15:56:02, cbruni wrote:
> > It looks spec compliant --- but I'm not sure if it necessarily interacts
well with the rest of the TypedArray implementation in V8. I might be
overthinking it, but maybe a cctest with mixesd contexts + TypedArray method
invocations might be helpful
> 
> Just use Realms, it's quite easy (see for instance
proxies-cross-realm-exception.js).

Well, to write a test I'll have to understand more about the potential issue. I
still don't really see what could go wrong here. But thanks for the
reference--that'll be really useful for when I'm testing @@species.

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:752: %FunctionSetPrototype(TypedArray, new GlobalObject());
On 2015/12/23 at 15:56:02, cbruni wrote:
> According to the Spec 22.2.2.3 the prototype should be read-only, but I think
in your case it's configurable (probably you have to create TypedArray in
bootstrapper.cc)

It seems to be nonconfigurable, nonwritable, nonenumerable, as the spec
requires. I added a test for this. I'm having trouble finding out how this works
exactly, though...

https://codereview.chromium.org/1541233002/diff/60001/test/mjsunit/es6/typed-...
File test/mjsunit/es6/typed-array-iterator.js (right):

https://codereview.chromium.org/1541233002/diff/60001/test/mjsunit/es6/typed-...
test/mjsunit/es6/typed-array-iterator.js:17:
assertTrue(TypedArrayPrototype.hasOwnProperty(Symbol.iterator));
On 2015/12/23 at 15:56:02, cbruni wrote:
> Could you check that all the other TypedArray constructor.prototypes don't
have these properties directly?

That's checked in typedarray-proto.js

[Dan Ehrenberg, 2015-12-23 23:14:56]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-23 23:15:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541233002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541233002/80001

[commit-bot: I haz the power, 2015-12-23 23:37:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-23 23:37:12]

Dry run: This issue passed the CQ dry run.

[Camillo Bruni, 2015-12-28 08:54:46]

lgtm
in case the SetForceInlineFlag should pose any problems, we can easily revert
it.

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js
File src/js/typedarray.js (right):

https://codereview.chromium.org/1541233002/diff/60001/src/js/typedarray.js#ne...
src/js/typedarray.js:752: %FunctionSetPrototype(TypedArray, new GlobalObject());
On 2015/12/23 at 23:14:45, Dan Ehrenberg wrote:
> On 2015/12/23 at 15:56:02, cbruni wrote:
> > According to the Spec 22.2.2.3 the prototype should be read-only, but I
think in your case it's configurable (probably you have to create TypedArray in
bootstrapper.cc)
> 
> It seems to be nonconfigurable, nonwritable, nonenumerable, as the spec
requires. I added a test for this. I'm having trouble finding out how this works
exactly, though...

ah ok then :) I thought that this would only be the case for things set up in
bootstrapper.cc.

[Dan Ehrenberg, 2015-12-28 16:43:24]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2015-12-28 16:43:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541233002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541233002/80001

[commit-bot: I haz the power, 2015-12-28 17:28:19]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-28 17:29:02]

Description was changed from

==========
Use ES2015-style TypedArray prototype chain

This patch switches TypedArrays to use the prototype chain described
in the ES2015 specification, which adds a %TypedArray% superclass above
all individual TypedArray types. Most methods are defined on the
superclass rather than the subclasses.

In order to prevent a performance regression, a few methods are
marked as inline. Inlining might prevent code which was previously
monomorphic from becoming polymorphic, and it was specifically
applied in places where methods became more polymorphic than before.
Tests with realistic workloads would be nice to do before this
ships in stable.

This patch does not bring TypedArrays up to full spec compliance. In
particular, @@species is not yet supported.

R=cbruni
BUG=v8:4085
LOG=Y
==========

to

==========
Use ES2015-style TypedArray prototype chain

This patch switches TypedArrays to use the prototype chain described
in the ES2015 specification, which adds a %TypedArray% superclass above
all individual TypedArray types. Most methods are defined on the
superclass rather than the subclasses.

In order to prevent a performance regression, a few methods are
marked as inline. Inlining might prevent code which was previously
monomorphic from becoming polymorphic, and it was specifically
applied in places where methods became more polymorphic than before.
Tests with realistic workloads would be nice to do before this
ships in stable.

This patch does not bring TypedArrays up to full spec compliance. In
particular, @@species is not yet supported.

R=cbruni
BUG=v8:4085
LOG=Y

Committed: https://crrev.com/07c91dccbe55c7be3ec75857dee5ad59873330b7
Cr-Commit-Position: refs/heads/master@{#33050}
==========

[commit-bot: I haz the power, 2015-12-28 17:29:03]

Patchset 5 (id:??) landed as
https://crrev.com/07c91dccbe55c7be3ec75857dee5ad59873330b7
Cr-Commit-Position: refs/heads/master@{#33050}

[Michael Achenbach, 2015-12-29 08:42:40]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1554523002/ by machenbach@chromium.org.

The reason for reverting is: [Sheriff] Changes layout tests. Please fix upstream
first if intended.

https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/bui....