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

Issue 602300 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

8.2%-79% regression in smoothness.gpu_rasterization.tough_filters_cases at 385941:386016

Project Member Reported by rsch...@chromium.org, Apr 11 2016

Issue description

See the link to graphs below.
 
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Apr 12 2016

Cc: robertph...@google.com
Owner: robertph...@google.com

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

Hi robertphillips@google.com, 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 : Revert conversion of SkColorFilterImageFilter to new API
Author  : robertphillips
Commit description:
  
BUG=598028

GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1864263002

Review URL: https://codereview.chromium.org/1864263002
Commit  : 6f01104b1d86b861b366b3511c8f2567eca4a9f3
Date    : Thu Apr 07 15:10:45 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@385981         48.667434   6.389848    6           good
chromium@385984         48.365516   3.639209    5           good
chromium@385984,skia@303c979b8250.05481    1.10217     5           good
chromium@385984,skia@6f01104b1d83.298188   5.237488    5           bad         <-
chromium@385984,skia@8cd4a2423678.501023   2.51127     5           bad
chromium@385984,skia@8af936d30483.915055   4.088565    5           bad
chromium@385985         81.510066   2.276261    4           bad
chromium@385986         82.301814   1.074785    5           bad
chromium@385987         86.238682   1.954599    6           bad

Bisect job ran on: mac_hdd_perf_bisect
Bug ID: 602300

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --also-run-disabled-tests smoothness.gpu_rasterization.tough_filters_cases
Test Metric: frame_times/http___static.bobdo.net_Analog_Clock.svg
Relative Change: 71.75%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_hdd_perf_bisect/builds/476
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9015662450974144992


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=602300

| 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 Tests>AutoBisect.  Thank you!
Cc: ashej...@chromium.org
Labels: TE-Triaged
@robertphillips: Hey, would you mind checking the above issue as per comment#2 ?

I really appreciate your help.

Thank you!
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 19 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/718a5adc6da857f08578cae434bcf81ea3f5aa3d

commit 718a5adc6da857f08578cae434bcf81ea3f5aa3d
Author: robertphillips <robertphillips@google.com>
Date: Tue Apr 19 17:21:02 2016

Switch SkColorFilterImageFilter over to new onFilterImage interface (again)

Back when this was originally reverted I was able to easily repro the perf regression locally. At ToT Skia/Chrome I can no longer repro the perf regression with this CL (in fact there is a modest perf improvement).

I propose landing this and then watching the Chromium perf bots.

BUG= 602300 ,598028
TBR=reed@google.com

GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1901513002

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

[modify] https://crrev.com/718a5adc6da857f08578cae434bcf81ea3f5aa3d/include/effects/SkColorFilterImageFilter.h
[modify] https://crrev.com/718a5adc6da857f08578cae434bcf81ea3f5aa3d/src/effects/SkColorFilterImageFilter.cpp


----------------

https://codereview.chromium.org/1782083002 (Switch SkColorFilterImageFilter over to new onFilterImage interface) landed in Chrome at 383061

This CL caused a GPU perf regression on Win7, Win8, linux & N5 on the letmespellitoutforyou.com tough_filters_case 
It also caused a raster perf regression on Win7, Win10, mac-hdd, linux, A1, etc also on letmespellitoutforyou.com tough_filters_case

https://bugs.chromium.org/p/chromium/issues/detail?id=598028 (25.4%-53.5% regression in smoothness.gpu_rasterization.tough_filters_cases at 383055:383086) was filed to track this regression.

----------------

This CL was reverted in https://codereview.chromium.org/1864263002 (Revert conversion of SkColorFilterImageFilter to new API) and landed in Chrome at 385985

The revert CL caused a GPU-only perf regression on mac-hdd,N6,A1 & N5X on the Analog_Clock.svg tough_filters_case

https://bugs.chromium.org/p/chromium/issues/detail?id=602300 (.2%-79% regression in smoothness.gpu_rasterization.tough_filters_cases at 385941:386016) was filed to track this regression

The four examples in this bug are just losing the perf gains brought about by the initial CL (and then some). Presumably these will be restored when the original CL is relanded.

----------------

In between these two events the CL https://codereview.chromium.org/1840303006 (Use Blink's computed filterPrimitiveSubregion() as CropRect) landed at 385031 which tightened the clipRect on the Blink-side. This lessened the perf regresssion of the original CL.

----------------

The original CL has been relanded in https://codereview.chromium.org/1901513002 (Switch SkColorFilterImageFilter over to new onFilterImage interface (again)) which has yet to land in Chrome.

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/246cf1d1431272fb94349e63adeecb6b0c4fb433

commit 246cf1d1431272fb94349e63adeecb6b0c4fb433
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Tue Apr 19 23:56:54 2016

Roll src/third_party/skia/ be991efbd..244a65350 (16 commits).

https://chromium.googlesource.com/skia.git/+log/be991efbd9fe..244a65350e52

$ git log be991efbd..244a65350 --date=short --no-merges --format='%ad %ae %s'
2016-04-19 mtklein skcpu: sse4.1 floor, f16c f16<->f32
2016-04-19 reed add gm to exercise rotated imagefiltesr w/ and w/o antialiasing
2016-04-19 mtklein Move CPU feature detection to its own file.
2016-04-19 brianosman Adding support for controlling the global sRGB SkColor switch.
2016-04-19 mtklein Remove static initializer for SkOpts::Init()
2016-04-19 herb Add onImageInfo call to SkImage_Base.
2016-04-19 egdaniel Fix createBuffer in Vulkan
2016-04-19 robertphillips Switch SkColorFilterImageFilter over to new onFilterImage interface (again)
2016-04-19 brianosman Adding support for playback to L32/S32/F16 canvas.
2016-04-19 cdalton Add optional data parameter to createBuffer
2016-04-19 robertphillips Make skpinfo more human friendly GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1901713004
2016-04-19 brianosman Move DM png code to picture_utils, for use by other tools.
2016-04-19 halcanary fiddle_build_test/gyp: fix mac mesa build (2)
2016-04-19 robertphillips Add more diagnostic messages to ReadWriteAlpha test
2016-04-19 scroggo Rename CodexTest.cpp to CodecTest.cpp
2016-04-19 halcanary fiddle_build_test/gyp: fix mac mesa build

BUG= 602300 ,598028

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=fmalita@google.com

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

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

[modify] https://crrev.com/246cf1d1431272fb94349e63adeecb6b0c4fb433/DEPS

Status: Fixed (was: Assigned)
I believe we have regained as much of the performance from the original patch as we're going to get.

Sign in to add a comment