New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 711275 link

Starred by 8 users

Issue metadata

Status: Fixed
Owner:
OOO until 2019-02-10
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: FeatureRequest

Blocking:
issue v8:5267
issue 698746
issue 700294


Participants' hotlists:
I-TF-Launch

Previous locations:
v8:5977


Sign in to add a comment

Port TypedArray subclass constructors to CSA

Project Member Reported by bmeu...@chromium.org, Feb 16 2017

Issue description

Currently the TypedArray subclass constructors (i.e. Uint8Array, Int32Array, etc.) are implemented in a funky mix of magical Crankshaft intrinsics, C++ runtime functions and JavaScript. This is not only a nightmare for security and already led to a couple of challenging security bugs, but also not ideal for performance.

The construct stub part of those constructors should be implemented as CSA builtins, ideally sharing the meat to avoid wasting too much code space. The [[Call]] part can be a simple C++ builtin, since it only throws.

Long-term plan is to be able to inline allocations of small typed arrays into TurboFan, so stuff like new Int32Array(4).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/56312be185230f483253bfb54c77952535b4dc0c

commit 56312be185230f483253bfb54c77952535b4dc0c
Author: Peter Marshall <petermarshall@chromium.org>
Date: Wed Feb 22 10:33:43 2017

[cleanup] Refactor builtins-typedarray.cc to use TF_BUILTIN macro.

This is in preparation for porting TypedArrayInitialize to CSA.

BUG=v8:5977

Change-Id: I8b4b4bc7a30f3d0dedf85081bb47ec613c7fce52
Reviewed-on: https://chromium-review.googlesource.com/445259
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43364}
[modify] https://crrev.com/56312be185230f483253bfb54c77952535b4dc0c/src/builtins/builtins-typedarray.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/45dfd76a9c38e6a872ac14e62faee2ace1f1dbac

commit 45dfd76a9c38e6a872ac14e62faee2ace1f1dbac
Author: Peter Marshall <petermarshall@chromium.org>
Date: Fri Feb 24 13:06:10 2017

[Test] Add perf tests for TypedArray constructors.

Adds a perf test for constructing a TypedArray from a regular array,
and from a pre-made ArrayBuffer. Runs both new tests with default and
future configurations for comparison.

BUG=v8:5977

Change-Id: Idd132ca879702c54b2947a0e57ed8fe782f2767f
Reviewed-on: https://chromium-review.googlesource.com/446342
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43412}
[modify] https://crrev.com/45dfd76a9c38e6a872ac14e62faee2ace1f1dbac/test/js-perf-test/JSTests.json
[add] https://crrev.com/45dfd76a9c38e6a872ac14e62faee2ace1f1dbac/test/js-perf-test/TypedArrays/construct-arraylike.js
[add] https://crrev.com/45dfd76a9c38e6a872ac14e62faee2ace1f1dbac/test/js-perf-test/TypedArrays/construct-buffer.js

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6

commit b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6
Author: Peter Marshall <petermarshall@chromium.org>
Date: Wed Mar 01 14:28:23 2017

[builtins] Port TypedArrayInitialize to CodeStubAssembler.

Turbofan is a lot slower than Crankshaft at constructing TypedArrays,
because we always go to the C++ builtin. Port the builtin to CSA
to improve performance, and to clean up the implementation, which is
split across multiple files and pieces at the moment.

This CL increases the performance with --future to roughly the same
as with crankshaft.

BUG=v8:5977

Change-Id: I5a4c4b544a735a56290b85bf33c2f3718df7e2b8
Reviewed-on: https://chromium-review.googlesource.com/445717
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43518}
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/assembler.cc
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/assembler.h
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/bootstrapper.cc
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/builtins/builtins-typedarray.cc
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/builtins/builtins.h
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/contexts.h
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/external-reference-table.cc
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/factory.cc
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/factory.h
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/src/js/typedarray.js
[modify] https://crrev.com/b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6/test/mjsunit/es6/typedarray.js

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a8e15e8fc5d15541d52351779e128615e602af8f

commit a8e15e8fc5d15541d52351779e128615e602af8f
Author: Peter Marshall <petermarshall@chromium.org>
Date: Wed Mar 01 15:55:51 2017

Revert "[builtins] Port TypedArrayInitialize to CodeStubAssembler."

This reverts commit b23b2c107b5d79fa1dbc7035cda17d668ad5f4f6.

Reason for revert: Makes Linux debug bot sad

