Revert "Only store resources in the GrResourceCache::fScratchMap that are available to be scratch."
This reverts commit 1a2326363a724ff9512b04455f7004ef810bc02f.
Reason for revert: breaking win10 quadro400 perf bot on vk and vkmsaa
Original change's description:
> 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>
TBR=egdaniel@google.com,bsalomon@google.com
Change-Id: I1e57e10e75f930adfecb0e4167c1d6269798c893
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368236
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp
index 82ec5b5..c1748a3 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -155,7 +155,12 @@
fBudgetedHighWaterBytes = std::max(fBudgetedBytes, fBudgetedHighWaterBytes);
#endif
}
- SkASSERT(!resource->cacheAccess().isUsableAsScratch());
+ if (resource->resourcePriv().getScratchKey().isValid() &&
+ !resource->getUniqueKey().isValid()) {
+ SkASSERT(!resource->resourcePriv().refsWrappedObjects());
+ fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
+ }
+
this->purgeAsNeeded();
}
@@ -181,7 +186,8 @@
fBudgetedBytes, "free", fMaxBytes - fBudgetedBytes);
}
- if (resource->cacheAccess().isUsableAsScratch()) {
+ if (resource->resourcePriv().getScratchKey().isValid() &&
+ !resource->getUniqueKey().isValid()) {
fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
}
if (resource->getUniqueKey().isValid()) {
@@ -279,8 +285,13 @@
AvailableForScratchUse() { }
bool operator()(const GrGpuResource* resource) const {
- // Everything that is in the scratch map should be usable as a
- // scratch resource.
+ SkASSERT(!resource->getUniqueKey().isValid() &&
+ resource->resourcePriv().getScratchKey().isValid());
+
+ // isScratch() also tests that the resource is budgeted.
+ if (resource->internalHasRef() || !resource->cacheAccess().isScratch()) {
+ return false;
+ }
return true;
}
};
@@ -290,7 +301,6 @@
GrGpuResource* resource = fScratchMap.find(scratchKey, AvailableForScratchUse());
if (resource) {
- fScratchMap.remove(scratchKey, resource);
this->refAndMakeResourceMRU(resource);
this->validate();
}
@@ -300,7 +310,7 @@
void GrResourceCache::willRemoveScratchKey(const GrGpuResource* resource) {
ASSERT_SINGLE_OWNER
SkASSERT(resource->resourcePriv().getScratchKey().isValid());
- if (resource->cacheAccess().isUsableAsScratch()) {
+ if (!resource->getUniqueKey().isValid()) {
fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
}
}
@@ -314,7 +324,7 @@
fUniqueHash.remove(resource->getUniqueKey());
}
resource->cacheAccess().removeUniqueKey();
- if (resource->cacheAccess().isUsableAsScratch()) {
+ if (resource->resourcePriv().getScratchKey().isValid()) {
fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
}
@@ -351,9 +361,8 @@
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. The isUsableAsScratch call depends on us not adding the new
- // unique key until after this check.
- if (resource->cacheAccess().isUsableAsScratch()) {
+ // from the ScratchMap
+ if (resource->resourcePriv().getScratchKey().isValid()) {
fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
}
}
@@ -388,8 +397,7 @@
this->validate();
}
-void GrResourceCache::notifyARefCntReachedZero(GrGpuResource* resource,
- GrGpuResource::LastRemovedRef removedRef) {
+void GrResourceCache::notifyRefCntReachedZero(GrGpuResource* resource) {
ASSERT_SINGLE_OWNER
SkASSERT(resource);
SkASSERT(!resource->wasDestroyed());
@@ -398,17 +406,6 @@
// 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
@@ -492,9 +489,6 @@
!resource->cacheAccess().hasRefOrCommandBufferUsage()) {
++fNumBudgetedResourcesFlushWillMakePurgeable;
}
- if (resource->cacheAccess().isUsableAsScratch()) {
- fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
- }
this->purgeAsNeeded();
} else {
SkASSERT(resource->resourcePriv().budgetedType() != GrBudgetedType::kUnbudgetedCacheable);
@@ -504,10 +498,6 @@
!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",
@@ -867,24 +857,29 @@
const GrScratchKey& scratchKey = resource->resourcePriv().getScratchKey();
const GrUniqueKey& uniqueKey = resource->getUniqueKey();
- if (resource->cacheAccess().isUsableAsScratch()) {
+ if (resource->cacheAccess().isScratch()) {
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() || resource->cacheAccess().hasRef());
+ uniqueKey.isValid());
+ if (!uniqueKey.isValid()) {
+ ++fCouldBeScratch;
+ SkASSERT(fScratchMap->countForKey(scratchKey));
+ }
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()) {
@@ -897,7 +892,8 @@
{
int count = 0;
fScratchMap.foreach([&](const GrGpuResource& resource) {
- SkASSERT(resource.cacheAccess().isUsableAsScratch());
+ SkASSERT(resource.resourcePriv().getScratchKey().isValid());
+ SkASSERT(!resource.getUniqueKey().isValid());
count++;
});
SkASSERT(count == fScratchMap.count());
@@ -945,7 +941,7 @@
SkASSERT(fBudgetedCount <= fBudgetedHighWaterCount);
#endif
SkASSERT(stats.fContent == fUniqueHash.count());
- SkASSERT(stats.fScratch == fScratchMap.count());
+ SkASSERT(stats.fScratch + stats.fCouldBeScratch == 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.