Only store resources in the GrResourceCache::fScratchMap that are available to be scratch.
Currently when we create a scratch resource, we immediately add it to
scratch map and it will stay there until we delete the resource. The one
exception to this is adding a unique key will remove a resource from
the scratch map. This means there are resources in the scratch map that
can't be returned when looking for a scratch because they are either
already in use by something else or their budget was changed to
unbudgeted. This means everything time we do a scratch lookup, even
after finding the list of resources that match a key, we still have to
iterate that list to see if we can use that resource or not.
The problem comes when we may have lots of resources that all match the
same key (think 1000s of identical buffers). Then the cost of iterating
this list starts to get very high.
This change makes it so only resources that can actively be used as a
scratch at that moment are stored in the scratch map. Thus when we find
a scratch resource we pull it out of the scratch map. When that resources
refs go back to zero it is added back to the scratch map. Similar removal
is also now used for changing a resource to and from budgeted.
Change-Id: I52b415d0e035dfc589f3d712be85799a56827bf0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367976
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp
index c1748a3..82ec5b5 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -155,12 +155,7 @@
fBudgetedHighWaterBytes = std::max(fBudgetedBytes, fBudgetedHighWaterBytes);
#endif
}
- if (resource->resourcePriv().getScratchKey().isValid() &&
- !resource->getUniqueKey().isValid()) {
- SkASSERT(!resource->resourcePriv().refsWrappedObjects());
- fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
- }
-
+ SkASSERT(!resource->cacheAccess().isUsableAsScratch());
this->purgeAsNeeded();
}
@@ -186,8 +181,7 @@
fBudgetedBytes, "free", fMaxBytes - fBudgetedBytes);
}
- if (resource->resourcePriv().getScratchKey().isValid() &&
- !resource->getUniqueKey().isValid()) {
+ if (resource->cacheAccess().isUsableAsScratch()) {
fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
}
if (resource->getUniqueKey().isValid()) {
@@ -285,13 +279,8 @@
AvailableForScratchUse() { }
bool operator()(const GrGpuResource* resource) const {
- SkASSERT(!resource->getUniqueKey().isValid() &&
- resource->resourcePriv().getScratchKey().isValid());
-
- // isScratch() also tests that the resource is budgeted.
- if (resource->internalHasRef() || !resource->cacheAccess().isScratch()) {
- return false;
- }
+ // Everything that is in the scratch map should be usable as a
+ // scratch resource.
return true;
}
};
@@ -301,6 +290,7 @@
GrGpuResource* resource = fScratchMap.find(scratchKey, AvailableForScratchUse());
if (resource) {
+ fScratchMap.remove(scratchKey, resource);
this->refAndMakeResourceMRU(resource);
this->validate();
}
@@ -310,7 +300,7 @@
void GrResourceCache::willRemoveScratchKey(const GrGpuResource* resource) {
ASSERT_SINGLE_OWNER
SkASSERT(resource->resourcePriv().getScratchKey().isValid());
- if (!resource->getUniqueKey().isValid()) {
+ if (resource->cacheAccess().isUsableAsScratch()) {
fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
}
}
@@ -324,7 +314,7 @@
fUniqueHash.remove(resource->getUniqueKey());
}
resource->cacheAccess().removeUniqueKey();
- if (resource->resourcePriv().getScratchKey().isValid()) {
+ if (resource->cacheAccess().isUsableAsScratch()) {
fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
}
@@ -361,8 +351,9 @@
SkASSERT(nullptr == fUniqueHash.find(resource->getUniqueKey()));
} else {
// 'resource' didn't have a valid unique key before so it is switching sides. Remove it
- // from the ScratchMap
- if (resource->resourcePriv().getScratchKey().isValid()) {
+ // from the ScratchMap. The isUsableAsScratch call depends on us not adding the new
+ // unique key until after this check.
+ if (resource->cacheAccess().isUsableAsScratch()) {
fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
}
}
@@ -397,7 +388,8 @@
this->validate();
}
-void GrResourceCache::notifyRefCntReachedZero(GrGpuResource* resource) {
+void GrResourceCache::notifyARefCntReachedZero(GrGpuResource* resource,
+ GrGpuResource::LastRemovedRef removedRef) {
ASSERT_SINGLE_OWNER
SkASSERT(resource);
SkASSERT(!resource->wasDestroyed());
@@ -406,6 +398,17 @@
// will be moved to the queue if it is newly purgeable.
SkASSERT(fNonpurgeableResources[*resource->cacheAccess().accessCacheIndex()] == resource);
+ if (removedRef == GrGpuResource::LastRemovedRef::kMainRef) {
+ if (resource->cacheAccess().isUsableAsScratch()) {
+ fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
+ }
+ }
+
+ if (resource->cacheAccess().hasRefOrCommandBufferUsage()) {
+ this->validate();
+ return;
+ }
+
#ifdef SK_DEBUG
// When the timestamp overflows validate() is called. validate() checks that resources in
// the nonpurgeable array are indeed not purgeable. However, the movement from the array to
@@ -489,6 +492,9 @@
!resource->cacheAccess().hasRefOrCommandBufferUsage()) {
++fNumBudgetedResourcesFlushWillMakePurgeable;
}
+ if (resource->cacheAccess().isUsableAsScratch()) {
+ fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
+ }
this->purgeAsNeeded();
} else {
SkASSERT(resource->resourcePriv().budgetedType() != GrBudgetedType::kUnbudgetedCacheable);
@@ -498,6 +504,10 @@
!resource->cacheAccess().hasRefOrCommandBufferUsage()) {
--fNumBudgetedResourcesFlushWillMakePurgeable;
}
+ if (!resource->cacheAccess().hasRef() && !resource->getUniqueKey().isValid() &&
+ resource->resourcePriv().getScratchKey().isValid()) {
+ fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
+ }
}
SkASSERT(wasPurgeable == resource->resourcePriv().isPurgeable());
TRACE_COUNTER2("skia.gpu.cache", "skia budget", "used",
@@ -857,29 +867,24 @@
const GrScratchKey& scratchKey = resource->resourcePriv().getScratchKey();
const GrUniqueKey& uniqueKey = resource->getUniqueKey();
- if (resource->cacheAccess().isScratch()) {
+ if (resource->cacheAccess().isUsableAsScratch()) {
SkASSERT(!uniqueKey.isValid());
+ SkASSERT(GrBudgetedType::kBudgeted == resource->resourcePriv().budgetedType());
+ SkASSERT(!resource->cacheAccess().hasRef());
++fScratch;
SkASSERT(fScratchMap->countForKey(scratchKey));
SkASSERT(!resource->resourcePriv().refsWrappedObjects());
} else if (scratchKey.isValid()) {
SkASSERT(GrBudgetedType::kBudgeted != resource->resourcePriv().budgetedType() ||
- uniqueKey.isValid());
- if (!uniqueKey.isValid()) {
- ++fCouldBeScratch;
- SkASSERT(fScratchMap->countForKey(scratchKey));
- }
+ uniqueKey.isValid() || resource->cacheAccess().hasRef());
SkASSERT(!resource->resourcePriv().refsWrappedObjects());
+ SkASSERT(!fScratchMap->has(resource, scratchKey));
}
if (uniqueKey.isValid()) {
++fContent;
SkASSERT(fUniqueHash->find(uniqueKey) == resource);
SkASSERT(GrBudgetedType::kBudgeted == resource->resourcePriv().budgetedType() ||
resource->resourcePriv().refsWrappedObjects());
-
- if (scratchKey.isValid()) {
- SkASSERT(!fScratchMap->has(resource, scratchKey));
- }
}
if (GrBudgetedType::kBudgeted == resource->resourcePriv().budgetedType()) {
@@ -892,8 +897,7 @@
{
int count = 0;
fScratchMap.foreach([&](const GrGpuResource& resource) {
- SkASSERT(resource.resourcePriv().getScratchKey().isValid());
- SkASSERT(!resource.getUniqueKey().isValid());
+ SkASSERT(resource.cacheAccess().isUsableAsScratch());
count++;
});
SkASSERT(count == fScratchMap.count());
@@ -941,7 +945,7 @@
SkASSERT(fBudgetedCount <= fBudgetedHighWaterCount);
#endif
SkASSERT(stats.fContent == fUniqueHash.count());
- SkASSERT(stats.fScratch + stats.fCouldBeScratch == fScratchMap.count());
+ SkASSERT(stats.fScratch == fScratchMap.count());
// This assertion is not currently valid because we can be in recursive notifyCntReachedZero()
// calls. This will be fixed when subresource registration is explicit.