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

only notify bitmaps that have been added to the cache

Browse Source 
old code:
- calls=2677 hit-rate=3.51139%

new code:
- calls=94 hit-rate=97.8723%

BUG=skia:

Review URL: https://codereview.chromium.org/960563002
This commit is contained in:
reed 2015-02-25 07:17:11 -08:00 committed by Commit bot
parent 8673765ab5
commit 83787d0ff0
8 changed files with 58 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
include/core/SkPixelRef.h
View File

@ -245,6 +245,12 @@ public:
// Takes ownership of listener.
void addGenIDChangeListener(GenIDChangeListener* listener);
// Call when this pixelref is part of the key to a resourcecache entry. This allows the cache
// to know automatically those entries can be purged when this pixelref is changed or deleted.
void notifyAddedToCache() {
fAddedToCache.store(true);
}
protected:
/**
* On success, returns true and fills out the LockRec for the pixels. On
@ -315,6 +321,7 @@ private:
mutable SkAtomic<uint32_t> fGenerationID;
mutable SkAtomic<bool> fUniqueGenerationID;
SkAtomic<bool> fAddedToCache;
#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
const uint32_t fStableID;
#endif

8
src/core/SkBitmapCache.cpp
View File

@ -8,6 +8,7 @@
#include "SkBitmapCache.h"
#include "SkResourceCache.h"
#include "SkMipMap.h"
#include "SkPixelRef.h"
#include "SkRect.h"
/**
@ -112,6 +113,7 @@ void SkBitmapCache::Add(const SkBitmap& src, SkScalar invScaleX, SkScalar invSca
BitmapRec* rec = SkNEW_ARGS(BitmapRec, (src.getGenerationID(), invScaleX, invScaleY,
get_bounds_from_bitmap(src), result));
CHECK_LOCAL(localCache, add, Add, rec);
src.pixelRef()->notifyAddedToCache();
}
bool SkBitmapCache::Find(uint32_t genID, const SkIRect& subset, SkBitmap* result,
@ -121,7 +123,7 @@ bool SkBitmapCache::Find(uint32_t genID, const SkIRect& subset, SkBitmap* result
return CHECK_LOCAL(localCache, find, Find, key, BitmapRec::Finder, result);
}
bool SkBitmapCache::Add(uint32_t genID, const SkIRect& subset, const SkBitmap& result,
bool SkBitmapCache::Add(SkPixelRef* pr, const SkIRect& subset, const SkBitmap& result,
SkResourceCache* localCache) {
SkASSERT(result.isImmutable());
@ -132,9 +134,10 @@ bool SkBitmapCache::Add(uint32_t genID, const SkIRect& subset, const SkBitmap& r
|| result.height() != subset.height()) {
return false;
} else {
BitmapRec* rec = SkNEW_ARGS(BitmapRec, (genID, SK_Scalar1, SK_Scalar1, subset, result));
BitmapRec* rec = SkNEW_ARGS(BitmapRec, (pr->getGenerationID(), 1, 1, subset, result));
CHECK_LOCAL(localCache, add, Add, rec);
pr->notifyAddedToCache();
return true;
}
}
@ -211,6 +214,7 @@ const SkMipMap* SkMipMapCache::AddAndRef(const SkBitmap& src, SkResourceCache* l
if (mipmap) {
MipMapRec* rec = SkNEW_ARGS(MipMapRec, (src, mipmap));
CHECK_LOCAL(localCache, add, Add, rec);
src.pixelRef()->notifyAddedToCache();
}
return mipmap;
}

2
src/core/SkBitmapCache.h
View File

@ -50,7 +50,7 @@ public:
* The width and the height of the provided subset must be the same as the result bitmap ones.
* result must be marked isImmutable()
*/
static bool Add(uint32_t genID, const SkIRect& subset, const SkBitmap& result,
static bool Add(SkPixelRef*, const SkIRect& subset, const SkBitmap& result,
SkResourceCache* localCache = NULL);
};

9
src/core/SkPixelRef.cpp
View File

@ -100,6 +100,7 @@ SkPixelRef::SkPixelRef(const SkImageInfo& info)
this->needsNewGenID();
fIsImmutable = false;
fPreLocked = false;
fAddedToCache.store(false);
}
@ -115,6 +116,7 @@ SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex)
this->needsNewGenID();
fIsImmutable = false;
fPreLocked = false;
fAddedToCache.store(false);
}
SkPixelRef::~SkPixelRef() {
@ -225,10 +227,11 @@ void SkPixelRef::callGenIDChangeListeners() {
fGenIDChangeListeners[i]->onChange();
}
// If we can flag the pixelref somehow whenever it was actually added to the cache,
// perhaps it would be nice to only call this notifier in that case. For now we always
// call it, since we don't know if it was cached or not.
// TODO: SkAtomic could add "old_value = atomic.xchg(new_value)" to make this clearer.
if (fAddedToCache.load()) {
SkNotifyBitmapGenIDIsStale(this->getGenerationID());
fAddedToCache.store(false);
}
}
// Listeners get at most one shot, so whether these triggered or not, blow them away.
fGenIDChangeListeners.deleteAll();

23
src/core/SkResourceCache.cpp
View File

