New issue
Advanced search Search tips

Issue 819185 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

5.2% regression in smoothness.tough_path_rendering_cases at 539947:540050

Project Member Reported by rmcilroy@chromium.org, Mar 6 2018

Issue description

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

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


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

chromium-rel-win7-dual
 Issue 819186  has been merged into this issue.
 Issue 819188  has been merged into this issue.
Project Member

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

Cc: fmalita@chromium.org bajones@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/12545b91440000

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 6 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 7 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 9 by 42576172...@developer.gserviceaccount.com, Mar 22 2018

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

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

📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/168f49e3440000
Status: Fixed (was: Assigned)
Although the Pinpoint couldn't reproduce a difference, the graph https://chromeperf.appspot.com/group_report?bug_id=819185 comes down significantly to the previous level after Point ID 548248. Mark as fixed.

Sign in to add a comment