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

Issue 735654 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

13%-26.1% regression in blink_perf.paint at 479939:479995

Project Member Reported by lanwei@chromium.org, Jun 21 2017

Issue description

See the link to graphs below.
 
Project Member

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

Cc: dmazz...@chromium.org
Owner: dmazz...@chromium.org

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

Hi dmazzoni@chromium.org, 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 : Dominic Mazzoni
  Commit : d5af4cd44c455022bd8fd747bf426f96c1a043e9
  Date   : Fri Jun 16 05:01:05 2017
  Subject: Add two image filtering modes to high contrast mode support.

Bisect Details
  Configuration: android_nexus7_perf_bisect
  Benchmark    : blink_perf.paint
  Metric       : large-table-background-change-with-invisible-collapsed-borders/large-table-background-change-with-invisible-collapsed-borders
  Change       : 30.42% | 745.145083333 -> 971.83275

Revision             Result                  N
chromium@479940      745.145 +- 11.1748      6      good
chromium@479947      735.129 +- 15.5813      6      good
chromium@479951      759.337 +- 26.8602      6      good
chromium@479953      729.477 +- 13.0502      6      good
chromium@479954      963.924 +- 16.6523      6      bad       <--
chromium@479968      1007.64 +- 36.4779      6      bad
chromium@479995      971.833 +- 32.2479      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 blink_perf.paint

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

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


| 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!
Status: Assigned (was: Untriaged)
Explictly assigning. A CL you landed tripped one of the speed metrics we measure in the lab. If this is the first time this has happened to one of your CLs, or if it's been a while, please read: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/addressing_performance_regressions.md

We're looking for one of the following:
1. Justification via explanation
2. Plan to revert or fix
3. Angry rage throwing of equipment at my head

Just be aware that I'm trained in trumpet playing and First Aid and am not afraid to use it.

Note: This was a bulk edit message and not very personal.
To be completely honest I don't see how my changed caused this. Nearly all of the code I added is only triggered if a flag is enabled, which is enabled only in a particular virtual test suite.

The change to Gradient.cpp could potentially slow things down a tiny bit if there was existing use of a gradient with a color filter, but (1) no tests failed, so I think this is unlikely, and (2) the tests that seemed to be slower don't seem to use gradients.

Any ideas?

I re-ran the bisect. If it comes back as your CL and you're not sure what to do, we'll cc the owners from go/chrome-benchmarks and ask them.
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Sep 19 2017


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

Hi dmazzoni@chromium.org, 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 : Dominic Mazzoni
  Commit : d5af4cd44c455022bd8fd747bf426f96c1a043e9
  Date   : Fri Jun 16 05:01:05 2017
  Subject: Add two image filtering modes to high contrast mode support.

Bisect Details
  Configuration: android_nexus7_perf_bisect
  Benchmark    : blink_perf.paint
  Metric       : large-table-background-change-with-zero-width-collapsed-borders/large-table-background-change-with-zero-width-collapsed-borders
  Change       : 30.56% | 752.89 -> 982.9877

Revision             Result                  N
chromium@479940      752.89 +- 14.875        6      good
chromium@479947      747.786 +- 10.8089      6      good
chromium@479951      768.762 +- 14.6686      6      good
chromium@479953      745.636 +- 14.9102      6      good
chromium@479954      968.948 +- 32.6235      6      bad       <--
chromium@479968      1018.16 +- 25.195       6      bad
chromium@479995      982.988 +- 35.8907      5      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 blink_perf.paint

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8968082290679495664


For feedback, file a bug with component Speed>Bisection
It does look like my change, but it's not showing up on any other platform.

Is it possible that it's due to a difference in compiler options? I believe Android optimizes a lot of code for size rather than for speed. Would it make sense to look into what specific files or modules are optimized for speed vs size here?

Having trouble running the benchmark locally. I created two changelists, a baseline and then one with the most likely portion of the key changelist commented out, and submitted perf try jobs:

Baseline: https://chromium-review.googlesource.com/c/chromium/src/+/673324
Patch:    https://chromium-review.googlesource.com/c/chromium/src/+/673265

Cc: wangxianzhu@chromium.org
Adding wangxianzhu, blink_perf.paint owner, to help with questions on reproducing and optimizing.
When I revert this patch on ToT I see no performance improvement.

Is it possible that any improvement from reverting this patch is being masked by the second performance regression that hit around ~488935?

https://chromeperf.appspot.com/group_report?rev=488935

I started a new bisect there:

https://chromeperf.appspot.com/buildbucket_job_status/8967957276429352016

Cc: nainar@chromium.org
Owner: wangxianzhu@chromium.org
Assigning to wangxianzhu for guidance on next steps, and cc'ing nainar since https://chromium-review.googlesource.com/572489 seems to be related.


Status: WontFix (was: Assigned)
I think the performance change is shadowed by  bug 750923  which we won't fix. Also won't fix this one.

Sign in to add a comment