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

Issue 786686 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue v8:3590
issue v8:5925
issue 759236



Sign in to add a comment

Performance regression in TypedArray.set in chrome 62.0.3202.94

Reported by igor.mir...@plarium.com, Nov 18 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36

Steps to reproduce the problem:
1. Start some http server
1. Open test webgl project from attached file in two different browsers: 62.0.3202.94, 61.0.3163.10 
2. Press go button
3. Look at the results in browser console  
4. Google_Chrome_64bit_v62.0.3202.94 - 6983mс
   Google_Chrome_64bit_v61.0.3163.10 - 1672mс

What is the expected behavior?
The same result in new browser as in older version of chrome

What went wrong?
There is a problem with our Unity3D WebGl project in new chrome(62.0.3202.94). The problem is the speed of Resources.Load Unity3D function, speed of loading unity resources increased  by 5 times.I submitted the request to unity3d support, they said that is the broblem of browser. 

Did this work before? Yes all versions before 62.0.3202.94

Chrome version: 62.0.3202.94  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version:
 
template.rar
4.1 MB Download

Comment 1 by junov@chromium.org, Nov 20 2017

Components: -Blink Blink>JavaScript
Status: Untriaged (was: Unconfirmed)
Summary: Performance regression in TypedArrayBuffer.set in chrome 62.0.3202.94 (was: webgl bug in chrome 62.0.3202.94)
I was able to reproduce.  I did a timeline capture in Chrome devtools. It revealed that TypedArrayBuffer.set is where all the CPU time is being spent.

Renaming issue: bug is unrelated to WebGL. It is a JavaScript related regression.
Labels: Needs-Bisect Needs-Triage-M62

Comment 3 by kbr@chromium.org, Nov 21 2017

Cc: bmeu...@chromium.org mstarzinger@chromium.org jkummerow@chromium.org
I think this has been reported before – possibly in the V8 issue tracker – but can't find the bug. V8 folks, can one of you help find it?

Comment 4 by kbr@chromium.org, Nov 21 2017

Cc: kbr@chromium.org

Comment 5 by kbr@chromium.org, Nov 21 2017

Labels: -Pri-2 Pri-1
Increasing priority to P1 because this is a huge performance regression for a client's app.

Cc: jgruber@chromium.org
Cc: vamshi.k...@techmahindra.com
Labels: Triaged-ET Needs-Feedback
"Tried to reproduce the issue on the reported chrome version stable 62.0.3202.94 using windows 10 with the below mwntioned steps.
1. Launched Chrome.
2. started http server.
3. Opened the attached file.
4. Pressed ""go"" after its loaded.
5. Opened console in dev tools.
We searched for 'the parameter 'mc' mentioned in comment#0 to know the performance, but the console page didn't show any such. Attaching the screen cast of the same.

@Reporter: Could you please check the screnn cast and let us know whether we have missed any steps to reproduce the issue.

Thanks!"
786686.mp4
1.8 MB View Download
Parameter "LOAD",it's a parameter from fourth step
image.png
452 KB View Download

Comment 9 by kbr@chromium.org, Nov 21 2017

Blocking: v8:5925 759236 v8:3590
Cc: fran...@chromium.org
Dug up the related bugs.

There seems to have been a significant performance regression in Chrome 62 and this should be investigated ASAP.

Comment 10 by kbr@chromium.org, Nov 21 2017

Summary: Performance regression in TypedArray.set in chrome 62.0.3202.94 (was: Performance regression in TypedArrayBuffer.set in chrome 62.0.3202.94)
Cc: -jgruber@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Runtime
Labels: Performance OS-Linux OS-Mac
Owner: jgruber@chromium.org
Status: Assigned (was: Untriaged)
I'm sorry for the trouble, this has been my fault. We're working on repairing this regression for the 64 release.
Another related bug: http://b/69608273
Labels: M-62
Cc: hablich@chromium.org
Labels: Merge-Review-63
The fix or a mitigation should land in 63. Let's decide what to do exactly after the fix landed.
Clarification: initial fix should land asap and than we need to mitigate/fix the situation for >=63

Comment 17 by cmasso@google.com, Nov 22 2017

Please land the fix before requesting merge review and make sure to add this bug ID in the CL.

Comment 18 by cmasso@google.com, Nov 27 2017

ping!
Sorry for the delay. We've been working on this, but I forgot to include this issue # on CLs (they are all tracked at  https://crbug.com/v8/3590 ).

Just ran the benchmark attached in #0. All x64 Linux release builds, lower is better:

61.0.3163.11: 1556 LOAD
62.0.3202.94: 4392 LOAD
64.0.3280.0:   926 LOAD

From these numbers it looks like the regression has recovered and we've actually improved over 61 by >1.5x.

The CLs that need to be backmerged are, in order:

https://chromium-review.googlesource.com/786414
https://chromium-review.googlesource.com/789010
https://chromium-review.googlesource.com/788857

They're all on the current canary 64.0.3279.0.
Labels: -Merge-Review-63 Merge-Approved-63
Please set to Fixed.

Merges are big but important. Canary looks ok. Please merge this to 63 today.
Status: Fixed (was: Assigned)
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 28 2017

Labels: merge-merged-6.3
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

Labels: -Merge-Approved-63

Sign in to add a comment