Steal a bit from the gen ID instead of managing two atomic values.

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"
@@ skipping 38 matching lines @@
     SkASSERT(SkIsPow2(PIXELREF_MUTEX_RING_COUNT));
 
     // atomic_inc might be overkill here. It may be fine if once in a while
     // we hit a race-condition and two subsequent calls get the same index...
     int index = sk_atomic_inc(&gPixelRefMutexRingIndex);
     return &gPixelRefMutexRing[index & (PIXELREF_MUTEX_RING_COUNT - 1)];
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
-int32_t SkNextPixelRefGenerationID();
-
-int32_t SkNextPixelRefGenerationID() {
-    static int32_t  gPixelRefGenerationID;
-    // do a loop in case our global wraps around, as we never want to
-    // return a 0
-    int32_t genID;
+static uint32_t next_gen_id() {
+    static uint32_t gNextGenID = 0;
+    uint32_t genID;
+    // Loop in case our global wraps around, as we never want to return a 0.
     do {
-        genID = sk_atomic_inc(&gPixelRefGenerationID) + 1;
+        genID = sk_atomic_fetch_add(&gNextGenID, 2u) + 2;  // Never set the low bit.
     } while (0 == genID);
     return genID;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkPixelRef::setMutex(SkBaseMutex* mutex) {
     if (NULL == mutex) {
         mutex = get_default_mutex();
     }
     fMutex = mutex;
 }
 
 // just need a > 0 value, so pick a funny one to aid in debugging
 #define SKPIXELREF_PRELOCKED_LOCKCOUNT     123456789
 
 static SkImageInfo validate_info(const SkImageInfo& info) {
     SkAlphaType newAlphaType = info.alphaType();
     SkAssertResult(SkColorTypeValidateAlphaType(info.colorType(), info.alphaType(), &newAlphaType));
     return info.makeAlphaType(newAlphaType);
 }
 
 SkPixelRef::SkPixelRef(const SkImageInfo& info)
     : fInfo(validate_info(info))
 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
-    , fStableID(SkNextPixelRefGenerationID())
+    , fStableID(next_gen_id())
 #endif
 
 {
     this->setMutex(NULL);
     fRec.zero();
     fLockCount = 0;
     this->needsNewGenID();
     fIsImmutable = false;
     fPreLocked = false;
     fAddedToCache.store(false);
 }
 
 
 SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex)
     : fInfo(validate_info(info))
 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
-    , fStableID(SkNextPixelRefGenerationID())
+    , fStableID(next_gen_id())
 #endif
 {
     this->setMutex(mutex);
     fRec.zero();
     fLockCount = 0;
     this->needsNewGenID();
     fIsImmutable = false;
     fPreLocked = false;
     fAddedToCache.store(false);
 }
 
 SkPixelRef::~SkPixelRef() {
     this->callGenIDChangeListeners();
 }
 
 void SkPixelRef::needsNewGenID() {
-    fGenerationID.store(0);
-    fUniqueGenerationID.store(false);
+    fTaggedGenID.store(0);
+    SkASSERT(!this->genIDIsUnique());

[reed1, 2015/02/25 16:41:31]

// this is a racy assert

[mtklein, 2015/02/25 16:44:45]

Done.

 }
 
 void SkPixelRef::cloneGenID(const SkPixelRef& that) {
     // This is subtle.  We must call that.getGenerationID() to make sure its genID isn't 0.
-    this->fGenerationID.store(that.getGenerationID());
-    this->fUniqueGenerationID.store(false);
-    that.fUniqueGenerationID.store(false);
+    uint32_t genID = that.getGenerationID();
+
+    // Neither ID is unique any more.
+    // (These & ~1u are actually redundant.  that.getGenerationID() just did it for us.)
+    this->fTaggedGenID.store(genID & ~1u);
+    that. fTaggedGenID.store(genID & ~1u);
+
+    SkASSERT(!this->genIDIsUnique());
+    SkASSERT(!that. genIDIsUnique());
 }
 
 void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctable) {
 #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED
     // only call me in your constructor, otherwise fLockCount tracking can get
     // out of sync.
     fRec.fPixels = pixels;
     fRec.fColorTable = ctable;
     fRec.fRowBytes = rowBytes;
     fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT;
@@ skipping 48 matching lines @@
 
 bool SkPixelRef::lockPixelsAreWritable() const {
     return this->onLockPixelsAreWritable();
 }
 
 bool SkPixelRef::onLockPixelsAreWritable() const {
     return true;
 }
 
 uint32_t SkPixelRef::getGenerationID() const {
-    uint32_t id = fGenerationID.load();
+    uint32_t id = fTaggedGenID.load();
     if (0 == id) {
-        id = SkNextPixelRefGenerationID();
-        fGenerationID.store(id);
-        fUniqueGenerationID.store(true);  // The only time we can be sure of this!
+        id = next_gen_id();
+        fTaggedGenID.store(id | 1u);  // TODO(mtklein): should become a compare-and-swap
+        SkASSERT(this->genIDIsUnique());
     }
-    return id;
+    return id & ~1u;  // Mask off bottom unique bit.
 }
 
 void SkPixelRef::addGenIDChangeListener(GenIDChangeListener* listener) {
-    if (NULL == listener || !fUniqueGenerationID.load()) {
+    if (NULL == listener || !this->genIDIsUnique()) {
         // 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.load()) {
+    if (this->genIDIsUnique()) {
         for (int i = 0; i < fGenIDChangeListeners.count(); i++) {
             fGenIDChangeListeners[i]->onChange();
         }
 
         // TODO: SkAtomic could add "old_value = atomic.xchg(new_value)" to make this clearer.
         if (fAddedToCache.load()) {
             SkNotifyBitmapGenIDIsStale(this->getGenerationID());
             fAddedToCache.store(false);
         }
     }
@@ skipping 33 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;
 }