Original change's description:
> [builtins] Port TypedArrayInitialize to CodeStubAssembler.
> 
> Turbofan is a lot slower than Crankshaft at constructing TypedArrays,
> because we always go to the C++ builtin. Port the builtin to CSA
> to improve performance, and to clean up the implementation, which is
> split across multiple files and pieces at the moment.
> 
> This CL increases the performance with --future to roughly the same
> as with crankshaft.
> 
> BUG=v8:5977
> 
> Change-Id: I5a4c4b544a735a56290b85bf33c2f3718df7e2b8
> Reviewed-on: https://chromium-review.googlesource.com/445717
> Commit-Queue: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#43518}

TBR=cbruni@chromium.org,petermarshall@chromium.org,bmeurer@chromium.org,v8-reviews@googlegroups.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5977

Change-Id: I5d5bc8b4677a405c716d78e688af80ae9c737b4a
Reviewed-on: https://chromium-review.googlesource.com/448558
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43520}
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/assembler.cc
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/assembler.h
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/bootstrapper.cc
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/builtins/builtins-typedarray.cc
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/builtins/builtins.h
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/contexts.h
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/external-reference-table.cc
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/factory.cc
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/factory.h
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/src/js/typedarray.js
[modify] https://crrev.com/a8e15e8fc5d15541d52351779e128615e602af8f/test/mjsunit/es6/typedarray.js

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5c200fa0f105a37bb1a505e386f71603f568c54e

commit 5c200fa0f105a37bb1a505e386f71603f568c54e
Author: Peter Marshall <petermarshall@chromium.org>
Date: Thu Mar 02 13:41:21 2017

[builtins] Delete unused TypedArrayInitialize intrinsic.

This CL only deletes code. We dont call these anymore, so they are safe
to remove.

BUG=v8:5977

Change-Id: I59889c3dbb9c2610f3502d582b6c307b1fb4f63b
Reviewed-on: https://chromium-review.googlesource.com/448517
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43543}
[modify] https://crrev.com/5c200fa0f105a37bb1a505e386f71603f568c54e/src/crankshaft/hydrogen.cc
[modify] https://crrev.com/5c200fa0f105a37bb1a505e386f71603f568c54e/src/crankshaft/hydrogen.h
[modify] https://crrev.com/5c200fa0f105a37bb1a505e386f71603f568c54e/src/runtime/runtime-typedarray.cc
[modify] https://crrev.com/5c200fa0f105a37bb1a505e386f71603f568c54e/src/runtime/runtime.h

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/cb672f4df6fea26eda8a9dbd83c8cada739b7e84

commit cb672f4df6fea26eda8a9dbd83c8cada739b7e84
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu Mar 02 13:48:53 2017

Revert "[builtins] Delete unused TypedArrayInitialize intrinsic."

This reverts commit 5c200fa0f105a37bb1a505e386f71603f568c54e.

Reason for revert: Breaks compile:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20builder/builds/23538

Maybe conflicts with a change that just landed.

Original change's description:
> [builtins] Delete unused TypedArrayInitialize intrinsic.
> 
> This CL only deletes code. We dont call these anymore, so they are safe
> to remove.
> 
> BUG=v8:5977
> 
> Change-Id: I59889c3dbb9c2610f3502d582b6c307b1fb4f63b
> Reviewed-on: https://chromium-review.googlesource.com/448517
> Commit-Queue: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#43543}

TBR=cbruni@chromium.org,petermarshall@chromium.org,bmeurer@chromium.org,v8-reviews@googlegroups.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5977

Change-Id: Iba1611f4c93d105a4163338b59bda42ea7937443
Reviewed-on: https://chromium-review.googlesource.com/448562
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43544}
[modify] https://crrev.com/cb672f4df6fea26eda8a9dbd83c8cada739b7e84/src/crankshaft/hydrogen.cc
[modify] https://crrev.com/cb672f4df6fea26eda8a9dbd83c8cada739b7e84/src/crankshaft/hydrogen.h
[modify] https://crrev.com/cb672f4df6fea26eda8a9dbd83c8cada739b7e84/src/runtime/runtime-typedarray.cc
[modify] https://crrev.com/cb672f4df6fea26eda8a9dbd83c8cada739b7e84/src/runtime/runtime.h

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e68ce4f237768cc192394816e4887312c9954d75

commit e68ce4f237768cc192394816e4887312c9954d75
Author: Peter Marshall <petermarshall@chromium.org>
Date: Thu Mar 02 13:50:05 2017

Revert "[builtins] Delete unused TypedArrayInitialize intrinsic."

