Make GrResourceCache dynamically change between LRU and random replacement strategies.

Make GrResourceCache dynamically change between LRU and random replacement strategies.

Random performs significantly better when each frame exceeds the budget by a small margin whereas LRU has worst case behavior.

The decision of which to use is made based on the history from a few frames of the ratio of total unique key cache misses to unique key cache misses of resources purged in the last 2 frames.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2321563006

Committed: https://skia.googlesource.com/skia/+/0f147ac2ae575bbad3515a526f13700bc5c8e9d7

[bsalomon, 2016-09-08 15:36:20]

Description was changed from

==========
Make GrResourceCache dynamically change between LRU and random replacement
strategies.

Random performs significantly better when each frame exceeds the budget by a
small margin whereas LRU has worst case behavior.

The decision of which to use is made based on the history from a few frames of
the ratio of total unique key cache misses to unique key cache misses of
resources purged in the last 2 frames.
==========

to

==========
Make GrResourceCache dynamically change between LRU and random replacement
strategies.

Random performs significantly better when each frame exceeds the budget by a
small margin whereas LRU has worst case behavior.

The decision of which to use is made based on the history from a few frames of
the ratio of total unique key cache misses to unique key cache misses of
resources purged in the last 2 frames.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2321563006
==========

[bsalomon, 2016-09-08 18:51:31]

Before:

  43/43 
MB	1	49.1ms	53.3ms	53.1ms	57.6ms	4%	▅█▁▃▂▄▆▄▄▅	gpu	image_cache_budget_dynamic_flip_flop
  44/44 
MB	1	45.9ms	50.9ms	49.5ms	51.7ms	5%	█▄█▇█▁▃▂▅█	gpu	image_cache_budget_dynamic_ping_pong
  44/44 
MB	1	13.8ms	13.9ms	14.3ms	16.5ms	6%	▁▁▂█▄▁▁▁▁▁	gpu	image_cache_budget_200_shuffle
  44/44 
MB	1	9.79ms	11.4ms	11.3ms	13.5ms	13%	▇▄▁▁▁▁▄█▄▆	gpu	image_cache_budget_125_shuffle
  44/44 
MB	1	7.28ms	8.81ms	8.43ms	9.58ms	10%	▂▃▆▁▆▃▃▆█▇	gpu	image_cache_budget_111_shuffle
  44/44 
MB	3	1.72ms	1.79ms	1.86ms	2.16ms	9%	▆▅▂▁▁▁▁▁▆█	gpu	image_cache_budget_95_shuffle
  44/44 
MB	1	16.9ms	20ms	22.2ms	30.4ms	21%	▅█▁▄▂▂█▂▂▃	gpu	image_cache_budget_200
  44/44  MB	1	17ms	19ms	18.9ms	22.9ms	10%	▂█▁▁▃▅▄▂▆▂	gpu	image_cache_budget_125
  44/44 
MB	1	16.5ms	20.1ms	20.1ms	25.3ms	16%	▁▄▅▆▂▁▂█▂█	gpu	image_cache_budget_111
  44/44 
MB	1	1.81ms	2.71ms	2.49ms	3.36ms	20%	▁▁▅▆▃▅█▅▃▄	gpu	image_cache_budget_95


After:

  42/42 
MB	1	52.1ms	57.1ms	56.6ms	60.1ms	5%	█▇██▄▂▁▅▅▁	gpu	image_cache_budget_dynamic_flip_flop
  42/42 
MB	1	33.1ms	35.6ms	35.5ms	38.4ms	5%	▁▇▂▃▃▆▄█▂▄	gpu	image_cache_budget_dynamic_ping_pong
  42/42 
MB	1	18ms	18.5ms	19.1ms	21.7ms	6%	▁▂▂▆▂▁▁▂█▃	gpu	image_cache_budget_200_shuffle
  42/42 
