Notify resource caches when pixelref genID goes stale

src/core/SkPixelRef.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
+#include "SkBitmapCache.h"
 #include "SkPixelRef.h"
 #include "SkThread.h"
 
 #ifdef SK_BUILD_FOR_WIN32
     // We don't have SK_BASE_MUTEX_INIT on Windows.
 
     // must be a power-of-2. undef to just use 1 mutex
     #define PIXELREF_MUTEX_RING_COUNT       32
     static SkBaseMutex gPixelRefMutexRing[PIXELREF_MUTEX_RING_COUNT];
 
@@ skipping 189 matching lines @@
 
 void SkPixelRef::addGenIDChangeListener(GenIDChangeListener* listener) {
     if (NULL == listener || !fUniqueGenerationID) {
         // No point in tracking this if we're not going to call it.
         SkDELETE(listener);
         return;
     }
     *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();
 }
 
 void SkPixelRef::notifyPixelsChanged() {
 #ifdef SK_DEBUG
     if (fIsImmutable) {
         SkDebugf("========== notifyPixelsChanged called on immutable pixelref");
     }
@@ skipping 24 matching lines @@
 
 bool SkPixelRef::onGetYUV8Planes(SkISize sizes[3], void* planes[3], size_t rowBytes[3],
                                  SkYUVColorSpace* colorSpace) {
     return false;
 }
 
 size_t SkPixelRef::getAllocatedSizeInBytes() const {
     return 0;
 }