This reverts commit 5c200fa0f105a37bb1a505e386f71603f568c54e.

Reason for revert: Relies on changes that were reverted.

Original change's description:
> [builtins] Delete unused TypedArrayInitialize intrinsic.
> 
> This CL only deletes code. We dont call these anymore, so they are safe
> to remove.
> 
> BUG=v8:5977
> 
> Change-Id: I59889c3dbb9c2610f3502d582b6c307b1fb4f63b
> Reviewed-on: https://chromium-review.googlesource.com/448517
> Commit-Queue: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#43543}

TBR=cbruni@chromium.org,petermarshall@chromium.org,bmeurer@chromium.org,v8-reviews@googlegroups.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5977

Change-Id: I41f32b0b8f74bcfdf9afbd7cc150cca9f5edd199
Reviewed-on: https://chromium-review.googlesource.com/448563
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43545}

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262

commit ff8b1abb1a8e609b7786d0f6cd6577cc9083d262
Author: Peter Marshall <petermarshall@chromium.org>
Date: Thu Mar 02 14:31:18 2017

[builtins] Reland of Port TypedArrayInitialize to CodeStubAssembler.

Turbofan is a lot slower than Crankshaft at constructing TypedArrays,
because we always go to the C++ builtin. Port the builtin to CSA
to improve performance, and to clean up the implementation, which is
split across multiple files and pieces at the moment.

This CL increases the performance with --future to roughly the same
as with crankshaft.

BUG=v8:5977

Change-Id: Id0d91a4592de41a3a308846d79bd44a608931762
Reviewed-on: https://chromium-review.googlesource.com/448537
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43548}
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/assembler.cc
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/assembler.h
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/bootstrapper.cc
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/builtins/builtins-typedarray.cc
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/builtins/builtins.h
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/contexts.h
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/external-reference-table.cc
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/factory.cc
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/factory.h
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/src/js/typedarray.js
[modify] https://crrev.com/ff8b1abb1a8e609b7786d0f6cd6577cc9083d262/test/mjsunit/es6/typedarray.js

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5f79c9231af02dad7125b31af2772bc3f4aa9793

commit 5f79c9231af02dad7125b31af2772bc3f4aa9793
Author: Peter Marshall <petermarshall@chromium.org>
Date: Fri Mar 03 14:42:34 2017

[builtins] Ensure length is within Smi range in TypedArray constructor.

The callsite in ConstructByArrayBuffer could have a length that is
above Smi range if the buffer had such a length. Check this before
calling. Add a test too.

BUG=v8:5977,  chromium:698201 

Change-Id: Ic22046a31607f1f85642c8caf7f5ed064edb3110
Reviewed-on: https://chromium-review.googlesource.com/449813
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43589}
[modify] https://crrev.com/5f79c9231af02dad7125b31af2772bc3f4aa9793/src/js/typedarray.js
[modify] https://crrev.com/5f79c9231af02dad7125b31af2772bc3f4aa9793/test/mjsunit/es6/typedarray.js

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ead3656cbde8a2314ec86a593b1401a59753978f

commit ead3656cbde8a2314ec86a593b1401a59753978f
Author: Peter Marshall <petermarshall@chromium.org>
Date: Tue Mar 07 09:15:17 2017

[builtins] Port TypedArrayConstructByLength to CodeStubAssembler.

Part of the performance and refactoring work to move the TypedArray
constructors into CSA. This CL moves ConstructByLength from JS
to CSA.

There are still other callers to typed_array_initialize in
typedarray.js, so we share the implementation using DoInitialize.

In a later CL we can split apart DoInitialize once we have more
TA constructors written in CSA, so that we can reuse specific
parts more easily.

BUG=v8:5977

Change-Id: Ia51e8363970e9a025a82933e56a7baaf82cb1eec
Reviewed-on: https://chromium-review.googlesource.com/448220
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43626}
[modify] https://crrev.com/ead3656cbde8a2314ec86a593b1401a59753978f/src/bootstrapper.cc
[modify] https://crrev.com/ead3656cbde8a2314ec86a593b1401a59753978f/src/builtins/builtins-typedarray.cc
[modify] https://crrev.com/ead3656cbde8a2314ec86a593b1401a59753978f/src/builtins/builtins.h
[modify] https://crrev.com/ead3656cbde8a2314ec86a593b1401a59753978f/src/contexts.h
[modify] https://crrev.com/ead3656cbde8a2314ec86a593b1401a59753978f/src/js/typedarray.js
[modify] https://crrev.com/ead3656cbde8a2314ec86a593b1401a59753978f/src/runtime/runtime-internal.cc
[modify] https://crrev.com/ead3656cbde8a2314ec86a593b1401a59753978f/src/runtime/runtime.h
[modify] https://crrev.com/ead3656cbde8a2314ec86a593b1401a59753978f/test/mjsunit/es6/typedarray.js

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/06fef85bdd2db7418b75c4f64ac164f3fbe90e30

