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

Issue 593298 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug-Regression



Sign in to add a comment

66.7% regression in smoothness.tough_filters_cases at 379770:379800

Project Member Reported by ericwilligers@chromium.org, Mar 9 2016

Issue description

ChromiumPerf/android-galaxy-s5/smoothness.tough_filters_cases / frame_times / Filter
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=593298

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


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

android-galaxy-s5
Cc: ericwilligers@chromium.org
 Issue 593297  has been merged into this issue.
Bisect started here: https://chromeperf.appspot.com/buildbucket_job_status/9017915642907867648
on the clock test.
Trying again on the clock test.
Cc: senorblanco@chromium.org
cc test owner

Cc: nyerramilli@chromium.org
gentle ping .. senorblanco@ could you please check the issue
Cc: dongseong.hwang@chromium.org
The only thing that I could see as possibly related is https://codereview.chromium.org/1648433002. dongseong.hwang@chromium.org, could you take a look?

tdresser@, rschoen@: did any of the bisects succeed?

Note: seems to be Android S5-only, and specific to the Analog Clock test.
Launched a new job to get fresh logs. It seems in the previous one the device went down at some point. 
new job: https://chromeperf.appspot.com/buildbucket_job_status/9015281990043499056
Owner: senorblanco@chromium.org
Owner: chrishtr@chromium.org
This regression went away when partial raster was disabled at r381316, and returned when it was re-enabled at r384374:

https://chromeperf.appspot.com/report?sid=744b776b5475dbcce3144a62683816b2a5091714d73fbc795794dd5801c704b7&start_rev=375685&end_rev=388725

So I'm pretty sure it's partial raster that's causing the problem. Chris, could you take a look or reassign?
The regression is apparently on this page:

http://static.bobdo.net/Analog_Clock.svg
Cc: vmp...@chromium.org
I can repro on my Xperia Z5. For some frames, raster ends up taking 40ms or more. Without partial raster, pretty much all frames take about 20ms.

Digging more with Vlad.
Cc: reed@chromium.org
I think this might be a Skia performance issue exhibited by partial raster.
To elaborate a bit, it seems that the performance difference is entirely during playback to canvas. The difference here is that without partial raster, we rasterize the full canvas (no initial clips set). During partial raster, we rasterize partial canvas after applying a clip. 

I wonder if partial raster should detect the percentage area that it can reuse, and if the reuse is too small, then it's simply not worth doing it (ie, do the full raster). 
Cc: -reed@chromium.org reed@google.com
At first, I thought that the clips might be causing misses in the image filter cache. But I don't think that's the case, since the primitive is redrawn at a different angle each time, so there are no cache hits in the unclipped case as well (cache only works when the primitive is identical from frame to frame).

It's also strange that this regression only seems to affect Galaxy S5, and no other bots (AFAIK).
Owner: senorblanco@chromium.org
Vlad and I experimented by randomizing the clip applied in RasterSource::RasterCommon. This makes raster a lot slower (by a huge factor).
We conclude that partial raster is hitting this problem sometimes, and that
explains the regression.

There must be an optimization in skia paths that invalidates when the clip changes.

senorblanco, any ideas?
Components: Internals>Compositing>Rasterization
The clip most certainly can affect performance, particularly for the software backend (it limits the rasterization area) - so I don't think it's generally surprising to see variance with randomized clips.

@senorblanco: it's true that the image filter cache wouldn't help inter-frame (due to animation), but how about multiple tiles within the same frame?  IIUC now we're clipping to the tile bounds - doesn't that defeat the cache and apply the filter repeatedly for each tile?

Re: #17: that's a good point about path rendering. I had only considered filter caching, but Ganesh does have caches for both tessellated and paths that would be affected by clips. On the other hand, those only affect Ganesh, and I had assumed we're getting software raster here. 

Can someone verify if we're getting software raster or Ganesh here?

Is this only affecting the S5? If so, which model of S5 is it (Adreno or Mali)? If not, which other devices are affected?
Chris, could you try a reduction that removes the filters, to narrow down if it is path rendering?
Florin confirmed offline by checking with other Skia folks that there is no path cache.
He agrees with you that the image filter cache seems most likely. Link:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/src/core/SkImageFilter.cpp&l=214

Will try the suggestion in comment 21 first.
Now that I think about it some more (and look at the demo again), I think it probably is filter cache misses on the clock body itself, which is static.

If possible, one possible mitigation might be to have partial raster round up the clip to some nearest granular size, to benefit from cache hits. OTOH I have no idea how partial raster works (or what its goals are).
Perf sheriff ping - what's needed to move this forward?
Labels: -M-51 M-53
This will require a moderate amount of work in Skia to fix. I think we should live with the regression in the medium term, and keep the bug on to do that work. I'll tag it for M-53.
Labels: -performance-sheriff -M-53 M-54 Performance-Sheriff
Perf sheriff ping: reminder to follow up on possible performance issues
Components: Blink>CSS>Filters
Labels: -Pri-2 -M-54 -Performance-Sheriff Pri-3
No update here, and no plans to fix for this release. I think we'll have to live with this for now, but I'd like to keep the bug around as reminder for possible future work.
Components: -Blink>CSS>Filters Blink>Compositing>Filters
Components: Internals>GPU>Metrics

Sign in to add a comment