All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=809441

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


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

chromium-rel-mac12

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/12e43221840000

📍 Found significant differences after each of 2 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/12e43221840000

canvas: Keep images locked during raster. by khushalsagar@chromium.org
chromium @ aa36bffbca291c237487d2374d416a9c59f4de2e

Add parser to deserialize authenticator responses by hongjunchoi@chromium.org
chromium @ 29d078b837b295a8e67130f8048136b8a9eaf314

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

The first of these two commits looks significantly more likely to be the cause of this regression.

Yup, very likely my change. I'll take a look.

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/13ee4631840000

So the root cause here seems to be the flushing of the GrContext. Prior to the change above, the GPU cache would regularly unlock images and in doing so cause the GrContext to flush (https://cs.chromium.org/chromium/src/cc/tiles/gpu_image_decode_cache.cc?q=gpu_image_dec&sq=package:chromium&l=193). Now this is deferred until the end of the frame since we keep the images locked until the complete recording is flushed. So the GPU main thread remains idle for almost ~72ms before skia actually flushes to the command buffer and also we end up waiting on a sync IPC (CommandBufferProxyImpl::WaitForGetOffset).

The reason this got better with the change that moved to using cc's cache on canvases was because it was implicitly causing a flush after each command that used the image. Now the behaviour is the same as it used to be prior to that change in using skia's cache.

+bsalomon, is this something we should address in skia?

Waiting on the sync IPC indicates that we're out of space either on the command buffer or the transfer buffer, and so we need the GPU to make progress. Flushing earlier could help, but so would increasing the command buffer / transfer buffer size.

IMO this flushing should be done by canvas2d. Skia doesn't have much context to know when a good or bad time to flush is. As we develop the SkDDL APIs we're  deferring a lot of gpu buffer allocations until flush time and so moving away from Ganesh flushing due to things like cache budget memory pressure.

We could do something like take a client provided call back that we query, say ever draw, and flush if told to.

 Issue 809473  has been merged into this issue.

We could look at re-enabling the CMD_HELPER_PERIODIC_FLUSH_CHECK feature in CommandBufferHelper, which was disabled on ANDROID a long time ago because of perf issues with the compositor context. The intent is to early flush before the command buffer gets full. Maybe we could make it optional so that it only affects command buffers that we expect to be "heavy" (canvas, webgl?) and don't have another explicit flushing mechanism (compositor worker)

The issue here is that skia defers flushing of its internal op list to the command buffer itself until explicitly told by the client, which happens at the end of canvas draw. We do most likely need a mechanism at the canvas level, which triggers skia to flush after every fixed number of ops.

I'm trying to do this in SkiaPaintCanvas. It knows the number of draw ops that have been issued to the SkCanvas, it should be able to manage flushing the GrContext.

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/16d64af1840000

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

canvas: Flush GrContext at regular intervals during draw. by khushalsagar@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/907867/3

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

Great! The patch above significantly improves frame times (https://pinpoint-dot-chromeperf.appspot.com/results2/16d64af1840000).

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

commit 058b1a95f1d7b5cd89fa106746ee01c5250ff562
Author: Khushal <khushalsagar@chromium.org>
Date: Wed Feb 14 21:25:50 2018

canvas: Flush GrContext at regular intervals during draw.

When drawing to an SkCanvas backed by a GrContext, skia internally
batches the GrOps which may be flushed only at the end of the canvas
draw. This results in inefficient parallelization with the GPU, since
these ops are not transferred via the CommandBuffer until the GrContext
is flushed, and it can quickly exceed the command buffer capacity which
results in synchronously waiting on the GPU before more commands can
be issued.

This change adds a setting to SkiaPaintCanvas for accelerated Canvas2D
that flushes the GrContext after a fixed number of draw calls have been
made to the SkCanvas.

R=piman@chromium.org

Bug:  809441 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Id85fc2a91db07b51b72d903117b68d8c4db14693
Reviewed-on: https://chromium-review.googlesource.com/907867
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536820}
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/cc/BUILD.gn
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/cc/paint/paint_op_buffer.cc
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/cc/paint/paint_op_buffer.h
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/cc/paint/skia_paint_canvas.cc
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/cc/paint/skia_paint_canvas.h
[add] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/cc/paint/skia_paint_canvas_unittest.cc
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/cc/test/test_skcanvas.cc
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/cc/test/test_skcanvas.h
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/gpu/command_buffer/client/cmd_buffer_helper.cc
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/third_party/WebKit/Source/platform/graphics/CanvasHeuristicParameters.h
[modify] https://crrev.com/058b1a95f1d7b5cd89fa106746ee01c5250ff562/third_party/WebKit/Source/platform/graphics/CanvasResourceProvider.cpp

Most graphs recovered with the change in #17 and a few other things got better too (https://chromeperf.appspot.com/group_report?rev=536820).