commit 06fef85bdd2db7418b75c4f64ac164f3fbe90e30
Author: Peter Marshall <petermarshall@chromium.org>
Date: Mon Mar 13 14:04:37 2017

[builtins] Port TypedArrayConstructByArrayBuffer to CodeStubAssembler.

Part of the performance and refactoring work to move the TypedArray
constructors into CSA. This CL moves ConstructByArrayBuffer from JS
to CSA.

BUG=v8:5977

Change-Id: I0a200e6b3f6261ea2372ea9c3d3ca98e313cf2c5
Reviewed-on: https://chromium-review.googlesource.com/451620
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43747}
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/bootstrapper.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/builtins/builtins-typedarray.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/builtins/builtins.h
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/code-stub-assembler.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/code-stub-assembler.h
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/code-assembler.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/code-assembler.h
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/instruction-selector.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/machine-graph-verifier.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/machine-operator.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/machine-operator.h
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/mips64/instruction-selector-mips64.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/opcodes.h
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/raw-machine-assembler.h
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/verifier.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/compiler/x64/instruction-selector-x64.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/contexts.h
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/js/typedarray.js
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/runtime/runtime-internal.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/runtime/runtime-typedarray.cc
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/src/runtime/runtime.h
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/test/mjsunit/es6/typedarray-construct-by-buffer-ordering.js
[add] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/test/mjsunit/es6/typedarray-construct-offset-not-smi.js
[modify] https://crrev.com/06fef85bdd2db7418b75c4f64ac164f3fbe90e30/test/mjsunit/es6/typedarray.js

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d3f236fa054413e9a81c95aef620605968282a82

commit d3f236fa054413e9a81c95aef620605968282a82
Author: bjaideep <bjaideep@ca.ibm.com>
Date: Mon Mar 13 19:18:08 2017

PPC/s390: [builtins] Port TypedArrayConstructByArrayBuffer to CodeStubAssembler.

Port 06fef85bdd2db7418b75c4f64ac164f3fbe90e30

Original Commit Message:

    Part of the performance and refactoring work to move the TypedArray
    constructors into CSA. This CL moves ConstructByArrayBuffer from JS
    to CSA.

R=petermarshall@chromium.org, joransiu@ca.ibm.com, jyan@ca.ibm.com, michael_dawson@ca.ibm.com
BUG=v8:5977
LOG=N

Review-Url: https://codereview.chromium.org/2742343005
Cr-Commit-Position: refs/heads/master@{#43756}

[modify] https://crrev.com/d3f236fa054413e9a81c95aef620605968282a82/src/compiler/ppc/instruction-selector-ppc.cc
[modify] https://crrev.com/d3f236fa054413e9a81c95aef620605968282a82/src/compiler/s390/instruction-selector-s390.cc

Blocking: chromium:700294
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/516374659100628cb171a074a878306cc858691d

commit 516374659100628cb171a074a878306cc858691d
Author: Peter Marshall <petermarshall@chromium.org>
Date: Mon Mar 20 13:23:52 2017

[builtins] Add a fastpath in TypedArrayConstructByArrayBuffer.

Add a fastpath for when byteOffset is undefined, which is the common
case. We can just replace it with 0 and avoid the modulo checks. Also
add a smi-fastpath for byteOffset, which avoids calling stubs for
arithmetic when unnecessary.

BUG=chromium:701668,v8:5977

Change-Id: Id431dad46bf3796ef32ab465f6787bbebe83437c
Reviewed-on: https://chromium-review.googlesource.com/456502
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Franziska Hinkelmann <franzih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43933}
[modify] https://crrev.com/516374659100628cb171a074a878306cc858691d/src/builtins/builtins-typedarray-gen.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/0feed731d03853cb48201f3284cabd58f1bc74e5

commit 0feed731d03853cb48201f3284cabd58f1bc74e5
Author: Peter Marshall <petermarshall@chromium.org>
Date: Tue Mar 21 12:01:28 2017

[Test] Add a perf test for TypedArray construct by typed array.

BUG=v8:5977

