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

Issue 698173 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
OOO until 2019-02-10
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Dramatic slow-down of Float32Array

Reported by defor...@gmail.com, Mar 3 2017

Issue description

Steps to reproduce the problem:
1. On Galaxy S6, goto https://3dthis.com/bugs/float32.htm 

What is the expected behavior?
Displayed time should be around 12ms

What went wrong?
Displayed time is around 90ms

Did this work before? Yes Release 56.0.2924.87

Chrome version: 57.0.2987.74  Channel: beta
OS Version: 6.0.1
Flash Version: 

Can be easily fixed by avoiding repetitive creation of Float32Array from Array, but it may impact existing sites using WebGL with large vertices arrays.

 

Comment 1 by defor...@gmail.com, Mar 3 2017

I also get the issue on PC :
Chrome release : around 4ms
Chrome dev 58.0.3026.3 : around 30ms

Components: -Blink Blink>JavaScript
Labels: Needs-Bisect Needs-Triage-M56
Cc: hdodda@chromium.org
Labels: -Pri-2 -Needs-Bisect -Needs-Triage-M56 hasbisect-per-revision M-58 OS-Linux OS-Mac OS-Windows Pri-1
Owner: bmeu...@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build:57.0.2743.0(Revision: 436483).
Bad build: 57.0.2745.0 (Revision:437133).

You are probably looking for a change made after 436601 (known good), but no later than 436602 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspectas some perf builds might get missing due to failure.
 
https://chromium.googlesource.com/chromium/src/+log/799bbe860fd36a73c92f0bdf7e7f061e0cde54e3..c5c726dfd53776578e1523d51adfb5f7b2101590

From the CL's above, suspecting the below change and assigning the issue to the reviewer as owner is not found in the list

https://chromium.googlesource.com/v8/v8/+/7a6f294ffe8e9cc98e266238ec5cd0aa74524c4a

@bmeurer - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

https://chromium.googlesource.com/v8/v8/+/7a6f294ffe8e9cc98e266238ec5cd0aa74524c4a

Thanks!


Owner: petermarshall@chromium.org
This is due to a bugfix in https://codereview.chromium.org/2544503002. With the fix we take the more expensive path through the typed array constructor. I think it is probably possible to restore the optimization in the new typed array constructor once ported to the CSA, where we'd check whether to skip the full iteration on arrays, similar to what we do for spread calls already.

Comment 6 by hdodda@chromium.org, Mar 20 2017

Able to reproduce the issue on mac os 10.12.2 using chrome canary M59 #59.0.3046.0 .

@Could someone please take a look into this.

Thanks!
Status: Started (was: Assigned)
I have a CL in the works that skips the iteration protocol where there are no observable side effects, like we do for spread calls. It gives a nice boost for TypedArray creation from arrays.
Project Member

Comment 8 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

Status: Fixed (was: Started)
This is much faster now as of 59.0.3061.0 where "Skip iteration when constructing TypedArrays if possible." landed. It also benefits from the follow-up CL "Add a fast path to construct TypedArrays from holey arrays." which landed in 60.0.3086.0 because the array passed to the TypedArray constructor is a holey smi array, because it is pre-allocated and then filled, so it needs that fast-path.
e.g. The benchmark went from 45 -> 25 -> 4ms on my Macbook with those CLs

Sign in to add a comment