Don't store resources with a unique key in GrResourceCache's fScratchMap
The reasoning here is that resources with a unique key are never selected from fScratchMap and just clog up the search for an available resource.
This knocks a 200x loop over the SVGbouncingrects case from 264ms down to 164ms.
BUG=603969
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2008083002
Review-Url: https://codereview.chromium.org/2008083002
diff --git a/src/core/SkTMultiMap.h b/src/core/SkTMultiMap.h
index 4c8683c..dc521de 100644
--- a/src/core/SkTMultiMap.h
+++ b/src/core/SkTMultiMap.h
@@ -102,6 +102,51 @@
int count() const { return fCount; }
#ifdef SK_DEBUG
+ class ConstIter {
+ public:
+ explicit ConstIter(const SkTMultiMap* mmap)
+ : fIter(&(mmap->fHash))
+ , fList(nullptr) {
+ if (!fIter.done()) {
+ fList = &(*fIter);
+ }
+ }
+
+ bool done() const {
+ return fIter.done();
+ }
+
+ const T* operator*() {
+ SkASSERT(fList);
+ return fList->fValue;
+ }
+
+ void operator++() {
+ if (fList) {
+ fList = fList->fNext;
+ }
+ if (!fList) {
+ ++fIter;
+ if (!fIter.done()) {
+ fList = &(*fIter);
+ }
+ }
+ }
+
+ private:
+ typename SkTDynamicHash<ValueList, Key>::ConstIter fIter;
+ const ValueList* fList;
+ };
+
+ bool has(const T* value, const Key& key) const {
+ for (ValueList* list = fHash.find(key); list; list = list->fNext) {
+ if (list->fValue == value) {
+ return true;
+ }
+ }
+ return false;
+ }
+
// This is not particularly fast and only used for validation, so debug only.
int countForKey(const Key& key) const {
int count = 0;
diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp
index 1d32c23..db97e35 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -145,7 +145,8 @@
fBudgetedHighWaterBytes = SkTMax(fBudgetedBytes, fBudgetedHighWaterBytes);
#endif
}
- if (resource->resourcePriv().getScratchKey().isValid()) {
+ if (resource->resourcePriv().getScratchKey().isValid() &&
+ !resource->getUniqueKey().isValid()) {
SkASSERT(!resource->resourcePriv().refsWrappedObjects());
fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
}
@@ -173,7 +174,8 @@
fBudgetedBytes, "free", fMaxBytes - fBudgetedBytes);
}
- if (resource->resourcePriv().getScratchKey().isValid()) {
+ if (resource->resourcePriv().getScratchKey().isValid() &&
+ !resource->getUniqueKey().isValid()) {
fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
}
if (resource->getUniqueKey().isValid()) {
@@ -235,6 +237,8 @@
AvailableForScratchUse(bool rejectPendingIO) : fRejectPendingIO(rejectPendingIO) { }
bool operator()(const GrGpuResource* resource) const {
+ SkASSERT(!resource->getUniqueKey().isValid() &&
+ resource->resourcePriv().getScratchKey().isValid());
if (resource->internalHasRef() || !resource->cacheAccess().isScratch()) {
return false;
}
@@ -279,7 +283,9 @@
void GrResourceCache::willRemoveScratchKey(const GrGpuResource* resource) {
SkASSERT(resource->resourcePriv().getScratchKey().isValid());
- fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
+ if (!resource->getUniqueKey().isValid()) {
+ fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
+ }
}
void GrResourceCache::removeUniqueKey(GrGpuResource* resource) {
@@ -290,6 +296,11 @@
fUniqueHash.remove(resource->getUniqueKey());
}
resource->cacheAccess().removeUniqueKey();
+
+ if (resource->resourcePriv().getScratchKey().isValid()) {
+ fScratchMap.insert(resource->resourcePriv().getScratchKey(), resource);
+ }
+
this->validate();
}
@@ -297,15 +308,21 @@
SkASSERT(resource);
SkASSERT(this->isInCache(resource));
- // Remove the entry for this resource if it already has a unique key.
- if (resource->getUniqueKey().isValid()) {
- SkASSERT(resource == fUniqueHash.find(resource->getUniqueKey()));
- fUniqueHash.remove(resource->getUniqueKey());
- SkASSERT(nullptr == fUniqueHash.find(resource->getUniqueKey()));
- }
-
// If another resource has the new key, remove its key then install the key on this resource.
if (newKey.isValid()) {
+ // Remove the entry for this resource if it already has a unique key.
+ if (resource->getUniqueKey().isValid()) {
+ SkASSERT(resource == fUniqueHash.find(resource->getUniqueKey()));
+ fUniqueHash.remove(resource->getUniqueKey());
+ 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()) {
+ fScratchMap.remove(resource->resourcePriv().getScratchKey(), resource);
+ }
+ }
+
if (GrGpuResource* old = fUniqueHash.find(newKey)) {
// If the old resource using the key is purgeable and is unreachable, then remove it.
if (!old->resourcePriv().getScratchKey().isValid() && old->isPurgeable()) {
@@ -314,15 +331,14 @@
SkDEBUGCODE(resource->cacheAccess().removeUniqueKey();)
old->cacheAccess().release();
} else {
- fUniqueHash.remove(newKey);
- old->cacheAccess().removeUniqueKey();
+ this->removeUniqueKey(old);
}
}
SkASSERT(nullptr == fUniqueHash.find(newKey));
resource->cacheAccess().setUniqueKey(newKey);
fUniqueHash.add(resource);
} else {
- resource->cacheAccess().removeUniqueKey();
+ this->removeUniqueKey(resource);
}
this->validate();
@@ -657,24 +673,32 @@
++fLocked;
}
+ const GrScratchKey& scratchKey = resource->resourcePriv().getScratchKey();
+ const GrUniqueKey& uniqueKey = resource->getUniqueKey();
+
if (resource->cacheAccess().isScratch()) {
- SkASSERT(!resource->getUniqueKey().isValid());
+ SkASSERT(!uniqueKey.isValid());
++fScratch;
- SkASSERT(fScratchMap->countForKey(resource->resourcePriv().getScratchKey()));
+ SkASSERT(fScratchMap->countForKey(scratchKey));
SkASSERT(!resource->resourcePriv().refsWrappedObjects());
- } else if (resource->resourcePriv().getScratchKey().isValid()) {
+ } else if (scratchKey.isValid()) {
SkASSERT(SkBudgeted::kNo == resource->resourcePriv().isBudgeted() ||
- resource->getUniqueKey().isValid());
- ++fCouldBeScratch;
- SkASSERT(fScratchMap->countForKey(resource->resourcePriv().getScratchKey()));
+ uniqueKey.isValid());
+ if (!uniqueKey.isValid()) {
+ ++fCouldBeScratch;
+ SkASSERT(fScratchMap->countForKey(scratchKey));
+ }
SkASSERT(!resource->resourcePriv().refsWrappedObjects());
}
- const GrUniqueKey& uniqueKey = resource->getUniqueKey();
if (uniqueKey.isValid()) {
++fContent;
SkASSERT(fUniqueHash->find(uniqueKey) == resource);
SkASSERT(!resource->resourcePriv().refsWrappedObjects());
SkASSERT(SkBudgeted::kYes == resource->resourcePriv().isBudgeted());
+
+ if (scratchKey.isValid()) {
+ SkASSERT(!fScratchMap->has(resource, scratchKey));
+ }
}
if (SkBudgeted::kYes == resource->resourcePriv().isBudgeted()) {
@@ -684,6 +708,19 @@
}
};
+ {
+ ScratchMap::ConstIter iter(&fScratchMap);
+
+ int count = 0;
+ for ( ; !iter.done(); ++iter) {
+ const GrGpuResource* resource = *iter;
+ SkASSERT(resource->resourcePriv().getScratchKey().isValid());
+ SkASSERT(!resource->getUniqueKey().isValid());
+ count++;
+ }
+ SkASSERT(count == fScratchMap.count()); // ensure the iterator is working correctly
+ }
+
Stats stats(this);
for (int i = 0; i < fNonpurgeableResources.count(); ++i) {