Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(159)

Issue 571053002: Revert of Picture Recording: fix the performance bottleneck in SkDeferredCanvas::isFullFrame (Closed)

Created:
6 years, 3 months ago by mtklein
Modified:
6 years, 3 months ago
CC:
reviews_skia.org
Base URL:
https://chromium.googlesource.com/skia.git@master
Project:
skia
Visibility:
Public.

Description

Revert of Picture Recording: fix the performance bottleneck in SkDeferredCanvas::isFullFrame (patchset #7 id:140001 of https://codereview.chromium.org/545813002/) Reason for revert: This is leaking memory: http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-ASAN/builds/2516/steps/RunDM/logs/stdio Original issue's description: > Picture Recording: fix the performance bottleneck in SkDeferredCanvas::isFullFrame > > blink skips all pending commands during picture recording if it is drawing an opaque full-frame > geometry or image. This may improve performance for some edge cases. To recognize an opaque > full-frame drawing should be cheap enough. Otherwise, the overhead will offset the improvement. > Unfortunately, data from perf for content_shell on Nexus7 shows that SkDeferredCanvas::isFullFrame > is far from cheap. Table below shows that how much isFullFrame() costs in the whole render process. > > benchmark percentage > my local benchmark(draw 1000 sprites) 4.1% > speedReading 2.8% > FishIETank(1000 fishes) 1.5% > GUIMark3 Bitmap 2.0% > > By contrast, real recording (SkGPipeCanvas::drawBitmapRectToRect) and real rasterization > (GrDrawTarget::drawRect) cost ~4% and ~6% in the whole render process respectively. Apparently, > SkDeferredCanvas::isFullFrame() is nontrivial. > > getDeviceSize() is the main contributor to this hotspot. The change simply save the canvasSize and > reuse it among drawings if it is not a fresh frame. This change cut off ~65% (or improved ~2 times) > of isFullFrame(). > > telemetry smoothness canvas_tough_test didn't show obvious improvement or regression. > > BUG=411166 > > Committed: https://skia.googlesource.com/skia/+/8e45c3777d886ba3fe239bb549d06b0693692152 TBR=junov@chromium.org,tomhudson@google.com,reed@google.com,yunchao.he@intel.com NOTREECHECKS=true NOTRY=true BUG=411166 Committed: https://skia.googlesource.com/skia/+/5087b2c06762d0ea8b471f63953a587ecafaae0f

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -73 lines) Patch
M include/utils/SkDeferredCanvas.h View 2 chunks +0 lines, -8 lines 0 comments Download
M src/utils/SkDeferredCanvas.cpp View 4 chunks +1 line, -12 lines 0 comments Download
M tests/DeferredCanvasTest.cpp View 2 chunks +0 lines, -53 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
mtklein
Created Revert of Picture Recording: fix the performance bottleneck in SkDeferredCanvas::isFullFrame
6 years, 3 months ago (2014-09-15 13:00:05 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/571053002/1
6 years, 3 months ago (2014-09-15 13:00:44 UTC) #2
commit-bot: I haz the power
6 years, 3 months ago (2014-09-15 13:00:53 UTC) #3
Message was sent while issue was closed.
Committed patchset #1 (id:1) as 5087b2c06762d0ea8b471f63953a587ecafaae0f

Powered by Google App Engine
This is Rietveld 408576698