Create a new disk_cache type, SHADER_CACHE.

Create a new disk_cache type, SHADER_CACHE.

Create a new type of disk_cache to be used by the GL Shader
cache. This cache acts similar to the regular DISK_CACHE
except the rank of the entry is not updated if the entry
is not modified. This means we can read entries as we
iterate and the iterators do not get shuffled around.

BUG=166763
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182343

[dsinclair, 2013-02-05 22:09:51]

Hello Ricardo,

I've created the new SHADER_CACHE type for the GL cache. Can you please take a
look at this CL for me?

Thanks,
dan

[rvargas (doing something else), 2013-02-06 20:06:39]

BackendImpl::SyncInit should dcheck that new_eviction_ is false.

We need a test following DiskCacheEntryTest::GetTimes that verifies that
LastModified changes and LastUsed does not.

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/backend_unitte...
File net/disk_cache/backend_unittest.cc (right):

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/backend_unitte...
net/disk_cache/backend_unittest.cc:1150: TEST_F(DiskCacheBackendTest,
ShaderCacheEnumerationReadData) {
nit: add a comment saying what is this about (verifying that Reads don't alter
the LRU)

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/backend_unitte...
net/disk_cache/backend_unittest.cc:1176: // Two entries and the iterator
pointing at "first".
The iterator points at "second"

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/histogram_macr...
File net/disk_cache/histogram_macros.h (right):

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/histogram_macr...
net/disk_cache/histogram_macros.h:115: case net::SHADER_CACHE:\
reminder: this implies adding a bunch of new histogram descriptions.

[dsinclair, 2013-02-07 18:03:07]

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/backend_unitte...
File net/disk_cache/backend_unittest.cc (right):

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/backend_unitte...
net/disk_cache/backend_unittest.cc:1150: TEST_F(DiskCacheBackendTest,
ShaderCacheEnumerationReadData) {
On 2013/02/06 20:06:39, rvargas wrote:
> nit: add a comment saying what is this about (verifying that Reads don't alter
> the LRU)

Done.

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/backend_unitte...
net/disk_cache/backend_unittest.cc:1176: // Two entries and the iterator
pointing at "first".
On 2013/02/06 20:06:39, rvargas wrote:
> The iterator points at "second"

Simplified the test case, should be more readable now.

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/histogram_macr...
File net/disk_cache/histogram_macros.h (right):

https://codereview.chromium.org/12224017/diff/1/net/disk_cache/histogram_macr...
net/disk_cache/histogram_macros.h:115: case net::SHADER_CACHE:\
On 2013/02/06 20:06:39, rvargas wrote:
> reminder: this implies adding a bunch of new histogram descriptions.

Copied the DiskCache.3 entries and made them DiskCache.4 entries. Didn't touch
any of the later stuff as it looked like experiments?

[rvargas (doing something else), 2013-02-11 23:12:47]

lgtm after fixing the unit test.

https://codereview.chromium.org/12224017/diff/3001/net/disk_cache/backend_uni...
File net/disk_cache/backend_unittest.cc (right):

https://codereview.chromium.org/12224017/diff/3001/net/disk_cache/backend_uni...
net/disk_cache/backend_unittest.cc:1175: ASSERT_EQ(net::OK, OpenEntry(second,
&entry2));
entry2 was already the first entry on the list... and OpenEntry doesn't affect
the LRU so this is somewhat confusing.

[dsinclair, 2013-02-12 16:05:50]

rvargas, realized that OnExternalCacheHit would not work with the shader cache.
I made a couple of changes to support it, can you please take a look and let me
know if everything is still good.

Thanks,
dan

https://codereview.chromium.org/12224017/diff/3001/net/disk_cache/backend_uni...
File net/disk_cache/backend_unittest.cc (right):

https://codereview.chromium.org/12224017/diff/3001/net/disk_cache/backend_uni...
net/disk_cache/backend_unittest.cc:1175: ASSERT_EQ(net::OK, OpenEntry(second,
&entry2));
On 2013/02/11 23:12:47, rvargas wrote:
> entry2 was already the first entry on the list... and OpenEntry doesn't affect
> the LRU so this is somewhat confusing.

Doh, I was thinking the Open would kick in and update the LRU but I see that
only happens for DISK_CACHE due to the new_eviction setting.

Added a new test for OnExternalCache hit as it will currently not work, and I
need it to work else I have to re-write the whole entry just to update the LRU
setting.

[rvargas (doing something else), 2013-02-13 02:27:17]

https://codereview.chromium.org/12224017/diff/10002/net/disk_cache/backend_im...
File net/disk_cache/backend_impl.cc (right):

https://codereview.chromium.org/12224017/diff/10002/net/disk_cache/backend_im...
net/disk_cache/backend_impl.cc:675: UpdateRankOnCacheHit(cache_entry, false);
Not so happy.

How about
UpdateRank(cache_entry, cache_type() == net::SHADER_CACHE);

[dsinclair, 2013-02-13 18:20:08]

https://codereview.chromium.org/12224017/diff/10002/net/disk_cache/backend_im...
File net/disk_cache/backend_impl.cc (right):

https://codereview.chromium.org/12224017/diff/10002/net/disk_cache/backend_im...
net/disk_cache/backend_impl.cc:675: UpdateRankOnCacheHit(cache_entry, false);
Done. That's much nicer. Thanks.

On 2013/02/13 02:27:17, rvargas wrote:
> Not so happy.
> 
> How about
> UpdateRank(cache_entry, cache_type() == net::SHADER_CACHE);

[rvargas (doing something else), 2013-02-13 20:25:28]

revision 4 lgtm.

[dsinclair, 2013-02-13 20:28:06]

On 2013/02/13 20:25:28, rvargas wrote:
> revision 4 lgtm.

Awesome, thanks. Quick question, does the histograms.xml file have to go in
first, this code in first, or it doesn't matter as long as they both go in?

[rvargas (doing something else), 2013-02-13 20:40:39]

On 2013/02/13 20:28:06, dsinclair wrote:
> On 2013/02/13 20:25:28, rvargas wrote:
> > revision 4 lgtm.
> 
> Awesome, thanks. Quick question, does the histograms.xml file have to go in
> first, this code in first, or it doesn't matter as long as they both go in?

It doesn't really matter. Usually the code goes first and then the description

[commit-bot: I haz the power, 2013-02-13 21:08:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dsinclair@chromium.org/12224017/13001

[commit-bot: I haz the power, 2013-02-13 23:57:39]

Change committed as 182343