Allow GPU resources to not be counted against the cache budget.

Allow GPU resources to not be counted against the cache budget.

BUG=skia:2889
Committed: https://skia.googlesource.com/skia/+/84c8e62fad59f0e19b40ac718467f5b7884b431d

[bsalomon, 2014-11-13 19:20:53]

bsalomon@google.com changed reviewers:
+ egdaniel@google.com, jvanverth@google.com, robertphillips@google.com

[bsalomon, 2014-11-13 19:20:54]

TBR=rob

Depends upon https://codereview.chromium.org/720033004/, which in turn depends
upon https://codereview.chromium.org/716143004/.

https://codereview.chromium.org/721353002/diff/20001/include/gpu/GrGpuResource.h
File include/gpu/GrGpuResource.h (right):

https://codereview.chromium.org/721353002/diff/20001/include/gpu/GrGpuResourc...
include/gpu/GrGpuResource.h:268: // This is not ref'ed but abandon() or
release() will be called before the GrGpu object
Mostly shuffling order and removed fContentKeySet since it is now a flag.

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

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrContext.cpp#ne...
src/gpu/GrContext.cpp:482: GrTexture* texture = fGpu->createTexture(descCopy,
srcData, rowBytes);
This was the only function where we were deliberately creating something outside
the cache. The old cache required explicit registration with the cache. The new
cache does not. So when switching to the new cache "texture" was counted against
the budget just by virtue of being born.

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

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrGpuResource.cp...
src/gpu/GrGpuResource.cpp:124: if (this->cacheAccess().isWrapped()) {
This is unnecessary. Will revert before submitting.

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrGpuResourceCac...
File src/gpu/GrGpuResourceCacheAccess.h (right):

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrGpuResourceCac...
src/gpu/GrGpuResourceCacheAccess.h:34: void setBudgeted(bool
countsAgainstBudget) { fResource->setBudgeted(countsAgainstBudget); }
This class provides privileged access the parts of GrGpuResource that deal with
the cache. It exists for code in src/gpu to manipulate caching without allowing
that state to be manipulated outside Skia.

I've added this function to control whether a resource is budgeted and a query
below.

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

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrResourceCache2...
src/gpu/GrResourceCache2.cpp:107: if (resource->cacheAccess().isBudgeted()) {
Previously isWrapped() controlled whether a resource counted against the budget.
Now isWrapped() implies not budgeted, but not all unbudgeted resources are
wrapped.

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

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrResourceCache2...
src/gpu/GrResourceCache2.h:284: void didChangeBudgetStatus(GrGpuResource*
resource) { fCache->didChangeBudgetStatus(resource); }
This class contains the interface to GrResourceCache2 that is only to be used by
GrGpuResource. This function is just a pass through to the private function on
GrResourceCache2, declared above.

[egdaniel, 2014-11-13 20:47:32]

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

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrGpuResource.cp...
src/gpu/GrGpuResource.cpp:25: , fScratchKey(GrResourceKey::NullScratchKey())  {
extra space

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrGpuResource.cp...
src/gpu/GrGpuResource.cpp:32: 
Extra \n added

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

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrResourceCache2...
src/gpu/GrResourceCache2.cpp:492: "\t\tEntry Count: current %d (%d budgeted, %d
wrapped, %d locked, %d scratch %.2g%% full), high %d\n",
over 100, but not the easiest to make look nice...

https://codereview.chromium.org/721353002/diff/20001/tests/ResourceCacheTest.cpp
File tests/ResourceCacheTest.cpp (right):

https://codereview.chromium.org/721353002/diff/20001/tests/ResourceCacheTest....
tests/ResourceCacheTest.cpp:253: cache2->getResourceBytes());
align this

https://codereview.chromium.org/721353002/diff/20001/tests/ResourceCacheTest....
tests/ResourceCacheTest.cpp:258: REPORTER_ASSERT(reporter, 1 ==
cache2->getResourceCount());
I notice at this point we stop checking getBudgetedResource*(), would it just be
silly to check that the count stays at zero (aka doesn't drop to -1)?

[bsalomon, 2014-11-13 20:53:34]

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

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrGpuResource.cp...
src/gpu/GrGpuResource.cpp:25: , fScratchKey(GrResourceKey::NullScratchKey())  {
On 2014/11/13 20:47:31, egdaniel wrote:
> extra space

Done.

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrGpuResource.cp...
src/gpu/GrGpuResource.cpp:32: 
On 2014/11/13 20:47:32, egdaniel wrote:
> Extra \n added

Done.

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

https://codereview.chromium.org/721353002/diff/20001/src/gpu/GrResourceCache2...
src/gpu/GrResourceCache2.cpp:492: "\t\tEntry Count: current %d (%d budgeted, %d
wrapped, %d locked, %d scratch %.2g%% full), high %d\n",
On 2014/11/13 20:47:32, egdaniel wrote:
> over 100, but not the easiest to make look nice...

wrapped

https://codereview.chromium.org/721353002/diff/20001/tests/ResourceCacheTest.cpp
File tests/ResourceCacheTest.cpp (right):

https://codereview.chromium.org/721353002/diff/20001/tests/ResourceCacheTest....
tests/ResourceCacheTest.cpp:253: cache2->getResourceBytes());
On 2014/11/13 20:47:32, egdaniel wrote:
> align this

Done.

https://codereview.chromium.org/721353002/diff/20001/tests/ResourceCacheTest....
tests/ResourceCacheTest.cpp:258: REPORTER_ASSERT(reporter, 1 ==
cache2->getResourceCount());
On 2014/11/13 20:47:32, egdaniel wrote:
> I notice at this point we stop checking getBudgetedResource*(), would it just
be
> silly to check that the count stays at zero (aka doesn't drop to -1)?

Done.

[egdaniel, 2014-11-13 20:56:06]

lgtm

[robertphillips, 2014-11-17 13:49:20]

The CQ bit was checked by robertphillips@google.com

[robertphillips, 2014-11-17 13:49:21]

lgtm

[commit-bot: I haz the power, 2014-11-17 13:50:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/721353002/40001

[commit-bot: I haz the power, 2014-11-17 13:50:16]

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

[commit-bot: I haz the power, 2014-11-17 13:50:17]

Failed to apply patch for src/gpu/GrGpuResource.cpp:
While running git apply --index -3 -p1;
  error: patch failed: src/gpu/GrGpuResource.cpp:95
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: src/gpu/GrGpuResource.cpp: patch does not apply

Patch:       src/gpu/GrGpuResource.cpp
Index: src/gpu/GrGpuResource.cpp
diff --git a/src/gpu/GrGpuResource.cpp b/src/gpu/GrGpuResource.cpp
index
1f1d313d6b0e68ab4224edd5efed2867acff0534..8645aaad8e4f201ed9314229b90a8be2e3bcbcc4
100644
--- a/src/gpu/GrGpuResource.cpp
+++ b/src/gpu/GrGpuResource.cpp
@@ -22,12 +22,12 @@ GrGpuResource::GrGpuResource(GrGpu* gpu, bool isWrapped)
     : fGpu(gpu)
     , fGpuMemorySize(kInvalidGpuMemorySize)
     , fUniqueID(CreateUniqueID())
-    , fScratchKey(GrResourceKey::NullScratchKey())
-    , fContentKeySet(false) {
+    , fScratchKey(GrResourceKey::NullScratchKey()) {
     if (isWrapped) {
-        fFlags = kWrapped_FlagBit;
+        fFlags = kWrapped_Flag;
     } else {
-        fFlags = 0;
+        // By default all non-wrapped resources are budgeted.
+        fFlags = kBudgeted_Flag;
     }
 }
 
@@ -95,15 +95,15 @@ bool GrGpuResource::setContentKey(const GrResourceKey&
contentKey) {
         return false;
     }
     
-    if (fContentKeySet) {
+    if (fFlags & kContentKeySet_Flag) {
         return false;
     }
 
     fContentKey = contentKey;
-    fContentKeySet = true;
+    fFlags |= kContentKeySet_Flag;
 
     if (!get_resource_cache2(fGpu)->resourceAccess().didSetContentKey(this)) {
-        fContentKeySet = false;
+        fFlags &= ~kContentKeySet_Flag;
         return false;
     }
     return true;
@@ -134,3 +134,21 @@ uint32_t GrGpuResource::CreateUniqueID() {
     } while (id == SK_InvalidUniqueID);
     return id;
 }
+
+void GrGpuResource::setBudgeted(bool countsAgainstBudget) {
+    // Wrapped resources never count against the budget, nothing to do. No
point in changing the
+    // budgeting of destroyed resources.
+    if (this->isWrapped() || this->wasDestroyed()) {
+        return;
+    }
+
+    uint32_t oldFlags = fFlags;
+    if (countsAgainstBudget) {
+        fFlags |= kBudgeted_Flag;
+    } else {
+        fFlags &= ~kBudgeted_Flag;
+    }
+    if (fFlags != oldFlags) {
+       
get_resource_cache2(fGpu)->resourceAccess().didChangeBudgetStatus(this);
+    }
+}

[bsalomon, 2014-11-17 17:01:05]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-11-17 17:02:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/721353002/80001

[commit-bot: I haz the power, 2014-11-17 17:04:46]

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

[commit-bot: I haz the power, 2014-11-17 17:04:47]

Try jobs failed on following builders:
  Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-Cl...)
  Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Android-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)
  Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)

[bsalomon, 2014-11-17 17:13:41]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-11-17 17:14:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/721353002/100001

[commit-bot: I haz the power, 2014-11-17 17:15:26]

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

[commit-bot: I haz the power, 2014-11-17 17:15:26]

Try jobs failed on following builders:
  Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-Cl...)
  Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)

[bsalomon, 2014-11-17 17:22:12]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-11-17 17:23:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/721353002/120001

[commit-bot: I haz the power, 2014-11-17 17:33:32]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/84c8e62fad59f0e19b40ac718467f5b7884b431d