AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

Browse Source 
This simplifies the API/contract, in that there are not any exposed
lock/unlock scopes.

patch from issue 572573002

BUG=skia:
R=mtklein@google.com, danakj@chromium.org

Author: reed@google.com

Review URL: https://codereview.chromium.org/567393002
This commit is contained in:
reed 2014-09-15 06:44:47 -07:00 committed by Commit bot
parent 5087b2c067
commit dee6a8e67d
5 changed files with 98 additions and 177 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
bench/ImageCacheBench.cpp
View File

@ -27,6 +27,10 @@ struct TestRec : public SkResourceCache::Rec {
virtual const Key& getKey() const SK_OVERRIDE { return fKey; }
virtual size_t bytesUsed() const SK_OVERRIDE { return sizeof(fKey) + sizeof(fValue); }
static bool Visitor(const SkResourceCache::Rec&, void*) {
return true;
}
};
}
@ -41,7 +45,7 @@ public:
void populateCache() {
for (int i = 0; i < CACHE_COUNT; ++i) {
fCache.unlock(fCache.addAndLock(SkNEW_ARGS(TestRec, (TestKey(i), i))));
fCache.add(SkNEW_ARGS(TestRec, (TestKey(i), i)));
}
}
@ -58,8 +62,8 @@ protected:
TestKey key(-1);
// search for a miss (-1)
for (int i = 0; i < loops; ++i) {
SkDEBUGCODE(SkResourceCache::ID id =) fCache.findAndLock(key);
SkASSERT(NULL == id);
SkDEBUGCODE(bool found =) fCache.find(key, TestRec::Visitor, NULL);
SkASSERT(!found);
}
}

43
src/core/SkBitmapCache.cpp
View File