MB	1	10.8ms	11.4ms	11.7ms	13.8ms	7%	▁▃▂▂▂▄▄▂█▂	gpu	image_cache_budget_125_shuffle
  42/42 
MB	1	6.36ms	7.36ms	7.35ms	9.46ms	12%	▂▂▁▃▃▃▂▃█▄	gpu	image_cache_budget_111_shuffle
  42/42 
MB	3	1.65ms	1.66ms	1.7ms	2ms	6%	█▂▁▁▁▁▂▁▁▁	gpu	image_cache_budget_95_shuffle
  42/42 
MB	1	21.2ms	24.9ms	24.1ms	26.7ms	8%	█▆▇█▇▃▄▂▂▁	gpu	image_cache_budget_200
  42/42 
MB	1	12.4ms	13.3ms	13.4ms	15.6ms	7%	▃▃▁▁▃▃▃█▄▄	gpu	image_cache_budget_125
  42/42 
MB	1	7.16ms	8.24ms	8.06ms	8.77ms	6%	▄▆▄█▆▁▆▃▆▄	gpu	image_cache_budget_111
  42/42 
MB	3	1.61ms	1.65ms	1.85ms	2.71ms	20%	█▅▁▁▁▂▁▁▁▄	gpu	image_cache_budget_95

The image_cache_budget_200 and shuffle variation get a bit worse. The others
stay the same or get better.

[bsalomon, 2016-09-08 18:51:57]

bsalomon@google.com changed reviewers:
+ robertphillips@google.com

[bsalomon, 2016-09-08 18:51:58]

When you have time, no rush.

[robertphillips, 2016-09-13 13:05:22]

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrDrawingManage...
File src/gpu/GrDrawingManager.cpp (right):

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrDrawingManage...
src/gpu/GrDrawingManager.cpp:132: // Avoid notifying the cache about redundant
client flushes.
Should this 'fLastFlushType' be 'type' ?
I guess I'm not groking what's going on here

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
File src/gpu/GrResourceCache.cpp (right):

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
src/gpu/GrResourceCache.cpp:69: , fStrategy(ReplacementStrategy::kLRU)
Should kInitialStrategyScoreMax just be kInitialStrategyScore ?

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
src/gpu/GrResourceCache.cpp:515: 
I wonder if it would be better to use function pointers rather than always
having an if test.

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache.h
File src/gpu/GrResourceCache.h (right):

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
src/gpu/GrResourceCache.h:283: 
missing space before '{' ?

[bsalomon, 2016-09-13 13:31:33]

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrDrawingManage...
File src/gpu/GrDrawingManager.cpp (right):

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrDrawingManage...
src/gpu/GrDrawingManager.cpp:132: // Avoid notifying the cache about redundant
client flushes.
On 2016/09/13 13:05:21, robertphillips wrote:
> Should this 'fLastFlushType' be 'type' ?
> I guess I'm not groking what's going on here

Changing it slightly to:

    // Avoid notifying the cache about successive client flushes where no
rendering occurred between
    // them.
    bool skipNotify = false;
    if (!flushed) {
        skipNotify = GrResourceCache::kExternal == fLastFlushType &&
                     GrResourceCache::kExternal == type;
    }


which hopefully is more correct and clear.

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
File src/gpu/GrResourceCache.cpp (right):

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
src/gpu/GrResourceCache.cpp:69: , fStrategy(ReplacementStrategy::kLRU)
On 2016/09/13 13:05:21, robertphillips wrote:
> Should kInitialStrategyScoreMax just be kInitialStrategyScore ?

Ha, yes.

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
src/gpu/GrResourceCache.cpp:515: 
On 2016/09/13 13:05:21, robertphillips wrote:
> I wonder if it would be better to use function pointers rather than always
> having an if test.

We're about to perform a fairly heavy operation, so I'm not sure it matters from
a performance perspective. Or did you mean for clarity?

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache.h
File src/gpu/GrResourceCache.h (right):

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
src/gpu/GrResourceCache.h:283: 
On 2016/09/13 13:05:21, robertphillips wrote:
> missing space before '{' ?

