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

Issue 592007 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

44.5%-69% regression in blink_perf.bindings at 378997:379010

Project Member Reported by lanwei@google.com, Mar 4 2016

Issue description

See the link to graphs below.
 
Cc: littledan@chromium.org
Owner: littledan@chromium.org

=== Auto-CCing suspected CL author littledan@chromium.org ===

Hi littledan@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Implement TypedArray(typedarray) constructor
Author  : littledan
Commit description:
  
The ES2016 draft spec defines a sort of fast path for constructing
a TypedArray based on another TypedArray. This patch implements that
alternative path in TypedArray construction. It is verified by
test262 tests, which now pass. This patch also has a slight cleanup
of TypedArray code by using a macro for TypedArray type checks, as
is done for other types.

This patch includes a minor spec violation: In the same-type case, the
spec indicates that the underlying ArrayBuffer should be copied until
the end, and this is fixed up by making the [[ArrayLength]] shorter.
This is observable with the buffer getter. This patch just copies the
used part of the underlying ArrayBuffer.

R=adamk
BUG= v8:4726 
LOG=Y

Review URL: https://codereview.chromium.org/1754593003

Cr-Commit-Position: refs/heads/master@{#34443}
Commit  : 2fa1c88442d83865f611efb890642d1eb643f320
Date    : Wed Mar 02 18:06:29 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@379003         311.303656  3.027309    5           good
chromium@379003,v8@b7a4351404310.499517  3.42541     5           good
chromium@379003,v8@56eca6d315311.788766  1.622158    5           good
chromium@379003,v8@2d090b11d0310.489075  2.586841    5           good
chromium@379003,v8@2fa1c88442122.001943  0.392664    5           bad
chromium@379003,v8@017375f328121.436855  0.710167    5           bad
chromium@379004         136.510823  30.685762   5           bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 592007

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests blink_perf.bindings
Test Metric: typed-array-construct-from-same-type/typed-array-construct-from-same-type
Relative Change: 56.15%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3469
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9019106562529293920


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with label Cr-Tests-AutoBisect.  Thank you!
Definitely looks like my patch caused the issue--the tests that got slower are getting at exactly what the patch changed. I'm working on reproducing locally.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 8 2016

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

commit ab6e48de4815a03cb6523fbce209fd6fb6df9696
Author: littledan <littledan@chromium.org>
Date: Tue Mar 08 20:08:24 2016

Optimize new TypedArray(typedArray) constructor

A previous spec compliance fix for TypedArrays caused a ~4x performance
regression. This patch removes the regression by calling out
to a path within the runtime which implements array copying more
efficiently.

BUG= chromium:592007 
R=adamk
LOG=Y

Review URL: https://codereview.chromium.org/1767893002

Cr-Commit-Position: refs/heads/master@{#34601}

[modify] https://crrev.com/ab6e48de4815a03cb6523fbce209fd6fb6df9696/src/js/typedarray.js

Status: Fixed (was: Assigned)

Sign in to add a comment