Notify resource caches when pixelref genID goes stale

src/core/SkPixelRef.cpp

Index: src/core/SkPixelRef.cpp
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index c73c2f4b7dc3db2487a4d18e19b972d50b0a970f..c0f67336d1339ef8bbd6031ac1d5f99a5da7cb49 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -5,6 +5,7 @@
  * found in the LICENSE file.
  */
 
+#include "SkBitmapCache.h"
 #include "SkPixelRef.h"
 #include "SkThread.h"
 
@@ -214,12 +215,14 @@ void SkPixelRef::addGenIDChangeListener(GenIDChangeListener* listener) {
     *fGenIDChangeListeners.append() = listener;
 }
 
+// we need to be called *before* the genID gets changed or zerod
 void SkPixelRef::callGenIDChangeListeners() {
     // We don't invalidate ourselves if we think another SkPixelRef is sharing our genID.
     if (fUniqueGenerationID) {
         for (int i = 0; i < fGenIDChangeListeners.count(); i++) {
             fGenIDChangeListeners[i]->onChange();
         }
+        SkNotifyBitmapGenIDIsStale(fGenerationID);

[f(malita), 2015/02/24 20:31:36]

IIUC, we're going to bombard the resource cache with notifications for every
single genid change - whether the resource is actually cached or not. Isn't that
going to have a significant pref impact?

[mtklein, 2015/02/24 20:48:37]

Sort of.  It's only visible changes.   It's pretty weird.  Here's an example
flow marked up:

 SkBitmap bm;
 bm.notifyPixelsChanged();
 // Does not trigger.  No one's read the gen ID, so there's no point.
 bm.getGenerationID();
 bm.notifyPixelsChanged();
 // Triggers.
 bm.notifyPixelsChanged();
 // Does not trigger.  No one's read the new gen ID.
 bm.notifyPixelsChanged();
 bm.notifyPixelsChanged();
 bm.notifyPixelsChanged();
 bm.notifyPixelsChanged();
 // Ditto.
 bm.getGenerationID();
 bm.notifyPixelsChanged();
 // Triggers.

I feel fairly confident about the perf impact being minimal, in that we're
already sending the same signal under the same conditions to Ganesh caches (via
fGenIDChangeListeners).  But, as always, we'll find out.  It might be fun to
have an excuse to make the underlying notification bus more efficient too.

[f(malita), 2015/02/24 20:54:55]

Ah, I did not notice the lazy impl - thanks for explaining!

[reed2, 2015/02/24 20:56:28]

Good thought. Perhaps we can have a flag on pixelref that announces that it has
been cached (ever)?

[mtklein, 2015/02/24 21:24:59]

Oh actually, this is a really good point, probably a problem, and we should
probably fix it preemptively.  The calls we're making to Ganesh are also
throttled by the fact that Ganesh has registered a listener, while we're calling
blind.  We'll be sending messages at a higher rate than we do today.

This might be a good case for using the GenIDChangeListener interface after all.
 Is there something we can have implement SkPixelRef::GenIDChangeListener
cleanly?  Maybe the cache, or the cache entry itself?

     }
     // Listeners get at most one shot, so whether these triggered or not, blow them away.
     fGenIDChangeListeners.deleteAll();