Bisect info: 494809 (good) - 494835 (bad)
https://chromium.googlesource.com/chromium/src/+log/71f48559..3d5fa21f?pretty=fuller
Suspecting r494821 "Update V8 to version 6.2.256"
Landed in 62.0.3188.0

In V8 log https://chromium.googlesource.com/v8/v8/+log/2375796a..2f124c83
the only possible CL is a50b67519ab3023d87ca03fad9375c78dc767cb2
"[runtime] Port TypedArraySetFormArrayLike to C++"

Attaching a simplified test.html:
Expected: each test completes almost instantaneously, for example:
	array size 10, repetitions 1000000 completed in 58 ms
	array size 100, repetitions 100000 completed in 29 ms
	array size 1000, repetitions 10000 completed in 23 ms
	array size 10000, repetitions 1000 completed in 23 ms
	array size 100000, repetitions 100 completed in 23 ms
Observed: each test takes half a second or more, for example:
	array size 10, repetitions 1000000 completed in 832 ms
	array size 100, repetitions 100000 completed in 788 ms
	array size 1000, repetitions 10000 completed in 674 ms
	array size 10000, repetitions 1000 completed in 676 ms
	array size 100000, repetitions 100 completed in 708 ms

Deleted: test.html 793 bytes

test.html 793 bytes View Download

This is marked as 'RB-Stable' for M62. So just wanted to check do we have further update on fix?

Thank you!

franzih@ Ping! This issue is marked as RB-Stable for M-62, could you please let us know is there any latest update available on this issue?

Thanks!

That regression on that particular micro benchmark is fully expected. We ported the self hosted JS to builtin code. The self hosted code is only that fast if you run nothing but the microbenchark. As soon as there's another TypedArray.set *anywhere* else in the codebase, you lose the performance. We don't see regression in real world code. 

Removing 'RB-Stable' as per c#6. Please feel free to update if required.

Hello, can you clarify comment 6 ? The regression was very noticeable in our application.

though TypedArray.set is ported to C++, it is using GetElement and SetElement which are expensive operations.

CL: https://chromium-review.googlesource.com/c/v8/v8/+/720661

this is a testcase for overlapped typedarrays which have the different path with array-like cases:
```
function test(size, repetitions) {
  var ab = new ArrayBuffer(4*size);
  var source = new Float64Array(ab, 8);
  var target = new Float32Array(ab);
  var t0 = performance.now();
  for (var i = 0 ; i < repetitions; ++i) {
    target.set(source, 1);
  }
  console.log('array size ' + size + ', repetitions ' + repetitions + ' completed in ' + (performance.now() - t0 |0) + ' ms');
}

test(1e1, 1e6);
test(1e2, 1e5);
test(1e3, 1e4);
test(1e4, 1e3);
test(1e5, 1e2);
```

Before commit 59de35b290ffdd927782bb70fd36fefd3eaee245:
array size 10, repetitions 1000000 completed in 237 ms
array size 100, repetitions 100000 completed in 35 ms
array size 1000, repetitions 10000 completed in 16 ms
array size 10000, repetitions 1000 completed in 14 ms
array size 100000, repetitions 100 completed in 14 ms

with 59de35b290ffdd927782bb70fd36fefd3eaee245:
array size 10, repetitions 1000000 completed in 413 ms
array size 100, repetitions 100000 completed in 340 ms
array size 1000, repetitions 10000 completed in 302 ms
array size 10000, repetitions 1000 completed in 307 ms
array size 100000, repetitions 100 completed in 308 ms

with my fix:
array size 10, repetitions 1000000 completed in 165 ms
array size 100, repetitions 100000 completed in 80 ms
array size 1000, repetitions 10000 completed in 74 ms
array size 10000, repetitions 1000 completed in 68 ms
array size 100000, repetitions 100 completed in 73 ms

replacing GetElement and SetElement with Get and Set of ElementsAccessor significantly reduce the overhead for overlapped typedarrays, but still it's slower than the original js implementation. not sure why. maybe we can prevent to create redundant handles.


For array-like,

Before:
array size 10, repetitions 1000000 completed in 862 ms
array size 100, repetitions 100000 completed in 597 ms
array size 1000, repetitions 10000 completed in 581 ms
array size 10000, repetitions 1000 completed in 592 ms
array size 100000, repetitions 100 completed in 613 ms

with my fix:
array size 10, repetitions 1000000 completed in 506 ms
array size 100, repetitions 100000 completed in 332 ms
array size 1000, repetitions 10000 completed in 279 ms
array size 10000, repetitions 1000 completed in 262 ms
array size 100000, repetitions 100 completed in 288 ms

