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

Issue 817942 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

18.7% regression in thread_times.key_silk_cases at 539951:540002

Project Member Reported by 42576172...@developer.gserviceaccount.com, Mar 1 2018

Issue description

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

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=d5d53a5a0fa7bbc2e48ca9dad2b6e00ef6f61f6f5f4d9c61c0e9297bef7ef796


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

android-webview-nexus6
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14e5ac24440000
Kicking off a few more bisects.
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Mar 16 2018

Cc: bajones@chromium.org fmalita@chromium.org liyuqian@google.com
Owner: liyuqian@google.com
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/16b45bde440000

Enable Delta Anti-Aliasing and Rebaseline by liyuqian@google.com
https://chromium.googlesource.com/chromium/src/+/e3053cdf8cfac55d0a4589a7f1fcc60796d60c41

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Mar 16 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/117f26de440000

Enable Delta Anti-Aliasing and Rebaseline by liyuqian@google.com
https://chromium.googlesource.com/chromium/src/+/e3053cdf8cfac55d0a4589a7f1fcc60796d60c41

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Mar 16 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1782a3ee440000

Enable Delta Anti-Aliasing and Rebaseline by liyuqian@google.com
https://chromium.googlesource.com/chromium/src/+/e3053cdf8cfac55d0a4589a7f1fcc60796d60c41

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 12 by 42576172...@developer.gserviceaccount.com, Mar 21 2018

📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/12a56b45440000
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 22 2018

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/ab46ac5ee801a048552425274804309eaaf9a805

commit ab46ac5ee801a048552425274804309eaaf9a805
Author: Yuqian Li <liyuqian@google.com>
Date: Thu Mar 22 16:24:24 2018

Better decisions about choosing AAA versus DAA

The Chrome perf regressions (817942, 819185) are
caused by the bad choices to use DAA instead of
AAA for stroking simple curves. (BTW, in my tests,
DAA is still faster than SAA so it's not too much
of a regression.)

We previously only considered the number of line segments,
but not the length of each segment. That leads to our
wrong judgement of the path complexity (the number of
intersections per scan line) when there are many short
line segments.

The change will bring the following performance change
to our nanobench:

    8.45% slower in clip_strategy_path_10
    8.39% slower in draw_stroke_bezier_quad_square_bevel_10
    3.91% slower in chart_aa
    3.76% faster in lines
    6.29% faster in path_stroke_big_oval
   22.81% faster in path_stroke_big_circle
   24.44% faster in giantdashline_vert_2

The two slower cases, clip_strategy_path_10
and draw_stroke_bezier_quad_square_bevel_10 are caused by
choosing AAA over DAA. Those two tests do seem to be simple
strokes. I'll later investigate why AAA is slower than DAA
for those two cases.

For now, I think that this change is sufficient to address
those chromium perf regresssions.

Bug:  chromium:817942   chromium:819185 
Change-Id: I1d13c968b17f257b4ede4c70e552db5016baf1ab
Reviewed-on: https://skia-review.googlesource.com/115583
Commit-Queue: Yuqian Li <liyuqian@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>

[modify] https://crrev.com/ab46ac5ee801a048552425274804309eaaf9a805/src/core/SkScan_AntiPath.cpp

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 22 2018

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

commit 011343525c605af1203468a807d5286ffca19a27
Author: skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Mar 22 20:30:28 2018

Roll src/third_party/skia/ 5fba7ad39..1b6930f55 (9 commits)

https://skia.googlesource.com/skia.git/+log/5fba7ad39a96..1b6930f55c7c

$ git log 5fba7ad39..1b6930f55 --date=short --no-merges --format='%ad %ae %s'
2018-03-22 angle-skia-autoroll Roll skia/third_party/externals/angle2/ 8b92c53b8..107c72476 (2 commits)
2018-03-22 mtklein lock before using fCacheKeys in ~SkImageFilter()
2018-03-22 halcanary dm: fix unused-private-field error when skia_use_expat=false
2018-03-22 mtklein add repro case for skia:7674
2018-03-22 mtklein rm SkTDArray::{Deleter,release()}
2018-03-22 fmalita [sksg] Simplify TrimEffect
2018-03-22 herb Remove the last uses of auto glyph cache.
2018-03-21 liyuqian Better decisions about choosing AAA versus DAA
2018-03-22 liyuqian Reland "Remove legacy precision boundary"

Created with:
  roll-dep src/third_party/skia
BUG= chromium:817942   chromium:819185 


The AutoRoll server is located here: https://autoroll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=jvanverth@chromium.org

Change-Id: Ica709d14fc6aa380c5facf6ca2f30c31e92f2388
Reviewed-on: https://chromium-review.googlesource.com/976382
Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#545231}
[modify] https://crrev.com/011343525c605af1203468a807d5286ffca19a27/DEPS

Project Member

Comment 16 by 42576172...@developer.gserviceaccount.com, Mar 22 2018

Cc: liyuqian@chromium.org
Owner: liyuqian@chromium.org
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/11bccfe5440000

Remove SK_SUPPORT_LEGACY_AA_CHOICE by liyuqian@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/976622/1

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Owner: liyuqian@google.com
Project Member

Comment 19 by 42576172...@developer.gserviceaccount.com, Mar 26 2018

Owner: liyuqian@chromium.org
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/14ffeb93440000

Remove SK_SUPPORT_LEGACY_AA_CHOICE by liyuqian@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/976622/1

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Owner: liyuqian@google.com
Status: Fixed (was: Assigned)
Cc: primiano@chromium.org
 Issue 830779  has been merged into this issue.

Sign in to add a comment