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

Issue 677420 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

30.8% regression in smoothness.sync_scroll.key_mobile_sites_smooth at 439493:439535

Project Member Reported by alexclarke@chromium.org, Dec 29 2016

Issue description

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

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


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

android-nexus6
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Dec 29 2016

Cc: fmalita@chromium.org
Owner: fmalita@chromium.org

=== PERF REGRESSION ===


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

Hi fmalita@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Clamp background tiles when possible
Author  : fmalita
Commit description:
  
We're currently always drawing tiled backgrounds in repeat/repeat mode,
even if we need tiling in one dimension only.

This has the unexpected side effect of opposite edge bleed, in the
dimension which doesn't require tiling, when rasterized for hidpi
devices (DSF causing Skia to sample across the tile boundary).

Detect cases where tile repetition is not needed, and strength-reduce the
shader tile mode accordingly.

Also make Image::drawPattern() protected, as it is only used in subclasses.

BUG= 673261 
R=reed@google.com,fs@opera.com,schenney@chromium.org

Review-Url: https://codereview.chromium.org/2582383002
Cr-Commit-Position: refs/heads/master@{#439529}
Commit  : a7e84a029b10a5a8c0b0faf4bda87e54187840fb
Date    : Mon Dec 19 20:01:39 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@439492  15.0659  23.7442  14  good
chromium@439514  13.1917  1.49373  6   good
chromium@439525  13.5897  1.84285  9   good
chromium@439528  14.0588  4.89534  9   good
chromium@439529  15.9566  2.2719   9   bad    <--
chromium@439530  16.186   3.15043  9   bad
chromium@439535  15.649   4.93229  14  bad

Bisect job ran on: android_nexus6_perf_bisect
Bug ID: 677420

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...cuteoverload.com smoothness.sync_scroll.key_mobile_sites_smooth
Test Metric: mean_input_event_latency/http___cuteoverload.com
Relative Change: 3.87%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2856
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8991948716260949072


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

| 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!
Status: Assigned (was: Untriaged)
This bug has an owner, but was in a state that our triage picked up. Marking as Assigned.
Cc: jmad...@chromium.org oetu...@nvidia.com ynovikov@chromium.org simonhatch@chromium.org
 Issue 677421  has been merged into this issue.
Cc: mtklein@chromium.org reed@google.com
The regression is somewhat expected: we're now sometimes using mixed tiling modes (clamp/repeat, repeat/clamp), and Skia is currently missing platform-optimized versions for these shaders.

I would argue we should just live with it for now:

1) the change fixes real rasterization bugs (correctness-vs-perf trade-off)

2) there are no plans to expand Skia's legacy shader specializations; the focus is now on the new raster pipeline impl, and when we switch it should hopefully also address these regressions.

reed@/mtklein@ - any thoughts?
Yeah, I stand by correctness over speed, and I don't think we're interested in developing or maintaining new contributions on any of that old code.

Are the Nexus 5x and Nexus 6 bots here running with software rasterization?  I'd expect we were using Ganesh in both cases.
Status: WontFix (was: Assigned)
WontFixing per #6 and #7.

Sign in to add a comment