it's is still slow. maybe GetElement of Object is slow..?

accessing elements of array also can be optimized, which is already implemented in elements.cc. When I use CopyElements in elements.cc, no more overheads for array-like.

array size 10, repetitions 1000000 completed in 155 ms
array size 100, repetitions 100000 completed in 31 ms
array size 1000, repetitions 10000 completed in 19 ms
array size 10000, repetitions 1000 completed in 18 ms
array size 100000, repetitions 100 completed in 18 ms

Maybe i need to clarify something : I focus only on the first test case (Float32Array.set). the second test case (named bypass) is just here for reference. The first test case has regressed a lot between chrome 61 an chrome 63. 

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

commit 501127995efd9e8ff4be11e2e6927a3e10597fd1
Author: Choongwoo Han <cwhan.tunz@gmail.com>
Date: Thu Oct 19 08:25:52 2017

[typedarrays] Reduce overheads of TA.p.set

Replace GetElement and SetElement to Get and Set, and use CopyElements,
which reduces 4x-13x overheads.

Bug:  chromium:768775 
Cq-Include-Trybots: master.tryserver.v8:v8_linux_noi18n_rel_ng
Change-Id: I58534b30c2035195c5f4b8f2c04e7c459bdbebaa
Reviewed-on: https://chromium-review.googlesource.com/720661
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48723}
[modify] https://crrev.com/501127995efd9e8ff4be11e2e6927a3e10597fd1/src/builtins/builtins-typedarray.cc
[modify] https://crrev.com/501127995efd9e8ff4be11e2e6927a3e10597fd1/src/elements.cc
[modify] https://crrev.com/501127995efd9e8ff4be11e2e6927a3e10597fd1/src/elements.h
[modify] https://crrev.com/501127995efd9e8ff4be11e2e6927a3e10597fd1/test/mjsunit/es6/typedarray.js
[modify] https://crrev.com/501127995efd9e8ff4be11e2e6927a3e10597fd1/test/test262/test262.status

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

commit f0db4d20aa5959a8cae2ed525a598f68eb331272
Author: Choongwoo Han <cwhan.tunz@gmail.com>
Date: Thu Oct 19 11:10:11 2017

[typedarrays] Check if the target is a typed array at TA.p.set entry

- Throw a TypeError exception if a given target argument is not a typed
array before converting a given offset argument to an integer.
- Add a testcase

Bug:  chromium:768775 
Change-Id: Id132a0f154fcf930f211922fcbef6c66f9d6f285
Reviewed-on: https://chromium-review.googlesource.com/728120
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48736}
[modify] https://crrev.com/f0db4d20aa5959a8cae2ed525a598f68eb331272/src/builtins/builtins-typedarray.cc
[modify] https://crrev.com/f0db4d20aa5959a8cae2ed525a598f68eb331272/test/mjsunit/es6/typedarray.js

I can second that for Uint8Array.set.

Test app (web assembly face tracking, camera necessary).
https://tastenkunst.s3.amazonaws.com/demos/brfv4/test/index.html

In Chrome 61: all was fine (running at full 30FPS).
In Chrome 62: Uint8Array.set takes over 55ms, reducing the app to 10 FPS.

The use case here is copying the image data from canvas to the emscripten buffer.

Hope that helps.

Switching from 

_lib["HEAPU8"].set(imageData, dataPtr);

to

var buf = _lib["HEAPU8"]; // Chrome 62 performance hit workaround.

for(var k = 0, l = imageData.length, ind = dataPtr * _uint8Bytes; k < l; ind += _uint8Bytes, ++k) {

	buf[ind] = imageData[k];

}

brings it "back to normal". Please consider investigating the why

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/wtf/typed_arrays/ArrayBufferView.h?l=121

got so slow.

Thanks

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

commit 6241e81c35125aa3e95a0fc83cca6c0131e7695e
Author: Choongwoo Han <cwhan.tunz@gmail.com>
Date: Mon Oct 23 10:34:11 2017

[typedarrays] Fix a wrong type casting in TA.p.set

- Fix a wrong type casting triggered when a given array's length is zero
- Add a regression test case

Bug:  chromium:777182 ,  chromium:768775 
Change-Id: I615b73e9d7bad657c872c96c7a204efe355d8289
Reviewed-on: https://chromium-review.googlesource.com/732865
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48821}
[modify] https://crrev.com/6241e81c35125aa3e95a0fc83cca6c0131e7695e/src/elements.cc
[add] https://crrev.com/6241e81c35125aa3e95a0fc83cca6c0131e7695e/test/mjsunit/es6/regress/regress-777182.js

