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

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

Created:
5 months, 3 weeks ago by sohan
Modified:
5 months, 3 weeks 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.
5 months, 3 weeks 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 ...
5 months, 3 weeks 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 ...
5 months, 3 weeks 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()) { ...
5 months, 3 weeks 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 ...
5 months, 3 weeks 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 ...
5 months, 3 weeks 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 ...
5 months, 3 weeks 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
5 months, 3 weeks 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)
5 months, 3 weeks 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
5 months, 3 weeks 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 ...
5 months, 3 weeks 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)
5 months, 3 weeks 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 ...
5 months, 3 weeks 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: ...
5 months, 3 weeks ago (2017-06-20 19:48:37 UTC) #27
ericrk
still LGTM, thanks!
5 months, 3 weeks 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
5 months, 3 weeks ago (2017-06-20 19:58:30 UTC) #34
commit-bot: I haz the power
5 months, 3 weeks 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 0eb02b776