Done.

[robertphillips, 2016-09-13 13:56:48]

lgtm

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrDrawingManage...
File src/gpu/GrDrawingManager.cpp (right):

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrDrawingManage...
src/gpu/GrDrawingManager.cpp:132: // Avoid notifying the cache about redundant
client flushes.
That is far clearer. \o/

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
File src/gpu/GrResourceCache.cpp (right):

https://codereview.chromium.org/2321563006/diff/20001/src/gpu/GrResourceCache...
src/gpu/GrResourceCache.cpp:515: 
A bit of both but it is certainly fine as is.

[bsalomon, 2016-09-13 14:15:46]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-09-13 14:15:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 14:18:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 14:18:03]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-GN-Trybot on master.client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Build-Ubuntu-GCC-x86_64-Release-GN-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[bsalomon, 2016-09-13 14:53:37]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-09-13 14:53:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/2321563006/#ps60001
(title: "work around gcc bug")

[commit-bot: I haz the power, 2016-09-13 14:53:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 15:03:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 15:03:18]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Release-GN-Trybot on
master.client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU...)

[bsalomon, 2016-10-03 20:41:26]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-10-03 20:41:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/2321563006/#ps140001
(title: "fix recursive flusing, add asserts")

[commit-bot: I haz the power, 2016-10-03 20:41:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-03 21:07:03]

Description was changed from

==========
Make GrResourceCache dynamically change between LRU and random replacement
strategies.

Random performs significantly better when each frame exceeds the budget by a
small margin whereas LRU has worst case behavior.

The decision of which to use is made based on the history from a few frames of
the ratio of total unique key cache misses to unique key cache misses of
resources purged in the last 2 frames.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2321563006
==========

to

==========
Make GrResourceCache dynamically change between LRU and random replacement
strategies.

Random performs significantly better when each frame exceeds the budget by a
small margin whereas LRU has worst case behavior.

The decision of which to use is made based on the history from a few frames of
the ratio of total unique key cache misses to unique key cache misses of
resources purged in the last 2 frames.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2321563006

Committed:
https://skia.googlesource.com/skia/+/0f147ac2ae575bbad3515a526f13700bc5c8e9d7
==========

[commit-bot: I haz the power, 2016-10-03 21:07:03]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/0f147ac2ae575bbad3515a526f13700bc5c8e9d7

[robertphillips, 2016-10-04 12:30:04]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/2386993004/ by robertphillips@google.com.

The reason for reverting is: Causing problems on Mac & Windows bots..

[robertphillips, 2016-10-04 12:31:54]

On this Macs this is aborting with the call stack:

	msaa16 gm  textblobmixedsizes

Stack trace:
    0   dm                                  0x000000010d6ea5ca
_ZL13crash_handleri + 138
    1   libsystem_platform.dylib            0x00007fff92c77eaa _sigtramp + 26
    2   ???                                 0x00007fff68e94f02 0x0 +
140734953508610
    3   libsystem_c.dylib                   0x00007fff953d36e7 abort + 129
    4   dm                                  0x000000010ddc1269
_Z17sk_abort_no_printv + 9
    5   dm                                  0x000000010d71ad54
_ZNK7GrIORefI13GrGpuResourceE8validateEv + 52
    6   dm                                  0x000000010d71ae02
_ZNK7GrIORefI13GrGpuResourceE23didRemoveRefOrPendingIOENS1_7CntTypeE + 18
    7   dm                                  0x000000010de05fba
_ZL13SkSafeSetNullI19GrStencilAttachmentEvRPT_ + 26
    8   dm                                  0x000000010de05f82
_ZN14GrRenderTarget9onReleaseEv + 18
    9   dm                                  0x000000010deea162
_ZN23GrGLTextureRenderTarget9onReleaseEv + 18
    10  dm                                  0x000000010ddf3057
