Revert of Change SkResourceCache to take a Visitor inside its find(). (patchset #2 id:20001 of https://codereview.chromium.org/567393002/)
Reason for revert:
crashes on android bots, haven't diagnosed yet
Original issue's description:
> Change SkResourceCache to take a Visitor inside its find().
>
> This simplifies the API/contract, in that there are not any exposed
> lock/unlock scopes.
>
>
> patch from issue 572573002
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/dee6a8e67db39fcbde2b3bb09be1d088ebb9db8a
R=mtklein@google.com, danakj@chromium.org
TBR=danakj@chromium.org, mtklein@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Author: reed@google.com
Review URL: https://codereview.chromium.org/569303002
diff --git a/src/core/SkResourceCache.cpp b/src/core/SkResourceCache.cpp
index 85b5b90..68248f5 100644
--- a/src/core/SkResourceCache.cpp
+++ b/src/core/SkResourceCache.cpp
@@ -187,34 +187,80 @@
////////////////////////////////////////////////////////////////////////////////
-bool SkResourceCache::find(const Key& key, VisitorProc visitor, void* context) {
+const SkResourceCache::Rec* SkResourceCache::findAndLock(const Key& key) {
Rec* rec = fHash->find(key);
if (rec) {
- if (visitor(*rec, context)) {
- this->moveToHead(rec); // for our LRU
- return true;
- } else {
- this->remove(rec); // stale
- return false;
- }
+ this->moveToHead(rec); // for our LRU
+ rec->fLockCount += 1;
}
- return false;
+ return rec;
+}
+
+const SkResourceCache::Rec* SkResourceCache::addAndLock(Rec* rec) {
+ SkASSERT(rec);
+ // See if we already have this key (racy inserts, etc.)
+ const Rec* existing = this->findAndLock(rec->getKey());
+ if (existing) {
+ SkDELETE(rec);
+ return existing;
+ }
+
+ this->addToHead(rec);
+ SkASSERT(1 == rec->fLockCount);
+ fHash->add(rec);
+ // We may (now) be overbudget, so see if we need to purge something.
+ this->purgeAsNeeded();
+ return rec;
}
void SkResourceCache::add(Rec* rec) {
SkASSERT(rec);
// See if we already have this key (racy inserts, etc.)
- Rec* existing = fHash->find(rec->getKey());
+ const Rec* existing = this->findAndLock(rec->getKey());
if (existing) {
SkDELETE(rec);
+ this->unlock(existing);
return;
}
this->addToHead(rec);
+ SkASSERT(1 == rec->fLockCount);
fHash->add(rec);
+ this->unlock(rec);
+}
+
+void SkResourceCache::unlock(SkResourceCache::ID id) {
+ SkASSERT(id);
+
+#ifdef SK_DEBUG
+ {
+ bool found = false;
+ Rec* rec = fHead;
+ while (rec != NULL) {
+ if (rec == id) {
+ found = true;
+ break;
+ }
+ rec = rec->fNext;
+ }
+ SkASSERT(found);
+ }
+#endif
+ const Rec* rec = id;
+ SkASSERT(rec->fLockCount > 0);
+ // We're under our lock, and we're the only possible mutator, so unconsting is fine.
+ const_cast<Rec*>(rec)->fLockCount -= 1;
+
+ // we may have been over-budget, but now have released something, so check
+ // if we should purge.
+ if (0 == rec->fLockCount) {
+ this->purgeAsNeeded();
+ }
}
void SkResourceCache::remove(Rec* rec) {
+ SkASSERT(0 == rec->fLockCount);
+
size_t used = rec->bytesUsed();
SkASSERT(used <= fTotalBytesUsed);
@@ -246,7 +292,9 @@
}
Rec* prev = rec->fPrev;
- this->remove(rec);
+ if (0 == rec->fLockCount) {
+ this->remove(rec);
+ }
rec = prev;
}
}
@@ -369,8 +417,16 @@
void SkResourceCache::dump() const {
this->validate();
- SkDebugf("SkResourceCache: count=%d bytes=%d %s\n",
- fCount, fTotalBytesUsed, fDiscardableFactory ? "discardable" : "malloc");
+ const Rec* rec = fHead;
+ int locked = 0;
+ while (rec) {
+ locked += rec->fLockCount > 0;
+ rec = rec->fNext;
+ }
+
+ SkDebugf("SkResourceCache: count=%d bytes=%d locked=%d %s\n",
+ fCount, fTotalBytesUsed, locked,
+ fDiscardableFactory ? "discardable" : "malloc");
}
size_t SkResourceCache::setSingleAllocationByteLimit(size_t newLimit) {
@@ -414,6 +470,35 @@
return gResourceCache;
}
+void SkResourceCache::Unlock(SkResourceCache::ID id) {
+ SkAutoMutexAcquire am(gMutex);
+ get_cache()->unlock(id);
+
+// get_cache()->dump();
+}
+
+void SkResourceCache::Remove(SkResourceCache::ID id) {
+ SkAutoMutexAcquire am(gMutex);
+ SkASSERT(id);
+
+#ifdef SK_DEBUG
+ {
+ bool found = false;
+ Rec* rec = get_cache()->fHead;
+ while (rec != NULL) {
+ if (rec == id) {
+ found = true;
+ break;
+ }
+ rec = rec->fNext;
+ }
+ SkASSERT(found);
+ }
+#endif
+ const Rec* rec = id;
+ get_cache()->remove(const_cast<Rec*>(rec));
+}
+
size_t SkResourceCache::GetTotalBytesUsed() {
SkAutoMutexAcquire am(gMutex);
return get_cache()->getTotalBytesUsed();
@@ -454,9 +539,14 @@
return get_cache()->purgeAll();
}
-bool SkResourceCache::Find(const Key& key, VisitorProc visitor, void* context) {
+const SkResourceCache::Rec* SkResourceCache::FindAndLock(const Key& key) {
SkAutoMutexAcquire am(gMutex);
- return get_cache()->find(key, visitor, context);
+ return get_cache()->findAndLock(key);
+}
+
+const SkResourceCache::Rec* SkResourceCache::AddAndLock(Rec* rec) {
+ SkAutoMutexAcquire am(gMutex);
+ return get_cache()->addAndLock(rec);
}
void SkResourceCache::Add(Rec* rec) {