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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 days, 8 hours ago by sohan
Modified:
1 day, 21 hours 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
Commit queue not available (can’t edit this change).

Messages

Total messages: 37 (20 generated)
sohan
PTAL, if this is what we wanted ? Thanks.
3 days, 6 hours 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 ...
3 days, 2 hours 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 ...
3 days, 1 hour 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()) { ...
2 days, 5 hours 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 ...
2 days, 2 hours 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 ...
2 days, 1 hour 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 ...
2 days, 1 hour 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
2 days 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)
1 day, 23 hours 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
1 day, 23 hours 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 ...
1 day, 23 hours 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)
1 day, 23 hours 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 ...
1 day, 23 hours 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: ...
1 day, 22 hours ago (2017-06-20 19:48:37 UTC) #27
ericrk
still LGTM, thanks!
1 day, 22 hours 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
1 day, 22 hours ago (2017-06-20 19:58:30 UTC) #34
commit-bot: I haz the power
1 day, 21 hours 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318