Change-Id: Ic756fd44a945f98d51c0914dcc6c3b82111d170d
Reviewed-on: https://chromium-review.googlesource.com/456419
Reviewed-by: Franziska Hinkelmann <franzih@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43978}
[modify] https://crrev.com/0feed731d03853cb48201f3284cabd58f1bc74e5/test/js-perf-test/JSTests.json
[add] https://crrev.com/0feed731d03853cb48201f3284cabd58f1bc74e5/test/js-perf-test/TypedArrays/construct-typedarray.js

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6f800b32ad381e302e80cf378447192927bf6287

commit 6f800b32ad381e302e80cf378447192927bf6287
Author: Peter Marshall <petermarshall@chromium.org>
Date: Wed Mar 22 12:26:43 2017

[builtins] Delete unused TypedArrayInitialize intrinsic.

Deletes unused crankshaft implementation and C++ implementation,
which have been replaced by a CSA implementation.

BUG=v8:5977

Change-Id: I3614561e45db48583ee886461f98abb14cd9cc4f
Reviewed-on: https://chromium-review.googlesource.com/458418
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44018}
[modify] https://crrev.com/6f800b32ad381e302e80cf378447192927bf6287/src/crankshaft/hydrogen.cc
[modify] https://crrev.com/6f800b32ad381e302e80cf378447192927bf6287/src/crankshaft/hydrogen.h
[modify] https://crrev.com/6f800b32ad381e302e80cf378447192927bf6287/src/runtime/runtime-typedarray.cc
[modify] https://crrev.com/6f800b32ad381e302e80cf378447192927bf6287/src/runtime/runtime.h

Labels: Priority-1
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/14e01da1cf6c02628252ed267ca5043f44d63a5d

commit 14e01da1cf6c02628252ed267ca5043f44d63a5d
Author: Peter Marshall <petermarshall@chromium.org>
Date: Fri Mar 24 17:40:22 2017

[builtins] Port TypedArrayConstructByArrayLike to CodeStubAssembler.

This helper is used directly when constructing from an object with
a length, as well as by ConstructByIterable and ByTypedArray.

BUG=v8:5977

Change-Id: I18a4829c2a22a6099cf3b0824ea1f698bfbf1917
Reviewed-on: https://chromium-review.googlesource.com/456707
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Franziska Hinkelmann <franzih@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44116}
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/bootstrapper.cc
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/builtins/builtins-typedarray-gen.cc
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/builtins/builtins.h
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/code-stub-assembler.cc
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/code-stub-assembler.h
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/contexts.h
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/js/typedarray.js
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/runtime/runtime-typedarray.cc
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/src/runtime/runtime.h
[add] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/test/mjsunit/es6/typedarray-construct-by-array-like.js
[modify] https://crrev.com/14e01da1cf6c02628252ed267ca5043f44d63a5d/test/mjsunit/es6/typedarray-construct-by-buffer-ordering.js

Project Member

Comment 19 by bugdroid1@chromium.org, Mar 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/160f1dc06af7e1673b0adf4c074a0e7b309711ec

commit 160f1dc06af7e1673b0adf4c074a0e7b309711ec
Author: Peter Marshall <petermarshall@chromium.org>
Date: Mon Mar 27 14:19:04 2017

[builtins] Delete unused ArrayIdToTypeAndSize and ArrayId.

These aren't used thanks to new implementation in CSA.

BUG=v8:5977

Change-Id: Ia4acfa0d1a925eba305a818913cbeff479b27792
Reviewed-on: https://chromium-review.googlesource.com/458477
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44151}
[modify] https://crrev.com/160f1dc06af7e1673b0adf4c074a0e7b309711ec/src/runtime/runtime-typedarray.cc
[modify] https://crrev.com/160f1dc06af7e1673b0adf4c074a0e7b309711ec/src/runtime/runtime.h

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1c8760c6b5a113478491b99b83d675a8be5ca980

commit 1c8760c6b5a113478491b99b83d675a8be5ca980
Author: Peter Marshall <petermarshall@chromium.org>
Date: Wed Mar 29 13:02:51 2017

[Tests] Add a test for constructing a TypedArray from the same kind.

This should be the fastest case, as we can just copy the backing store
directly. Adding this test so that we can monitor if upcoming changes
regress this path.

BUG=v8:5977