@ -59,25 +59,16 @@ struct BitmapRec : public SkResourceCache::Rec {
virtual const Key& getKey() const SK_OVERRIDE { return fKey; }
virtual size_t bytesUsed() const SK_OVERRIDE { return sizeof(fKey) + fBitmap.getSize(); }
};
static bool find_and_return(const BitmapKey& key, SkBitmap* result) {
const BitmapRec* rec = (BitmapRec*)SkResourceCache::FindAndLock(key);
if (rec) {
*result = rec->fBitmap;
SkResourceCache::Unlock(rec);
static bool Visitor(const SkResourceCache::Rec& baseRec, void* contextBitmap) {
const BitmapRec& rec = static_cast<const BitmapRec&>(baseRec);
SkBitmap* result = (SkBitmap*)contextBitmap;
*result = rec.fBitmap;
result->lockPixels();
if (result->getPixels()) {
return true;
}
SkResourceCache::Remove(rec);
result->reset();
// fall-through to false
}
return false;
return SkToBool(result->getPixels());
}
};
bool SkBitmapCache::Find(const SkBitmap& src, SkScalar invScaleX, SkScalar invScaleY,
SkBitmap* result) {
@ -86,7 +77,7 @@ bool SkBitmapCache::Find(const SkBitmap& src, SkScalar invScaleX, SkScalar invSc
return false;
}
BitmapKey key(src.getGenerationID(), invScaleX, invScaleY, get_bounds_from_bitmap(src));
return find_and_return(key, result);
return SkResourceCache::Find(key, BitmapRec::Visitor, result);
}
void SkBitmapCache::Add(const SkBitmap& src, SkScalar invScaleX, SkScalar invScaleY,
@ -102,7 +93,7 @@ void SkBitmapCache::Add(const SkBitmap& src, SkScalar invScaleX, SkScalar invSca
bool SkBitmapCache::Find(uint32_t genID, const SkIRect& subset, SkBitmap* result) {
BitmapKey key(genID, SK_Scalar1, SK_Scalar1, subset);
return find_and_return(key, result);
return SkResourceCache::Find(key, BitmapRec::Visitor, result);
}
bool SkBitmapCache::Add(uint32_t genID, const SkIRect& subset, const SkBitmap& result) {
@ -138,16 +129,22 @@ struct MipMapRec : public SkResourceCache::Rec {
virtual const Key& getKey() const SK_OVERRIDE { return fKey; }
virtual size_t bytesUsed() const SK_OVERRIDE { return sizeof(fKey) + fMipMap->getSize(); }
};
static bool Visitor(const SkResourceCache::Rec& baseRec, void* contextMip) {
const MipMapRec& rec = static_cast<const MipMapRec&>(baseRec);
const SkMipMap** result = (const SkMipMap**)contextMip;
*result = SkRef(rec.fMipMap);
// mipmaps don't use the custom allocator yet, so we don't need to check pixels
return true;
}
};
const SkMipMap* SkMipMapCache::FindAndRef(const SkBitmap& src) {
BitmapKey key(src.getGenerationID(), 0, 0, get_bounds_from_bitmap(src));
const MipMapRec* rec = (MipMapRec*)SkResourceCache::FindAndLock(key);
const SkMipMap* result = NULL;
if (rec) {
result = SkRef(rec->fMipMap);
SkResourceCache::Unlock(rec);
const SkMipMap* result;
if (!SkResourceCache::Find(key, MipMapRec::Visitor, &result)) {
result = NULL;
}
return result;
}

114
src/core/SkResourceCache.cpp
View File

@ -187,80 +187,34 @@ SkResourceCache::~SkResourceCache() {
////////////////////////////////////////////////////////////////////////////////
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) {
if (visitor(*rec, context)) {
this->moveToHead(rec); // for our LRU
rec->fLockCount += 1;
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();
}
}
void SkResourceCache::remove(Rec* rec) {
SkASSERT(0 == rec->fLockCount);
size_t used = rec->bytesUsed();
SkASSERT(used <= fTotalBytesUsed);
@ -292,9 +246,7 @@ void SkResourceCache::purgeAsNeeded(bool forcePurge) {
}
Rec* prev = rec->fPrev;
if (0 == rec->fLockCount) {
this->remove(rec);
}
rec = prev;
}
}
@ -417,16 +369,8 @@ void SkResourceCache::validate() const {
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) {
@ -470,35 +414,6 @@ static SkResourceCache* get_cache() {
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();
@ -539,14 +454,9 @@ void SkResourceCache::PurgeAll() {
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) {

49
src/core/SkResourceCache.h
View File

@ -60,7 +60,7 @@ public:
struct Rec {
typedef SkResourceCache::Key Key;
Rec() : fLockCount(1) {}
Rec() {}
virtual ~Rec() {}
uint32_t getHash() const { return this->getKey().hash(); }
@ -75,13 +75,24 @@ public:
private:
Rec* fNext;
Rec* fPrev;
int32_t fLockCount;
friend class SkResourceCache;
};
typedef const Rec* ID;
/**
* Callback function for find(). If called, the cache will have found a match for the
* specified Key, and will pass in the corresponding Rec, along with a caller-specified
* context. The function can read the data in Rec, and copy whatever it likes into context
* (casting context to whatever it really is).
*
* The return value determines what the cache will do with the Rec. If the function returns
* true, then the Rec is considered "valid". If false is returned, the Rec will be considered
* "stale" and will be purged from the cache.
*/
typedef bool (*VisitorProc)(const Rec&, void* context);
/**
* Returns a locked/pinned SkDiscardableMemory instance for the specified
* number of bytes, or NULL on failure.
@ -93,11 +104,17 @@ public:
* instance of this cache.
*/
static const Rec* FindAndLock(const Key& key);
static const Rec* AddAndLock(Rec*);
/**
* Returns true if the visitor was called on a matching Key, and the visitor returned true.
*
* Find() will search the cache for the specified Key. If no match is found, return false and
* do not call the VisitorProc. If a match is found, return whatever the visitor returns.
* Its return value is interpreted to mean:
* true : Rec is valid
* false : Rec is "stale" -- the cache will purge it.
*/
static bool Find(const Key& key, VisitorProc, void* context);
static void Add(Rec*);
static void Unlock(ID);
static void Remove(ID);
static size_t GetTotalBytesUsed();
static size_t GetTotalByteLimit();
@ -139,18 +156,17 @@ public:
explicit SkResourceCache(size_t byteLimit);
~SkResourceCache();
const Rec* findAndLock(const Key& key);
const Rec* addAndLock(Rec*);
void add(Rec*);
void remove(Rec*);
/**
* Given a non-null ID ptr returned by either findAndLock or addAndLock,
* this releases the associated resources to be available to be purged
* if needed. After this, the cached bitmap should no longer be
* referenced by the caller.
* Returns true if the visitor was called on a matching Key, and the visitor returned true.
*
* find() will search the cache for the specified Key. If no match is found, return false and
* do not call the VisitorProc. If a match is found, return whatever the visitor returns.
* Its return value is interpreted to mean:
* true : Rec is valid
* false : Rec is "stale" -- the cache will purge it.
*/
void unlock(ID);
bool find(const Key&, VisitorProc, void* context);
void add(Rec*);
size_t getTotalBytesUsed() const { return fTotalBytesUsed; }
size_t getTotalByteLimit() const { return fTotalByteLimit; }
@ -202,6 +218,7 @@ private:
void moveToHead(Rec*);
void addToHead(Rec*);
void detach(Rec*);
void remove(Rec*);
void init(); // called by constructors

53
tests/ImageCacheTest.cpp
View File

@ -27,49 +27,46 @@ struct TestingRec : public SkResourceCache::Rec {
virtual const Key& getKey() const SK_OVERRIDE { return fKey; }
virtual size_t bytesUsed() const SK_OVERRIDE { return sizeof(fKey) + sizeof(fValue); }
static bool Visitor(const SkResourceCache::Rec& baseRec, void* context) {
const TestingRec& rec = static_cast<const TestingRec&>(baseRec);
intptr_t* result = (intptr_t*)context;
*result = rec.fValue;
return true;
}
};
}
static const int COUNT = 10;
static const int DIM = 256;
static void test_cache(skiatest::Reporter* reporter, SkResourceCache& cache,
bool testPurge) {
SkResourceCache::ID id;
static void test_cache(skiatest::Reporter* reporter, SkResourceCache& cache, bool testPurge) {
for (int i = 0; i < COUNT; ++i) {
TestingKey key(i);
intptr_t value = -1;
const TestingRec* rec = (const TestingRec*)cache.findAndLock(key);
REPORTER_ASSERT(reporter, NULL == rec);
REPORTER_ASSERT(reporter, !cache.find(key, TestingRec::Visitor, &value));
REPORTER_ASSERT(reporter, -1 == value);
TestingRec* newRec = SkNEW_ARGS(TestingRec, (key, i));
const TestingRec* addedRec = (const TestingRec*)cache.addAndLock(newRec);
REPORTER_ASSERT(reporter, addedRec);
cache.add(SkNEW_ARGS(TestingRec, (key, i)));
const TestingRec* foundRec = (const TestingRec*)cache.findAndLock(key);
REPORTER_ASSERT(reporter, foundRec == addedRec);
REPORTER_ASSERT(reporter, foundRec->fValue == i);
cache.unlock(foundRec);
cache.unlock(addedRec);
REPORTER_ASSERT(reporter, cache.find(key, TestingRec::Visitor, &value));
REPORTER_ASSERT(reporter, i == value);
}
if (testPurge) {
// stress test, should trigger purges
for (size_t i = 0; i < COUNT * 100; ++i) {
TestingKey key(i);
SkResourceCache::ID id = cache.addAndLock(SkNEW_ARGS(TestingRec, (key, i)));
REPORTER_ASSERT(reporter, id);
cache.unlock(id);
cache.add(SkNEW_ARGS(TestingRec, (key, i)));
}
}
// test the originals after all that purging
for (int i = 0; i < COUNT; ++i) {
id = cache.findAndLock(TestingKey(i));
if (id) {
cache.unlock(id);
}
intptr_t value;
(void)cache.find(TestingKey(i), TestingRec::Visitor, &value);
}
cache.setTotalByteLimit(0);
@ -109,15 +106,11 @@ DEF_TEST(ImageCache_doubleAdd, r) {
TestingKey key(1);
SkResourceCache::ID id1 = cache.addAndLock(SkNEW_ARGS(TestingRec, (key, 2)));
SkResourceCache::ID id2 = cache.addAndLock(SkNEW_ARGS(TestingRec, (key, 3)));
// We don't really care if id1 == id2 as long as unlocking both works.
cache.unlock(id1);
cache.unlock(id2);
cache.add(SkNEW_ARGS(TestingRec, (key, 2)));
cache.add(SkNEW_ARGS(TestingRec, (key, 3)));
// Lookup can return either value.
const TestingRec* rec = (const TestingRec*)cache.findAndLock(key);
REPORTER_ASSERT(r, rec);
REPORTER_ASSERT(r, 2 == rec->fValue || 3 == rec->fValue);
cache.unlock(rec);
intptr_t value = -1;
REPORTER_ASSERT(r, cache.find(key, TestingRec::Visitor, &value));
REPORTER_ASSERT(r, 2 == value || 3 == value);
}