Change SkResourceCache to take a Visitor inside its find().

src/core/SkResourceCache.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkChecksum.h"
 #include "SkResourceCache.h"
 #include "SkMipMap.h"
@@ skipping 169 matching lines @@
     while (rec) {
         Rec* next = rec->fNext;
         SkDELETE(rec);
         rec = next;
     }
     delete fHash;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
-const SkResourceCache::Rec* SkResourceCache::findAndLock(const Key& key) {
+bool SkResourceCache::find(const Key& key, VisitorProc visitor, void* context) {
     Rec* rec = fHash->find(key);
     if (rec) {
-        this->moveToHead(rec);  // for our LRU
-        rec->fLockCount += 1;
+        if (visitor(*rec, context)) {
+            this->moveToHead(rec);  // for our LRU
+            return true;
+        } else {
+            this->remove(rec);  // stale
+            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;
+    return false;
 }
 
 void SkResourceCache::add(Rec* rec) {
     SkASSERT(rec);
     // See if we already have this key (racy inserts, etc.)
-    const Rec* existing = this->findAndLock(rec->getKey());
+    Rec* existing = fHash->find(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();

[reed1, 2014/09/15 18:15:41]

The bug in the new CL was that we forgot to "purge" after the Add -- hence we
could overrun our budget!

[danakj, 2014/09/15 18:21:16]

Where is the purge in this CL then? I don't see one being added anywhere.

[reed1, 2014/09/15 18:22:49]

Just wanted to post the original that I'm reverting.
patch #2 has the purge-fix.

-    }
 }
 
 void SkResourceCache::remove(Rec* rec) {
-    SkASSERT(0 == rec->fLockCount);

[danakj, 2014/09/15 18:21:17]

This seems reasonable to keep, while the fLockCount still exists?

-
     size_t used = rec->bytesUsed();
     SkASSERT(used <= fTotalBytesUsed);
 
     this->detach(rec);
     fHash->remove(rec->getKey());
 
     SkDELETE(rec);
 
     fTotalBytesUsed -= used;
     fCount -= 1;
@@ skipping 11 matching lines @@
         byteLimit = fTotalByteLimit;
     }
 
     Rec* rec = fTail;
     while (rec) {
         if (!forcePurge && fTotalBytesUsed < byteLimit && fCount < countLimit) {
             break;
         }
 
         Rec* prev = rec->fPrev;
-        if (0 == rec->fLockCount) {
-            this->remove(rec);
-        }
+        this->remove(rec);
         rec = prev;
     }
 }
 
 size_t SkResourceCache::setTotalByteLimit(size_t newLimit) {
     size_t prevLimit = fTotalByteLimit;
     fTotalByteLimit = newLimit;
     if (newLimit < prevLimit) {
         this->purgeAsNeeded();
     }
@@ skipping 102 matching lines @@
     }
 
     SkASSERT(0 == count);
     SkASSERT(0 == used);
 }
 #endif
 
 void SkResourceCache::dump() const {
     this->validate();
 
-    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");
+    SkDebugf("SkResourceCache: count=%d bytes=%d %s\n",
+             fCount, fTotalBytesUsed, fDiscardableFactory ? "discardable" : "malloc");
 }
 
 size_t SkResourceCache::setSingleAllocationByteLimit(size_t newLimit) {
     size_t oldLimit = fSingleAllocationByteLimit;
     fSingleAllocationByteLimit = newLimit;
     return oldLimit;
 }
 
 size_t SkResourceCache::getSingleAllocationByteLimit() const {
     return fSingleAllocationByteLimit;
@@ skipping 23 matching lines @@
 #ifdef SK_USE_DISCARDABLE_SCALEDIMAGECACHE
         gResourceCache = SkNEW_ARGS(SkResourceCache, (SkDiscardableMemory::Create));
 #else
         gResourceCache = SkNEW_ARGS(SkResourceCache, (SK_DEFAULT_IMAGE_CACHE_LIMIT));
 #endif
         atexit(cleanup_gResourceCache);
     }
     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();
 }
 
 size_t SkResourceCache::GetTotalByteLimit() {
     SkAutoMutexAcquire am(gMutex);
     return get_cache()->getTotalByteLimit();
 }
 
@@ skipping 20 matching lines @@
 size_t SkResourceCache::GetSingleAllocationByteLimit() {
     SkAutoMutexAcquire am(gMutex);
     return get_cache()->getSingleAllocationByteLimit();
 }
 
 void SkResourceCache::PurgeAll() {
     SkAutoMutexAcquire am(gMutex);
     return get_cache()->purgeAll();
 }
 
-const SkResourceCache::Rec* SkResourceCache::FindAndLock(const Key& key) {
+bool SkResourceCache::Find(const Key& key, VisitorProc visitor, void* context) {
     SkAutoMutexAcquire am(gMutex);
-    return get_cache()->findAndLock(key);
-}
-
-const SkResourceCache::Rec* SkResourceCache::AddAndLock(Rec* rec) {
-    SkAutoMutexAcquire am(gMutex);
-    return get_cache()->addAndLock(rec);
+    return get_cache()->find(key, visitor, context);
 }
 
 void SkResourceCache::Add(Rec* rec) {
     SkAutoMutexAcquire am(gMutex);
     get_cache()->add(rec);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "SkGraphics.h"
@@ skipping 15 matching lines @@
 }
 
 size_t SkGraphics::SetResourceCacheSingleAllocationByteLimit(size_t newLimit) {
     return SkResourceCache::SetSingleAllocationByteLimit(newLimit);
 }
 
 void SkGraphics::PurgeResourceCache() {
     return SkResourceCache::PurgeAll();
 }