Change-Id: I021a199061ac845f265a906bda68b7ad3e8d5708
Reviewed-on: https://chromium-review.googlesource.com/461183
Reviewed-by: Franziska Hinkelmann <franzih@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44225}
[modify] https://crrev.com/1c8760c6b5a113478491b99b83d675a8be5ca980/test/js-perf-test/JSTests.json
[add] https://crrev.com/1c8760c6b5a113478491b99b83d675a8be5ca980/test/js-perf-test/TypedArrays/construct-same-typedarray.js

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a450c18544df40f7c7cbed206a7cdd1ed38853aa

commit a450c18544df40f7c7cbed206a7cdd1ed38853aa
Author: Peter Marshall <petermarshall@chromium.org>
Date: Fri Mar 31 10:37:57 2017

[builtins] Copy array contents using JS in ConstructByArrayLike.

The last CL https://chromium-review.googlesource.com/c/456707/ caused
some pretty heavy performance regressions. After experimenting, it
seems the easiest and most straight-forward way to copy the elements
into the new typed array is to do it in JS.

Adds a fast path for typed arrays, where the source typed array has
the same elements kind, in which case we can just copy the backing
store using memcpy.

This CL also removes regression test 319120 which is from a pwn2own
vulnerability. The old code path enforced a maximum byte_length
that was too low, which this change removes. The length property of
the typed array must be a Smi, but the byte_length, which can be up
to 8x larger than length for a Float64Array, can be a heap number.

We can also re-use some of the logic from ConstructByLength when
deciding whether to allocate the buffer on- or off-heap, so that
is factored out into InitializeBasedOnLength. We can also re-use
the DoInitialize helper instead of calling into the runtime,
meaning we can remove InitializeFromArrayLike.

BUG=v8:5977,chromium:705503, chromium:705394 

Change-Id: I63372652091d4bdf3a9491acef9b4e3ac793a755
Reviewed-on: https://chromium-review.googlesource.com/459621
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44301}
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/assembler.cc
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/assembler.h
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/builtins/builtins-typedarray-gen.cc
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/code-stub-assembler.cc
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/code-stub-assembler.h
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/contexts.h
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/external-reference-table.cc
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/js/typedarray.js
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/objects.h
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/runtime/runtime-typedarray.cc
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/src/runtime/runtime.h
[modify] https://crrev.com/a450c18544df40f7c7cbed206a7cdd1ed38853aa/test/mjsunit/es6/typedarray-construct-by-array-like.js
[delete] https://crrev.com/42f285fcbb6f2546ce83f8d2e444328f754517ef/test/mjsunit/regress/regress-319120.js

Project Member

Comment 22 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/143dcc6c412e63d3b339423c0e5cf5747173d4a6

commit 143dcc6c412e63d3b339423c0e5cf5747173d4a6
Author: Peter Marshall <petermarshall@chromium.org>
Date: Fri Mar 31 10:51:26 2017

[builtins] Skip iteration when constructing TypedArrays if possible.

This CL uses the same logic as spread calls to check whether the
iteration over an array would produce different results to simply
accessing the backing store directly. Skipping the full iteration
protocol for normal arrays gives us a ~10x speedup on the
construct-typedarray benchmark.

BUG=v8:5977, v8:5699 , v8:4782 , chromium:698173 

Change-Id: Ib878d39691e99b739afef0dd05a6a6efc5b6b5d4
Reviewed-on: https://chromium-review.googlesource.com/463367
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44304}
[modify] https://crrev.com/143dcc6c412e63d3b339423c0e5cf5747173d4a6/src/bootstrapper.cc
[modify] https://crrev.com/143dcc6c412e63d3b339423c0e5cf5747173d4a6/src/builtins/builtins-array.cc
[modify] https://crrev.com/143dcc6c412e63d3b339423c0e5cf5747173d4a6/src/builtins/builtins-definitions.h
[modify] https://crrev.com/143dcc6c412e63d3b339423c0e5cf5747173d4a6/src/contexts.h
[modify] https://crrev.com/143dcc6c412e63d3b339423c0e5cf5747173d4a6/src/js/typedarray.js

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e28f7fc90daf5b2b28e40fa6239e8a8e766b96e6

commit e28f7fc90daf5b2b28e40fa6239e8a8e766b96e6
Author: Peter Marshall <petermarshall@chromium.org>
Date: Thu Apr 06 14:56:07 2017

[builtins] Don't clear buffer memory that will be overwritten.

Currently we initialize the allocated buffer to be full of 0s, which
adds significant overhead.

TypedArrayConstructByArrayLike will always either fully initialize the
buffer, or throw an exception, in which case the buffer will not be
leaked to user code.