Thanks a lot. Another real app if needed (not our app, found it while
investigating WASM) :

https://d2jta7o2zej4pf.cloudfront.net/

Use emboss filter : chrome 59, set ~ 12%
chrome 62 ; set ~ 70%, JS beats WASM

I think we should merge it to 63.

+ benhenry@ to take his opinion on the merge. Pls note this issue exists on M62 Stable and M63 Beta.

+abdulsyed@ (M62 Desktop TPM) to keep him in loop.

There are three CLs, which one is up for merge? Also, I think it's safe to merge at this point as it actually does affect real sites.

Would be great to hear hablich@'s thoughts on merging.

Comment 13 and Comment 17 are up for merge. 

+adamk@ & danno@ as hablich@ is on vacation (ptal comments #25, #26 & #27).

Approved merge to M63 for 501127995efd9e8ff4be11e2e6927a3e10597fd1 and 6241e81c35125aa3e95a0fc83cca6c0131e7695e

Assigning to petermarshall (who reviewed these changes) if cwhan.tunz needs any help with merging.

Please merge your change M63 branch 3239 by 4:00 PM PT Monday (11/06/17) so we can take it for next week Beta release. Thank you.

I guess I don't have a permission to merge. @petermarshall, Could you merge this two commits?

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Thought I could take this on, but it's not a clean merge, so punting back to petermarshall.

So in my opinion we shouldn't merge this, just due to the sensitivity of this area of the codebase. It is really easy for security exploits to surface in typed arrays so the longer these fixes have in dev/beta and the longer clusterfuzz has to work on them, the better. #16 shows a temporary workaround. Plus, like Adam says the merge is not totally clean.

Marking as rejected per #c35.

Just to update: I'm currently working on backmerging these as part of  https://crbug.com/786686#c20 . The TypedArray.p.set regression is critical and needs to be fixed in 63.

CL in #13 is required by  https://crbug.com/786686#c20  due to its changes to ElementsAccessor::CopyElements. #17 fixes a bug in #13 and thus also needs to be included in the backmerge.

Please merge. We didn't get any negative feedback from Chromecrash or CF that this is not stable enough.

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

commit f126e6b4cd698e2916b53f705c446efc7b1a5de4
Author: jgruber <jgruber@chromium.org>
Date: Tue Nov 28 13:20:30 2017

Backmerge TypedArray.prototype.set performance improvements

This is a squashed backmerge of the following CLs:

[typedarrays] Reduce overheads of TA.p.set
https://chromium-review.googlesource.com/720661

[typedarrays] Fix a wrong type casting in TA.p.set
https://chromium-review.googlesource.com/732865

[builtins]: Simple port of %TypedArray%.prototype.set() to CSA TFJ.
https://chromium-review.googlesource.com/786414

[typedarray] Add set fast path for JSArray source arguments
https://chromium-review.googlesource.com/789010

[typedarray] Widen set fast path for JSTypedArray source arguments
https://chromium-review.googlesource.com/788857

LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Bug:  chromium:786686 , chromium:768775 , v8:3590 
Cq-Include-Trybots: master.tryserver.v8:v8_linux_noi18n_rel_ng
Change-Id: Ic1668a1e597c860f306a9e0fa4a73daab5565dba
Reviewed-on: https://chromium-review.googlesource.com/793042
Reviewed-by: Michael Hablich <hablich@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#91}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/assembler.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/assembler.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/builtins/builtins-definitions.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/builtins/builtins-typedarray-gen.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/builtins/builtins-typedarray.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/code-stub-assembler.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/code-stub-assembler.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/compiler/code-assembler.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/compiler/code-assembler.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/compiler/raw-machine-assembler.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/compiler/raw-machine-assembler.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/elements.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/elements.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/external-reference-table.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/isolate.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/isolate.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/messages.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/runtime/runtime-test.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/runtime/runtime-typedarray.cc
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/src/runtime/runtime.h
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/test/message/typedarray.out
[add] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/test/mjsunit/es6/regress/regress-777182.js
[add] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/test/mjsunit/es6/typedarray-set-bytelength-not-smi.js
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/test/mjsunit/es6/typedarray.js
[modify] https://crrev.com/f126e6b4cd698e2916b53f705c446efc7b1a5de4/test/test262/test262.status

Thanks to everyone involved in fixing this bug. 
Everything seems to be back to normal.
You are all doing a great job!

Cheers,
Marcel