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

Issue 692936 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Email to this user bounced
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

43.8% regression in thread_times.simple_mobile_sites at 450433:450486

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

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=692936

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguJ7EsgsM


Bot(s) for this bug's original alert(s):

android-one
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Feb 16 2017

Cc: msar...@google.com
Owner: msar...@google.com

=== Auto-CCing suspected CL author msarett@google.com ===

Hi msarett@google.com, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Matt Sarett
  Commit : 8572d853514e3c73077540341edbf62a3f486605
  Date   : Tue Feb 14 17:50:52 2017
  Subject: Make raster pipeline support all pixel conversions

Bisect Details
  Configuration: android_one_perf_bisect
  Benchmark    : thread_times.simple_mobile_sites
  Metric       : thread_raster_cpu_time_per_frame/thread_raster_cpu_time_per_frame
  Change       : 26.02% | 2.82371286042 -> 3.55840503614

Revision                             Result                   N
chromium@450432                      2.82371 +- 0.398376      6      good
chromium@450459                      2.59365 +- 0.347381      6      good
chromium@450466                      2.79629 +- 0.636106      6      good
chromium@450468                      2.67491 +- 0.16489       6      good
chromium@450469                      2.73051 +- 0.19237       6      good
chromium@450469,skia@4bf560a056      2.72784 +- 0.141283      6      good
chromium@450469,skia@8572d85351      3.76582 +- 0.226714      6      bad       <--
chromium@450470                      3.84477 +- 0.194886      6      bad
chromium@450473                      3.61504 +- 0.549741      6      bad
chromium@450486                      3.55841 +- 0.493629      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests thread_times.simple_mobile_sites

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8987514951631174160

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=6131034594213888


| 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 component Speed>Bisection.  Thank you!

Comment 4 by msar...@google.com, Feb 16 2017

Cc: mtklein@chromium.org
Project Member

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

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/973ca80f078de0d55808d15f95cbfc13281a735c

commit 973ca80f078de0d55808d15f95cbfc13281a735c
Author: Matt Sarett <msarett@google.com>
Date: Wed Feb 22 00:29:27 2017

SkConvertPixels: Add Alpha8 fast path

BUG:692936

Change-Id: I8394554764b1f46bd8eaabb0194d52f361d477c1
Reviewed-on: https://skia-review.googlesource.com/8826
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>

[modify] https://crrev.com/973ca80f078de0d55808d15f95cbfc13281a735c/src/core/SkConvertPixels.cpp

Comment 6 by msar...@google.com, Feb 24 2017

alexclarke@ do you know if there is a trybot that will run these tests?  I'm having a lot of trouble getting them to run locally.

Also, the fact that #5 didn't help makes it look like the websites are using must be using ARGB_4444.

Seems really strange to me... maybe it's something we do on low end devices.  I'm not sure that this regression is something that we'll want to fix (do we care about 4444).  Would be interesting to know if we saw the same issue on a more recent phone.

Comment 7 by msar...@google.com, Mar 2 2017

Status: WontFix (was: Untriaged)
Looking closer at the test history, this only affects android_one (which must be using 4444) - all other devices are fine.  The old 4444 code path doesn't make much sense now that we've added color space support - the new code handles more cases and is WAI.

Sign in to add a comment