Project: skia Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 9 users
Status: Accepted
Owner:
Cc:
Area: ----
Priority: Medium
Type: Performance



Sign in to add a comment
Need better Blur performance on the Verge
Project Member Reported by tomhud...@chromium.org, Nov 11 2014 Back to list
See http://crbug.com/431021.

* All blurs on the page are nested rectangles, which avoids our analytic fast path.

* Only 1 in 3 nine-patchable blurs is hitting the cache.
* * This may be unavoidable if it's our first exposure to each of them, but it's not clear why we aren't able to reuse cached masks when many have similar parameters.
* * e.g. we'll see 4 consecutive constructions of a r45 753x843 mask.

* Only 1 in 3 blurs is nine-patchable.
* * Could we be caching non-ninepatched masks?
* * Are we being over-conservative in our ninepatchability decision? There are 525x225 areas with 45px-radius blurs that are falling into the "dy<0 => no 9p" branch, which seems wrong.


 
Project Member Comment 1 by qiankun....@intel.com, Nov 21 2014
* Only 1 in 3 nine-patchable blurs is hitting the cache.

If a blur effect cross several tiles on the page, blur mask will be calculated for each tile with different nested rectangles. The outer phrase of these rectangles are different, but these rectangles have the same size and same inner phrase. In current caching method, these rectangles are treated as different. I uploaded a patch to fix this: https://codereview.chromium.org/708073002/ .

* Only 1 in 3 blurs is nine-patchable.
For blurs cannot break into nine-patch, I made a proof of concept: https://codereview.chromium.org/729463002/. 

Project Member Comment 3 by bugdroid1@chromium.org, Jan 8 2015
The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/eabd0d73eebb940ec5ea06625f612a80156b61db

commit eabd0d73eebb940ec5ea06625f612a80156b61db
Author: qiankun.miao <qiankun.miao@intel.com>
Date: Wed Jan 07 19:20:49 2015 -0800

Revert of Cache blur mask for rects which can not break into nine-patch (patchset #10 id:200001 of https://codereview.chromium.org/729463002/)

Reason for revert:
revert it due to a memory leak.
==8017==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 25992 byte(s) in 2 object(s) allocated from:
    #0 0x7feb53030e0b in __interceptor_malloc
(/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/gm+0x24fe0b)
    #1 0x7feb54d54f76 in sk_malloc_flags(unsigned long, unsigned int)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/ports/SkMemory_malloc.cpp:54:15
    #2 0x7feb54d54d4a in sk_malloc_throw(unsigned long)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/ports/SkMemory_malloc.cpp:40:12
    #3 0x7feb539f5a77 in SkMask::AllocImage(unsigned long)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMask.cpp:37:22
    #4 0x7feb53fe5c34 in (anonymous
namespace)::copy_cacheddata_to_mask(SkCachedData*, SkMask*)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMaskCache.cpp:25:20
    #5 0x7feb53fea064 in SkMaskCache::FindAndCopy(float, SkBlurStyle,
SkBlurQuality, SkRect const*, int, SkMask*)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMaskCache.cpp:224:26
    #6 0x7feb539f957e in SkMaskFilter::filterPath(SkPath const&, SkMatrix
const&, SkRasterClip const&, SkBlitter*, SkPaint::Style) const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkMaskFilter.cpp:270:14
    #7 0x7feb5392e920 in SkDraw::drawPath(SkPath const&, SkPaint const&,
SkMatrix const*, bool, bool, SkBlitter*) const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkDraw.cpp:1117:13
    #8 0x7feb53694afc in SkDraw::drawPath(SkPath const&, SkPaint const&,
SkMatrix const*, bool) const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../include/core/SkDraw.h:54:9
    #9 0x7feb5368d799 in SkBitmapDevice::drawPath(SkDraw const&, SkPath const&,
SkPaint const&, SkMatrix const*, bool)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkBitmapDevice.cpp:216:5
    #10 0x7feb5386aa57 in SkCanvas::onDrawPath(SkPath const&, SkPaint const&)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkCanvas.cpp:1908:9
    #11 0x7feb5386386b in SkCanvas::drawPath(SkPath const&, SkPaint const&)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../src/core/SkCanvas.cpp:1703:5
    #12 0x7feb53109572 in Blur2RectsNonNinePatchGM::onDraw(SkCanvas*)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-000/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Debug-ASAN/build/skia/out/Debug/../../gm/blurs.cpp:166:9

Original issue's description:
> Cache blur mask for rects which can not break into nine-patch
>
> With this CL performance improves:
> blurrectsnonninepatch   42.4us -> 20.5us        0.48x
>
> BUG=431021,skia:3118
>
> Committed: https://skia.googlesource.com/skia/+/5a5b4e900ee69540fc35952882a323c63d6d0db9

TBR=reed@google.com,humper@google.com,reed@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=431021,skia:3118

Review URL: https://codereview.chromium.org/844673002

[modify] http://crrev.com/eabd0d73eebb940ec5ea06625f612a80156b61db/src/core/SkMaskCache.cpp
[modify] http://crrev.com/eabd0d73eebb940ec5ea06625f612a80156b61db/src/core/SkMaskCache.h
[modify] http://crrev.com/eabd0d73eebb940ec5ea06625f612a80156b61db/src/core/SkMaskFilter.cpp
[modify] http://crrev.com/eabd0d73eebb940ec5ea06625f612a80156b61db/src/effects/SkBlurMaskFilter.cpp

Is there plan to reland this ?
Project Member Comment 5 by qiankun....@intel.com, Feb 10 2015
reed is thinking about the cache policy. It depends on his decision.
Project Member Comment 6 by reed@google.com, Feb 10 2015
Cc: humper@google.com robertph...@google.com
Owner: reed@google.com
The issue is related to caching large resources (e.g. blurs) when only a small portion is needed.

When skia has to compute something (expensive), it always limits the are of interest to the visible portion. In the general case this is very much required, as the api allows a client to draw, say, a blurred circle 10,000x10,000. We pre-clip this before we blur to the bounds of the visible portion (current clip). This is the behavior we do for complex blurs on The Verge.

The blur types we cache today are "scalable". They have enough symmetry that we can only blur a shrunk version, and then produce a "nine-patch" overlay which stretches that to fit the current target. This is a fine balance, as the nine-patch element is finite to begin with. Some parts of the Verge take advantage of this, but not all. When we see an arbitrary path (something we can't see how to "shrink") then we don't cache it if it is much larger than the clip.

We will continue to analyze the particulars of the geometry coming out of the Verge, in hopes of finding more tricks that speed things up but don't kill our cache.
Sign in to add a comment