Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2945813002: cc: Interface to evict images from cc image decode cache. (Closed)

Created:
8 months, 1 week ago by sohan
Modified:
8 months, 1 week ago
Reviewers:
vmpstr, ericrk
CC:
chromium-reviews, cc-bugs_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

cc: Interface to evict images from cc image decode cache. This adds interface to enable eviction of images from image decode cache in cc. This CL sets up the interface and eviction logic in gpu image decode cache. A follow up would handle the sw cache eviction. BUG=730784 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2945813002 Cr-Commit-Position: refs/heads/master@{#480961} Committed: https://chromium.googlesource.com/chromium/src/+/676e8ccff70ea1c30674e8f97e38296bdd1af8fc

Patch Set 1 #

Patch Set 2 : update #

Total comments: 10

Patch Set 3 : update #

Total comments: 2

Patch Set 4 : update #

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -0 lines) Patch
M cc/raster/image_hijack_canvas_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M cc/tiles/gpu_image_decode_cache.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cc/tiles/gpu_image_decode_cache.cc View 1 2 3 4 1 chunk +17 lines, -0 lines 0 comments Download
M cc/tiles/gpu_image_decode_cache_unittest.cc View 1 2 3 4 1 chunk +37 lines, -0 lines 0 comments Download
M cc/tiles/image_controller_unittest.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M cc/tiles/image_decode_cache.h View 1 chunk +6 lines, -0 lines 0 comments Download
M cc/tiles/software_image_decode_cache.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cc/tiles/software_image_decode_cache.cc View 1 2 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 37 (20 generated)
sohan
PTAL, if this is what we wanted ? Thanks.
8 months, 1 week ago (2017-06-19 12:17:21 UTC) #4
vmpstr
Yes, this looks exactly like the change we need. Thanks! https://codereview.chromium.org/2945813002/diff/40001/cc/tiles/gpu_image_decode_cache.cc File cc/tiles/gpu_image_decode_cache.cc (right): https://codereview.chromium.org/2945813002/diff/40001/cc/tiles/gpu_image_decode_cache.cc#newcode628 ...
8 months, 1 week ago (2017-06-19 16:34:42 UTC) #7
ericrk
https://codereview.chromium.org/2945813002/diff/40001/cc/tiles/gpu_image_decode_cache.cc File cc/tiles/gpu_image_decode_cache.cc (right): https://codereview.chromium.org/2945813002/diff/40001/cc/tiles/gpu_image_decode_cache.cc#newcode628 cc/tiles/gpu_image_decode_cache.cc:628: if (it != persistent_cache_.end() && !it->second->decode.is_locked()) { On 2017/06/19 ...
8 months, 1 week ago (2017-06-19 17:09:30 UTC) #8
sohan
PTAL. Thanks. https://codereview.chromium.org/2945813002/diff/40001/cc/tiles/gpu_image_decode_cache.cc File cc/tiles/gpu_image_decode_cache.cc (right): https://codereview.chromium.org/2945813002/diff/40001/cc/tiles/gpu_image_decode_cache.cc#newcode628 cc/tiles/gpu_image_decode_cache.cc:628: if (it != persistent_cache_.end() && !it->second->decode.is_locked()) { ...
8 months, 1 week ago (2017-06-20 13:31:39 UTC) #9
ericrk
LGTM, thanks! https://codereview.chromium.org/2945813002/diff/40001/cc/tiles/gpu_image_decode_cache_unittest.cc File cc/tiles/gpu_image_decode_cache_unittest.cc (right): https://codereview.chromium.org/2945813002/diff/40001/cc/tiles/gpu_image_decode_cache_unittest.cc#newcode1717 cc/tiles/gpu_image_decode_cache_unittest.cc:1717: TEST(GpuImageDecodeCacheTest, RemoveUnusedImage) { On 2017/06/20 13:31:39, sohan ...
8 months, 1 week ago (2017-06-20 16:06:05 UTC) #13
vmpstr
lgtm https://codereview.chromium.org/2945813002/diff/80001/cc/tiles/image_controller_unittest.cc File cc/tiles/image_controller_unittest.cc (right): https://codereview.chromium.org/2945813002/diff/80001/cc/tiles/image_controller_unittest.cc#newcode132 cc/tiles/image_controller_unittest.cc:132: void NotifyImageUnused(uint32_t skimage_id) override {}; I think format ...
8 months, 1 week ago (2017-06-20 17:10:23 UTC) #14
sohan
PTAL. Thanks. https://codereview.chromium.org/2945813002/diff/80001/cc/tiles/image_controller_unittest.cc File cc/tiles/image_controller_unittest.cc (right): https://codereview.chromium.org/2945813002/diff/80001/cc/tiles/image_controller_unittest.cc#newcode132 cc/tiles/image_controller_unittest.cc:132: void NotifyImageUnused(uint32_t skimage_id) override {}; On 2017/06/20 ...
8 months, 1 week ago (2017-06-20 17:19:08 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2945813002/100001
8 months, 1 week ago (2017-06-20 17:40:28 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/451924)
8 months, 1 week ago (2017-06-20 18:42:05 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2945813002/100001
8 months, 1 week ago (2017-06-20 18:43:54 UTC) #22
ericrk
On 2017/06/20 18:42:05, commit-bot: I haz the power wrote: > Try jobs failed on following ...
8 months, 1 week ago (2017-06-20 18:44:05 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/468835)
8 months, 1 week ago (2017-06-20 18:53:11 UTC) #25
sohan
On 2017/06/20 18:53:11, commit-bot: I haz the power wrote: > Try jobs failed on following ...
8 months, 1 week ago (2017-06-20 19:09:19 UTC) #26
sohan
On 2017/06/20 19:09:19, sohan wrote: > On 2017/06/20 18:53:11, commit-bot: I haz the power wrote: ...
8 months, 1 week ago (2017-06-20 19:48:37 UTC) #27
ericrk
still LGTM, thanks!
8 months, 1 week ago (2017-06-20 19:56:00 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2945813002/120001
8 months, 1 week ago (2017-06-20 19:58:30 UTC) #34
commit-bot: I haz the power
8 months, 1 week ago (2017-06-20 21:26:39 UTC) #37
Message was sent while issue was closed.
Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/676e8ccff70ea1c30674e8f97e38...

Powered by Google App Engine
This is Rietveld 408576698