Implement a benchmark for GrResourceCache

Implement a benchmark for GrResourceCache

Adds "grresourcecache_add" and "grresourcecache_find" bench tests to test
GrResourceCache::add and GrResourceCache::find. The tests work only
with GPU backends, since GrResourceCache needs an GrGpu.

Modifies bench tests to override SkBenchmark::isSuitableFor(Backend)
function that specifies what kind of backend the test is inteded
for. This replaces the previous "fIsRendering" flag that would
indicate test that did no rendering.

Adds SkCanvas::getGrContext() call to get the GrContext that the
canvas ends up drawing to. The member function solves a common
use-case that is also used in the benchmark added here.

Committed: http://code.google.com/p/skia/source/detail?r=12334

[mtklein, 2013-11-19 12:59:00]

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cpp
File bench/GrResourceCacheBench.cpp (right):

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cp...
bench/GrResourceCacheBench.cpp:50: SK_DECLARE_INST_COUNT(StencilResource);
I'm not too clear on what the instance counting is for, but if it's necessary,
this probably wants to be SK_DECLARE_INST_COUNT(TextureResource)?

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cp...
bench/GrResourceCacheBench.cpp:193: GrResourceCache* cache = new
GrResourceCache(CACHE_SIZE_COUNT, CACHE_SIZE_BYTES);
Any reason this can't go on the stack?

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cp...
bench/GrResourceCacheBench.cpp:199: if (!check_cache_contents(cache, k)) {
I think we can just rely on one layer or checks?  Can we either make the method
not SkASSERT(false) all over the place, or just rename it
check_cache_contents_or_die (or something like that) instead of returning bool?

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cp...
bench/GrResourceCacheBench.cpp:237: if (fCache) {
For something like this, would prefer it if you made fCache an
SkAutoTDelete<GrResourceCache>.  Its .reset() will handle this logic for you,
and you won't need to have a postDraw().

https://codereview.chromium.org/73643005/diff/1/bench/SkBenchmark.h
File bench/SkBenchmark.h (right):

https://codereview.chromium.org/73643005/diff/1/bench/SkBenchmark.h#newcode124
bench/SkBenchmark.h:124: virtual void setContext(GrContext* context) { fContext
= context; };
Would something like GrContext* GM::GetGr(SkCanvas*) (in gm/gm.cpp) work here
too?  Seems annoying to have to thread the context through if it's retrievable
from the canvas.  If it is, we could extract that function into a little file to
share between bench and GM.

[Kimmo Kinnunen, 2013-11-19 14:12:48]

Thanks for the fast feedback.
Updated the patch according to the review suggestions.

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cpp
File bench/GrResourceCacheBench.cpp (right):

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cp...
bench/GrResourceCacheBench.cpp:50: SK_DECLARE_INST_COUNT(StencilResource);
On 2013/11/19 12:59:01, mtklein wrote:
> I'm not too clear on what the instance counting is for, but if it's necessary,
> this probably wants to be SK_DECLARE_INST_COUNT(TextureResource)?

Done.

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cp...
bench/GrResourceCacheBench.cpp:193: GrResourceCache* cache = new
GrResourceCache(CACHE_SIZE_COUNT, CACHE_SIZE_BYTES);
On 2013/11/19 12:59:01, mtklein wrote:
> Any reason this can't go on the stack?

Done. (It was like that to match the actual usage more accurately)

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cp...
bench/GrResourceCacheBench.cpp:199: if (!check_cache_contents(cache, k)) {
On 2013/11/19 12:59:01, mtklein wrote:
> I think we can just rely on one layer or checks?  Can we either make the
method
> not SkASSERT(false) all over the place, or just rename it
> check_cache_contents_or_die (or something like that) instead of returning
bool?

Done.

https://codereview.chromium.org/73643005/diff/1/bench/GrResourceCacheBench.cp...
bench/GrResourceCacheBench.cpp:237: if (fCache) {
On 2013/11/19 12:59:01, mtklein wrote:
> For something like this, would prefer it if you made fCache an
> SkAutoTDelete<GrResourceCache>.  Its .reset() will handle this logic for you,
> and you won't need to have a postDraw().

Done.

https://codereview.chromium.org/73643005/diff/1/bench/SkBenchmark.h
File bench/SkBenchmark.h (right):

https://codereview.chromium.org/73643005/diff/1/bench/SkBenchmark.h#newcode124
bench/SkBenchmark.h:124: virtual void setContext(GrContext* context) { fContext
= context; };
On 2013/11/19 12:59:01, mtklein wrote:
> Would something like GrContext* GM::GetGr(SkCanvas*) (in gm/gm.cpp) work here
> too?  Seems annoying to have to thread the context through if it's retrievable
> from the canvas.  If it is, we could extract that function into a little file
to
> share between bench and GM.

Done.

[bsalomon, 2013-11-19 14:40:46]

Thanks for writing this!

https://codereview.chromium.org/73643005/diff/200002/bench/ChecksumBench.cpp
File bench/ChecksumBench.cpp (right):

https://codereview.chromium.org/73643005/diff/200002/bench/ChecksumBench.cpp#...
bench/ChecksumBench.cpp:38: virtual bool isSuitableFor(Backend backend)
SK_OVERRIDE {
I like this mechanism.

https://codereview.chromium.org/73643005/diff/200002/src/gpu/utils/GrUtils.h
File src/gpu/utils/GrUtils.h (right):

https://codereview.chromium.org/73643005/diff/200002/src/gpu/utils/GrUtils.h#...
src/gpu/utils/GrUtils.h:19: GrContext* GetGr(/*very nearly const*/ SkCanvas*);
Having a getGrContext() function on SkCanvas keeps coming up and seems generally
useful. Why don't we just add it to SkCanvas rather than having a utility
function.

[mtklein, 2013-11-19 14:50:06]

On 2013/11/19 14:40:46, bsalomon wrote:
> Thanks for writing this!
> 
> https://codereview.chromium.org/73643005/diff/200002/bench/ChecksumBench.cpp
> File bench/ChecksumBench.cpp (right):
> 
>
https://codereview.chromium.org/73643005/diff/200002/bench/ChecksumBench.cpp#...
> bench/ChecksumBench.cpp:38: virtual bool isSuitableFor(Backend backend)
> SK_OVERRIDE {
> I like this mechanism.
> 
> https://codereview.chromium.org/73643005/diff/200002/src/gpu/utils/GrUtils.h
> File src/gpu/utils/GrUtils.h (right):
> 
>
https://codereview.chromium.org/73643005/diff/200002/src/gpu/utils/GrUtils.h#...
> src/gpu/utils/GrUtils.h:19: GrContext* GetGr(/*very nearly const*/ SkCanvas*);
> Having a getGrContext() function on SkCanvas keeps coming up and seems
generally
> useful. Why don't we just add it to SkCanvas rather than having a utility
> function.

This lgtm.  Wherever you and Brian find is best to put GetGr() is fine by me. 
I'll defer to Brian from here on.

[Kimmo Kinnunen, 2013-11-20 07:15:36]

https://codereview.chromium.org/73643005/diff/200002/src/gpu/utils/GrUtils.h
File src/gpu/utils/GrUtils.h (right):

https://codereview.chromium.org/73643005/diff/200002/src/gpu/utils/GrUtils.h#...
src/gpu/utils/GrUtils.h:19: GrContext* GetGr(/*very nearly const*/ SkCanvas*);
On 2013/11/19 14:40:47, bsalomon wrote:
> Having a getGrContext() function on SkCanvas keeps coming up and seems
generally
> useful. Why don't we just add it to SkCanvas rather than having a utility
> function.

Done.

[bsalomon, 2013-11-20 15:40:10]

On 2013/11/20 07:15:36, kkinnunen wrote:
> https://codereview.chromium.org/73643005/diff/200002/src/gpu/utils/GrUtils.h
> File src/gpu/utils/GrUtils.h (right):
> 
>
https://codereview.chromium.org/73643005/diff/200002/src/gpu/utils/GrUtils.h#...
> src/gpu/utils/GrUtils.h:19: GrContext* GetGr(/*very nearly const*/ SkCanvas*);
> On 2013/11/19 14:40:47, bsalomon wrote:
> > Having a getGrContext() function on SkCanvas keeps coming up and seems
> generally
> > useful. Why don't we just add it to SkCanvas rather than having a utility
> > function.
> 
> Done.

lgtm

[commit-bot: I haz the power, 2013-11-21 06:10:16]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/kkinnunen@nvidia.com/73643005/270001

[commit-bot: I haz the power, 2013-11-21 06:22:21]

Change committed as 12334