The length of the new TypedArray (and thus the buffer) is derived from
the length of the source Array/TypedArray, so we know that we will
always set every byte of the new buffer, or throw trying.

Bug:v8:5977

Change-Id: I8ceaa883cfad85f8708a5bdaada3ce463d97e007
Reviewed-on: https://chromium-review.googlesource.com/469348
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44447}
[modify] https://crrev.com/e28f7fc90daf5b2b28e40fa6239e8a8e766b96e6/src/bootstrapper.cc
[modify] https://crrev.com/e28f7fc90daf5b2b28e40fa6239e8a8e766b96e6/src/builtins/builtins-arraybuffer.cc
[modify] https://crrev.com/e28f7fc90daf5b2b28e40fa6239e8a8e766b96e6/src/builtins/builtins-definitions.h
[modify] https://crrev.com/e28f7fc90daf5b2b28e40fa6239e8a8e766b96e6/src/builtins/builtins-typedarray-gen.cc
[modify] https://crrev.com/e28f7fc90daf5b2b28e40fa6239e8a8e766b96e6/src/contexts.h

Project Member

Comment 24 by bugdroid1@chromium.org, Apr 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9679a3661f4e3a3dd40bf9fdf6ab6a3a6883333d

commit 9679a3661f4e3a3dd40bf9fdf6ab6a3a6883333d
Author: Peter Marshall <petermarshall@chromium.org>
Date: Tue Apr 11 13:46:10 2017

[test] Add a benchmark for constructing all types of TypedArrays.

This constructs different typed arrays from different types of other
typed arrays, hopefully countering microbenchmarks which are able to
optimize for exactly one pair of types.

Bug: v8:5977
Change-Id: Ie3b07d6ecaaca6db0be410e902e437a2a643d71c
Reviewed-on: https://chromium-review.googlesource.com/474748
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44576}
[modify] https://crrev.com/9679a3661f4e3a3dd40bf9fdf6ab6a3a6883333d/test/js-perf-test/JSTests.json
[add] https://crrev.com/9679a3661f4e3a3dd40bf9fdf6ab6a3a6883333d/test/js-perf-test/TypedArrays/construct-all-typedarrays.js

Blocking: chromium:698746
Labels: ReleaseBlock-Stable
Moving this issue to Chromium to feed it into the releaseblock machinery
Project: chromium
Moved issue v8:5977 to now be  issue chromium:711275 .
Components:
Labels: -Priority-1 M-59 Pri-1
Peter, please make sure that the necessary changes are merged to 5.9.
Project Member

Comment 30 by bugdroid1@chromium.org, Apr 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c326e73d91df3c8c84ff6f25bffe4bef750c94b1

commit c326e73d91df3c8c84ff6f25bffe4bef750c94b1
Author: Peter Marshall <petermarshall@chromium.org>
Date: Tue Apr 25 12:42:06 2017

[builtins] Cleanup TypedArray constructors and reduce code size.

This CL is purely refactoring, no behavior changes.

Remove InitializeBasedOnLength and combine it with a new Stub-ified
TypedArrayInitialize which now allocates the buffer in both the
on-heap and off-heap cases.

Add TypedArrayInitializeWithBuffer because this was essentially a
special case that didn't share much logic with Initialize.
Factor out the common pieces into SetupTypedArray and AttachBuffer.

We can also always pass in the elementsSize, so there is no need
to calculate this again. LoadMapAndElementsSize is changed to 
LoadMapForType.

This reduces code size by ~8k.

Bug:  chromium:711275 , chromium:701768 
Change-Id: I6ad8701e9c72f53bfd9484725fb82055be568c25
Reviewed-on: https://chromium-review.googlesource.com/483481
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44850}
[modify] https://crrev.com/c326e73d91df3c8c84ff6f25bffe4bef750c94b1/src/bootstrapper.cc
[modify] https://crrev.com/c326e73d91df3c8c84ff6f25bffe4bef750c94b1/src/builtins/builtins-definitions.h
[modify] https://crrev.com/c326e73d91df3c8c84ff6f25bffe4bef750c94b1/src/builtins/builtins-typedarray-gen.cc
[modify] https://crrev.com/c326e73d91df3c8c84ff6f25bffe4bef750c94b1/src/code-stub-assembler.cc
[modify] https://crrev.com/c326e73d91df3c8c84ff6f25bffe4bef750c94b1/src/code-stub-assembler.h
[modify] https://crrev.com/c326e73d91df3c8c84ff6f25bffe4bef750c94b1/src/contexts.h

Project Member

