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

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


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

mac-10_12_laptop_low_end-perf

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/15aa9281640000

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

blink/canvas: Queue cleanup task to purge locked images for canvas2d. by khushalsagar@chromium.org
https://chromium.googlesource.com/chromium/src/+/a8ae8cd395ac425d4a521cbbec134c95c3e0e2f9
54.5 → 75.36 (+20.87)

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

The regression for frame_time in many_images is from https://chromium-review.googlesource.com/c/chromium/src/+/1181741/2/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc#512. Which limits the maximum number of entries the CanvasImageProvider locks to 500. The aim is to avoid unbounded growth of |locked_entries_| by putting some sane cap on it.

I wasn't able to repro it locally initially because a similar change landed downstream in the GpuImageDecodeCache to cap based on number of entries (https://chromium-review.googlesource.com/c/chromium/src/+/1179378). The test case has 400 unique images which were hitting the cap of 256 for working set size in the Gpu cache. So the regression here is expected just from placing a cap on how many images can be locked simultaneously for canvas.

But looking at the microsoft_speed_reading case, we're hitting a case where a small number of images are used in repeated draws. The CanvasImageProvider can't distinguish between the images it is locking, so it still releases the locked images even if it was the same images used in repeated draws with the 500 cap. I think the correct solution for this is to move this locking within the GpuImageDecodeCache which has the completion data for the memory and count of images we're locking. I'll follow up with this change.

As for the tasks_per_frame_total_all regressions, I'm surprised with the increase for a few cases. For tasks_per_frame_total_all/microsoft_speed_reading for instance,  it increased by ~200. I wasn't able to repro locally on my linux machine. I can see one shortcoming in the logic: it posts a task each time we start adding to |locked_images_|, even if these were evicted for any other reason in which we'd end up with multiple redundant tasks (https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc?q=CanvasImageProvi&sq=package:chromium&g=0&l=546). I could make it so it doesn't schedule another task if one is already pending.

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14eb6506640000

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/1288258a640000

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

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

commit 9500d6e2d1035ac42928e80f4d6d88cf675efccf
Author: Khushal <khushalsagar@chromium.org>
Date: Tue Aug 21 15:55:12 2018

blink/canvas: Limit cleanup tasks for locked images.

Don't queue a cleanup task if one is already pending. This avoid
queueing multiple redundant tasks for the case where images are being
purged mid-draw due to exceeding budget constraints.

R=fserb@chromium.org

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I17ba7c59ee6031bf2c18f81e3691685a78634f3c
Bug:  876001 ,  872117 
Reviewed-on: https://chromium-review.googlesource.com/1182601
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584759}
[modify] https://crrev.com/9500d6e2d1035ac42928e80f4d6d88cf675efccf/third_party/blink/renderer/platform/graphics/canvas_resource_provider.cc
[modify] https://crrev.com/9500d6e2d1035ac42928e80f4d6d88cf675efccf/third_party/blink/renderer/platform/graphics/canvas_resource_provider.h

I'll check the graphs again in a couple of days, after they've caught up with the change in #9.

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14eb6506640000

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/1288258a640000

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/12ceffb2640000

All windows regressions have recovered, and the only remaining one is many_images on mac. I'll validate that it is because of the assessment in #5 before closing this bug.

Triggered a pinpoint for traces with cc,blink. Just looking at the current traces from the bot, there is an increase in main frame duration (ThreadProxy::BeginMainFrame), from ~28 to ~38 ms on low-end and ~15 to ~19 on high end. I think its the unlocking of 500 images multiple times in a draw now, the traces should validate that.

https://pinpoint-dot-chromeperf.appspot.com/job/16876d26640000

The NextAction date has arrived: 2018-08-23

Looked at the traces and it is what I suspected. The test does 2000 draws in a single frame, with a cap of 500 |locked_images_|, we lock/unlock these images 4 times during a frame now instead of once earlier.

 Issue 876102  has been merged into this issue.