only notify bitmaps that have been added to the cache

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

6 files changed, 39 insertions, 10 deletions

diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp
index c411a1bd9d..f569db8e8a 100644
--- a/src/core/SkBitmapCache.cpp
+++ b/src/core/SkBitmapCache.cpp
@@ -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;
 }
diff --git a/src/core/SkBitmapCache.h b/src/core/SkBitmapCache.h
index c54f96141c..de50aabe1e 100644
--- a/src/core/SkBitmapCache.h
+++ b/src/core/SkBitmapCache.h
@@ -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);
 };
 
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 7aed66c722..560748c463 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -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.
-        SkNotifyBitmapGenIDIsStale(this->getGenerationID());
+        // 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();
diff --git a/src/core/SkResourceCache.cpp b/src/core/SkResourceCache.cpp
index 9da90c45dc..43e752b38d 100644
--- a/src/core/SkResourceCache.cpp
+++ b/src/core/SkResourceCache.cpp
@@ -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) {
diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp
index b0e89092cb..01444af705 100644
--- a/src/gpu/SkGrPixelRef.cpp
+++ b/src/gpu/SkGrPixelRef.cpp
@@ -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);
     }
diff --git a/src/lazy/SkCachingPixelRef.cpp b/src/lazy/SkCachingPixelRef.cpp
index 570fc6fbd7..dc53a5d6c7 100644
--- a/src/lazy/SkCachingPixelRef.cpp
+++ b/src/lazy/SkCachingPixelRef.cpp
@@ -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.