Comment 31 by bugdroid1@chromium.org, Apr 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e855e514d111261a185f9de98add42c224d7bcbe

commit e855e514d111261a185f9de98add42c224d7bcbe
Author: Peter Marshall <petermarshall@chromium.org>
Date: Wed Apr 26 15:36:36 2017

[builtins] Add a fast path to construct TypedArrays from holey arrays.

For holey Smi and double source arrays, we would go to the general
case, which is much slower than before. We already check that there
are no prototype chain changes in IterableToListCanBeElided, and
there is no JS-code run between that check and the copying of the
elements, so we can safely check for the hole and convert it to
undefined, which is then converted to 0/NaN appropriately for the
given TypedArray.

Bug:  chromium:713570 , chromium:711275 
Change-Id: I5b21c915907d71eebb73b7b1eea8eb58b4a5436d
Reviewed-on: https://chromium-review.googlesource.com/485520
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44899}
[modify] https://crrev.com/e855e514d111261a185f9de98add42c224d7bcbe/src/elements.cc
[add] https://crrev.com/e855e514d111261a185f9de98add42c224d7bcbe/test/mjsunit/es6/typedarray-construct-by-array-like-prototype-element-added.js
[modify] https://crrev.com/e855e514d111261a185f9de98add42c224d7bcbe/test/mjsunit/es6/typedarray-construct-by-array-like.js

Reminder that M59 Stable is launch is coming soon (less than 2 weeks)! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!
Reminder that M59 Stable is launch is coming soon (less than 2 weeks)! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!
Labels: Merge-Request-59
I worked on a series of CLs for TypedArray constructors, and the branch-cut happened before this series was completely finished. There are 7 CLs in this project that have landed in 60, but didn't make the 59 branch point.

Pros of merging them:
1. The performance of TypedArray constructors in 59 without them will be worse than in 58. We have seen several bugs logged with us about performance in the past, because it shows up often for 3d graphics where TypedArrays are used as vectors. If we merge the rest of the CLs to 59, the performance will be even better than 58.
2. The CLs aren't wide-reaching and should apply cleanly.
3. All of the CLs have made it to Dev in 60, so we are fairly confident about stability.

Cons of merging them:
1. There are 7 CLs. This is a lot.
2. We won't have a lot of baking time on beta. (partially offset by how long they have been on 60 canary/dev).
3. These affect performance-only, they don't deal with bugfixes.

https://chromium.googlesource.com/v8/v8/+/356e9246b2f9653f1ef3bf47b3e801391b1824c3
https://chromium.googlesource.com/v8/v8/+/c326e73d91df3c8c84ff6f25bffe4bef750c94b1
https://chromium.googlesource.com/v8/v8/+/e855e514d111261a185f9de98add42c224d7bcbe
https://chromium.googlesource.com/v8/v8/+/c1699fdeefbd290dc068d097a9c5a420c4bb8978
https://chromium.googlesource.com/v8/v8/+/74aa7ff308e64266ce72b04f4dfac160d44b90da
https://chromium.googlesource.com/v8/v8/+/0d582ada5135b4070a9e154e373deb5d6c19f380
https://chromium.googlesource.com/v8/v8/+/4d611d1dc322313cf10be4bde5158627ba944814

Project Member

Comment 35 by sheriffbot@chromium.org, May 19 2017

Labels: -Merge-Request-59 Merge-Review-59 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I'd be in favor of back-merging those CLs. 
Cc: hablich@chromium.org
+hablich for V8 review. 

Labels: -Performance Performance-Responsiveness
@abdulsyed: Unfortunately hablich is OOO until July
Cc: adamk@chromium.org eisinger@chromium.org
+ eisinger@, danno@, adamk@ (based on hablich@ OOO message). 
Can you please review this merge request for V8. 
Cc: -eisinger@chromium.org jochen@chromium.org
The last CL looks like it's actually a bug fix?

Would those changes actually apply cleanly on M59 at this point, and pass tests?
The last CL is a bugfix, for a bug introduced in the first CL.
I think we might get merge conflicts in the big macro-lists e.g. in runtime.h, contexts.h. I think the main content would apply cleanly.
jochen@ - please let us know if you approve this merge. 
Peter, how bad is the performance regression? If it's only small or moderate, I think we shouldn't merge but wait.
I think at this point we should wait. I would call it 'moderate'
Labels: -Merge-Review-59 Merge-Rejected-59
Labels: -ReleaseBlock-Stable
Status: Fixed (was: Started)

Sign in to add a comment