New issue
Advanced search Search tips

Issue 734573 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

5.7% regression in smoothness.tough_filters_cases at 480160:480277

Project Member Reported by chiniforooshan@chromium.org, Jun 19 2017

Issue description

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

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


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

chromium-rel-win7-dual
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jun 19 2017

Cc: reed@google.com
Owner: reed@google.com

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

Hi reed@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 : Mike Reed
  Commit : 0c182fc77e044edddb6606b7cf51b9a5b6c2eb54
  Date   : Fri Jun 16 23:38:29 2017
  Subject: refactor lighting imagefilter to save codesize

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : smoothness.tough_filters_cases
  Metric       : frame_times/Filter Terrain SVG
  Change       : 5.63% | 34.2531238329 -> 36.1811945018

Revision                             Result                   N
chromium@480159                      34.2531 +- 1.57415       6      good
chromium@480218                      34.0065 +- 0.72763       6      good
chromium@480248                      34.4064 +- 0.505277      6      good
chromium@480263                      34.1679 +- 1.07538       6      good
chromium@480270                      33.5221 +- 0.524315      6      good
chromium@480271                      33.5249 +- 0.621528      6      good
chromium@480271,skia@0c182fc77e      36.0061 +- 1.20061       6      bad       <--
chromium@480271,skia@741d7261f2      35.6961 +- 0.238876      6      bad
chromium@480272                      36.0897 +- 0.91454       6      bad
chromium@480274                      35.9667 +- 1.07706       6      bad
chromium@480277                      36.1812 +- 0.751854      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=Filter.Terrain.SVG smoothness.tough_filters_cases

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

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


| 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!
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Jun 20 2017

Cc: perezju@chromium.org
 Issue 734782  has been merged into this issue.
Status: Assigned (was: Untriaged)
reed: any update on this regression?
Cc: senorblanco@chromium.org
reed: any update? adding smoothness.tough_filters_cases owner senorblanco to help understand priority of fixing this.
It looks like the CL was intended to reduce binary size. Mike, was a perf regression expected here? This is not a deal-breaker (lighting filters are not widely used), but it is unfortunate.

On a related note, there were some mammoth regressions in the filter tests later on for which the alerts were ignored. Is there a way to find out the reasoning for that?
Note that the regression is somewhat worse (10-12%) on Android. I imagine this is the cost of a virtual function call per-pixel. If we were to do a reduced test case with just lighting (this test has other filters), it might be worse.
Re #8:

> On a related note, there were some mammoth regressions in the filter tests later on for which the alerts were ignored. Is there a way to find out the reasoning for that?

Do you mean for example the one with chromium commit position range 482862-482921 in the following graph?

https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg7pOA-AoM

It could be that a perf sheriff marked it to be ignored after the graph went back down 9 days later; that is the usual scenario. I don't know if we can see when the alert was ignored in the backend, though. 
Re: #10:

Ah yes, I didn't see the recovery because I didn't drag the bars out to the right. Makes sense, thanks!
Status: WontFix (was: Assigned)
WontFixing since this bug is pretty narrowly scoped regression with no activity for several months.

Sign in to add a comment