@ -305,11 +305,22 @@ void SkResourceCache::purgeAsNeeded(bool forcePurge) {
}
}
//#define SK_TRACK_PURGE_SHAREDID_HITRATE
#ifdef SK_TRACK_PURGE_SHAREDID_HITRATE
static int gPurgeCallCounter;
static int gPurgeHitCounter;
#endif
void SkResourceCache::purgeSharedID(uint64_t sharedID) {
if (0 == sharedID) {
return;
}
#ifdef SK_TRACK_PURGE_SHAREDID_HITRATE
gPurgeCallCounter += 1;
bool found = false;
#endif
// go backwards, just like purgeAsNeeded, just to make the code similar.
// could iterate either direction and still be correct.
Rec* rec = fTail;
@ -318,9 +329,21 @@ void SkResourceCache::purgeSharedID(uint64_t sharedID) {
if (rec->getKey().getSharedID() == sharedID) {
// SkDebugf("purgeSharedID id=%llx rec=%p\n", sharedID, rec);
this->remove(rec);
#ifdef SK_TRACK_PURGE_SHAREDID_HITRATE
found = true;
#endif
}
rec = prev;
}
#ifdef SK_TRACK_PURGE_SHAREDID_HITRATE
if (found) {
gPurgeHitCounter += 1;
}
SkDebugf("PurgeShared calls=%d hits=%d rate=%g\n", gPurgeCallCounter, gPurgeHitCounter,
gPurgeHitCounter * 100.0 / gPurgeCallCounter);
#endif
}
size_t SkResourceCache::setTotalByteLimit(size_t newLimit) {

2
src/gpu/SkGrPixelRef.cpp
View File

@ -189,7 +189,7 @@ bool SkGrPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
// If we are here, pixels were read correctly from the surface.
cachedBitmap.setImmutable();
//Add to the cache
SkBitmapCache::Add(this->getGenerationID(), bounds, cachedBitmap);
SkBitmapCache::Add(this, bounds, cachedBitmap);
dst->swap(cachedBitmap);
}

3
src/lazy/SkCachingPixelRef.cpp
View File

@ -63,8 +63,7 @@ bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) {
return false;
}
fLockedBitmap.setImmutable();
SkBitmapCache::Add(
this->getGenerationID(), info.bounds(), fLockedBitmap);
SkBitmapCache::Add(this, info.bounds(), fLockedBitmap);
}
// Now bitmap should contain a concrete PixelRef of the decoded image.

22
tests/SkResourceCacheTest.cpp
View File

@ -108,20 +108,22 @@ DEF_TEST(BitmapCache_add_rect, reporter) {
SkBitmap bm;
SkIRect rect = SkIRect::MakeWH(5, 5);
uint32_t cachedID = cachedBitmap.getGenerationID();
SkPixelRef* cachedPR = cachedBitmap.pixelRef();
// Wrong subset size
REPORTER_ASSERT(reporter, !SkBitmapCache::Add(cachedBitmap.getGenerationID(), SkIRect::MakeWH(4, 6), cachedBitmap, cache));
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
REPORTER_ASSERT(reporter, !SkBitmapCache::Add(cachedPR, SkIRect::MakeWH(4, 6), cachedBitmap, cache));
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedID, rect, &bm, cache));
// Wrong offset value
REPORTER_ASSERT(reporter, !SkBitmapCache::Add(cachedBitmap.getGenerationID(), SkIRect::MakeXYWH(-1, 0, 5, 5), cachedBitmap, cache));
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
REPORTER_ASSERT(reporter, !SkBitmapCache::Add(cachedPR, SkIRect::MakeXYWH(-1, 0, 5, 5), cachedBitmap, cache));
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedID, rect, &bm, cache));
// Should not be in the cache
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedID, rect, &bm, cache));
REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.getGenerationID(), rect, cachedBitmap, cache));
REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedPR, rect, cachedBitmap, cache));
// Should be in the cache, we just added it
REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedID, rect, &bm, cache));
}
#include "SkMipMap.h"
@ -215,7 +217,7 @@ static void test_bitmap_notify(skiatest::Reporter* reporter, SkResourceCache* ca
src[i].setImmutable();
dst[i].allocN32Pixels(5, 5);
dst[i].setImmutable();
SkBitmapCache::Add(src[i].getGenerationID(), subset, dst[i], cache);
SkBitmapCache::Add(src[i].pixelRef(), subset, dst[i], cache);
}
for (int i = 0; i < N; ++i) {
@ -255,7 +257,7 @@ DEF_TEST(BitmapCache_discarded_bitmap, reporter) {
SkIRect rect = SkIRect::MakeWH(5, 5);
// Add a bitmap to the cache.
REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.getGenerationID(), rect, cachedBitmap, cache));
REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.pixelRef(), rect, cachedBitmap, cache));
REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
// Finding more than once works fine.
@ -277,7 +279,7 @@ DEF_TEST(BitmapCache_discarded_bitmap, reporter) {
cachedBitmap.unlockPixels();
// We can add the bitmap back to the cache and find it again.
REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.getGenerationID(), rect, cachedBitmap, cache));
REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.pixelRef(), rect, cachedBitmap, cache));
REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
test_mipmapcache(reporter, cache);