_ZN13GrGpuResource7releaseEv + 71
    11  dm                                  0x000000010de0d441
_ZN13GrGpuResource11CacheAccess7releaseEv + 17
    12  dm                                  0x000000010de0c594
_ZN15GrResourceCache21internalPurgeAsNeededEb + 516
    13  dm                                  0x000000010ddf3516
_ZNK13GrGpuResource20notifyRefCountIsZeroEv + 86
    14  dm                                  0x000000010d76b14b
_ZNK7GrIORefI13GrGpuResourceE5unrefEv + 75
    15  dm                                  0x000000010de05fba
_ZL13SkSafeSetNullI19GrStencilAttachmentEvRPT_ + 26
    16  dm                                  0x000000010de05f82
_ZN14GrRenderTarget9onReleaseEv + 18
    17  dm                                  0x000000010deea162
_ZN23GrGLTextureRenderTarget9onReleaseEv + 18
    18  dm                                  0x000000010ddf3057
_ZN13GrGpuResource7releaseEv + 71
    19  dm                                  0x000000010de0d441
_ZN13GrGpuResource11CacheAccess7releaseEv + 17
    20  dm                                  0x000000010de0bed9
_ZN15GrResourceCache20notifyCntReachedZeroEP13GrGpuResourcej + 793
    21  dm                                  0x000000010ddf3516
_ZNK13GrGpuResource20notifyRefCountIsZeroEv + 86
    22  dm                                  0x000000010d76b14b
_ZNK7GrIORefI13GrGpuResourceE5unrefEv + 75
    23  dm                                  0x000000010da54781
_ZN5sk_spI14GrRenderTargetED2Ev + 17
    24  dm                                  0x000000010dde1934
_ZN13GrDrawContextD2Ev + 84
    25  dm                                  0x000000010dde198e
_ZN13GrDrawContextD0Ev + 14
    26  dm                                  0x000000010d7c6531
_ZN5sk_spI13GrDrawContextED2Ev + 17
    27  dm                                  0x000000010df02ba5
_ZN11SkGpuDeviceD2Ev + 37
    28  dm                                  0x000000010df0274e
_ZN11SkGpuDeviceD0Ev + 14
    29  dm                                  0x000000010db90e33 _ZN8DeviceCMD2Ev
+ 19
    30  dm                                  0x000000010db8723b
_ZN8SkCanvas15internalRestoreEv + 347
    31  dm                                  0x000000010db870a6 _ZN8SkCanvasD2Ev
+ 38
    32  dm                                  0x000000010db872ae _ZN8SkCanvasD0Ev
+ 14
    33  dm                                  0x000000010dca1d75
_ZN14SkSurface_BaseD2Ev + 53
    34  dm                                  0x000000010df0942e
_ZN13SkSurface_GpuD0Ev + 14
    35  dm                                  0x000000010d6fde81
_ZN5sk_spI9SkSurfaceED2Ev + 17
    36  dm                                  0x000000010d6f87d1
_ZNK2DM7GPUSink4drawERKNS_3SrcEP8SkBitmapP9SkWStreamP8SkString + 849
    37  dm                                  0x000000010d6ede39 _ZN4Task3RunERKS_
+ 249
    38  dm                                  0x000000010d6e948d _Z7dm_mainv +
1309
    39  libdyld.dylib                       0x00007fff9b5475ad start + 1
    40  ???                                 0x000000000000018c 0x0 + 396

[robertphillips, 2016-10-04 12:33:13]

On Windows this is failing on the assert:

c:\b\work\skia\include\gpu\grgpuresource.h:72: fatal error: "assert(fRefCnt >=
0)"

Caught exception 2147483651 EXCEPTION_BREAKPOINT, was running:
	nvprdit16 image gen_codec_gpu abnormal.wbmp
Likely culprit:
	nvprdit16 image gen_codec_gpu abnormal.wbmp