New issue
Advanced search Search tips

Issue 733649 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

12.1%-17.9% regression in blink_perf.paint at 479057:479072

Project Member Reported by majidvp@chromium.org, Jun 15 2017

Issue description

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

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


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

android-webview-nexus6
Project Member

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

Cc: enne@chromium.org
Owner: enne@chromium.org

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

Hi enne@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 : Adrienne Walker
  Commit : 3839cba609280e72ffbde0d51da07a97c8501fb6
  Date   : Tue Jun 13 17:29:37 2017
  Subject: cc: Remove PaintOpBuffer first op optimization

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : blink_perf.paint
  Metric       : paint-offset-changes/paint-offset-changes
  Change       : 15.99% | 1315.83883333 -> 1526.21908333

Revision             Result                  N
chromium@479056      1315.84 +- 13.9128      6      good
chromium@479057      1496.96 +- 9.93434      6      bad       <--
chromium@479058      1502.48 +- 16.086       6      bad
chromium@479060      1489.99 +- 25.0532      6      bad
chromium@479064      1518.76 +- 14.987       6      bad
chromium@479072      1526.22 +- 26.1799      6      bad

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

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

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


| 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.

Comment 5 by enne@chromium.org, Aug 21 2017

Cc: danakj@chromium.org chrishtr@chromium.org vmp...@chromium.org
Belatedly investigating this bug.  This looks like a real regression in paint time and I think I underestimated how many "single item" DisplayItemLists there are.  I had thought that once PaintOpBuffer and DisplayItemList merged that there would be fewer/none of these, but in practice it seems like that's not the case.  That said, these pages have a whole bunch of constantly repainting small DisplayItemLists and so are hitting this particular code path.

However, removing this is a big complexity cleanup in a piece of code that's currently being changed a good bit.  I'm not really sure that this is worth doing again as-is, and would maybe want to think in the future about whether we need to do something like Skia's small pictures or whether DrawingRecorder should provide an initial buffer (to prevent the allocation during the resize for small recordings).

I'm inclined to WontFix this as (1) I think addressing this isn't a huge priority relative to other ongoing work and (2) I think these tests are exacerbating an edge case that I think is hit less often in the real world.

I can open up another bug with a feature request to consider evaluating POB performance or some other way to track this.
Thanks for investigating! From the perf sheriff perspective, WontFix is okay.

Comment 7 by enne@chromium.org, Aug 22 2017

Status: WontFix (was: Assigned)
Filed  issue 757870  to capture this future re-optimization.

Sign in to add a comment