SkGlyphCache_Globals: SkMutex -> SkSpinlock

src/core/SkGlyphCache.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkGlyphCache.h"
 #include "SkGlyphCache_Globals.h"
 #include "SkGraphics.h"
 #include "SkLazyPtr.h"
 #include "SkMutex.h"
 #include "SkPaint.h"
 #include "SkPath.h"
 #include "SkTLS.h"
 #include "SkTemplates.h"
 #include "SkTypeface.h"
 
 //#define SPEW_PURGE_STATUS
 
 namespace {
 
 SkGlyphCache_Globals* create_globals() {
-    return SkNEW_ARGS(SkGlyphCache_Globals, (SkGlyphCache_Globals::kYes_UseMutex));
+    return SkNEW(SkGlyphCache_Globals);
 }
 
 }  // namespace
 
 SK_DECLARE_STATIC_LAZY_PTR(SkGlyphCache_Globals, globals, create_globals);
 
 // Returns the shared globals
 static SkGlyphCache_Globals& getSharedGlobals() {
     return *globals.get();
 }
 
 // Returns the TLS globals (if set), or the shared globals
 static SkGlyphCache_Globals& getGlobals() {

[mtklein_C, 2015/07/09 16:46:22]

seems like we can merge getGlobals() and getSharedGlobals() now?

(If we do merge them, Skia style wants this  static functionto be named
get_globals())

[herb_g, 2015/07/09 18:21:15]

Done.

-    SkGlyphCache_Globals* tls = SkGlyphCache_Globals::FindTLS();
-    return tls ? *tls : getSharedGlobals();
+    return getSharedGlobals();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifdef SK_GLYPHCACHE_TRACK_HASH_STATS
     #define RecordHashSuccess()             fHashHitCount += 1
     #define RecordHashCollisionIf(pred)     do { if (pred) fHashMissCount += 1; } while (0)
 #else
     #define RecordHashSuccess()             (void)0
     #define RecordHashCollisionIf(pred)     (void)0
@@ skipping 361 matching lines @@
         rec->fProc(rec->fData);
         AuxProcRec* next = rec->fNext;
         SkDELETE(rec);
         rec = next;
     }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 ///////////////////////////////////////////////////////////////////////////////
 
+
+class AutoAcquire {
+public:
+    AutoAcquire(SkSpinlock& lock) : fLock(lock) { fLock.acquire(); }
+    ~AutoAcquire() { fLock.release(); }
+private:
+    SkSpinlock& fLock;
+};
+
 size_t SkGlyphCache_Globals::setCacheSizeLimit(size_t newLimit) {
     static const size_t minLimit = 256 * 1024;
     if (newLimit < minLimit) {
         newLimit = minLimit;
     }
 
-    SkAutoMutexAcquire    ac(fMutex);
+    AutoAcquire ac(fLock);
 
     size_t prevLimit = fCacheSizeLimit;
     fCacheSizeLimit = newLimit;
     this->internalPurge();
     return prevLimit;
 }
 
 int SkGlyphCache_Globals::setCacheCountLimit(int newCount) {
     if (newCount < 0) {
         newCount = 0;
     }
 
-    SkAutoMutexAcquire    ac(fMutex);
+    AutoAcquire ac(fLock);
 
     int prevCount = fCacheCountLimit;
     fCacheCountLimit = newCount;
     this->internalPurge();
     return prevCount;
 }
 
 void SkGlyphCache_Globals::purgeAll() {
-    SkAutoMutexAcquire    ac(fMutex);
+    AutoAcquire ac(fLock);
     this->internalPurge(fTotalMemoryUsed);
 }
 
 /*  This guy calls the visitor from within the mutext lock, so the visitor
     cannot:
     - take too much time
     - try to acquire the mutext again
     - call a fontscaler (which might call into the cache)
 */
 SkGlyphCache* SkGlyphCache::VisitCache(SkTypeface* typeface,
                               const SkDescriptor* desc,
                               bool (*proc)(const SkGlyphCache*, void*),
                               void* context) {
     if (!typeface) {
         typeface = SkTypeface::GetDefaultTypeface();
     }
     SkASSERT(desc);
 
     SkGlyphCache_Globals& globals = getGlobals();
-    SkAutoMutexAcquire    ac(globals.fMutex);
     SkGlyphCache*         cache;
-    bool                  insideMutex = true;
 
-    globals.validate();
+    {
+        AutoAcquire ac(globals.fLock);
 
-    for (cache = globals.internalGetHead(); cache != NULL; cache = cache->fNext) {
-        if (cache->fDesc->equals(*desc)) {
-            globals.internalDetachCache(cache);
-            goto FOUND_IT;
+        globals.validate();
+
+        for (cache = globals.internalGetHead(); cache != NULL; cache = cache->fNext) {
+            if (cache->fDesc->equals(*desc)) {
+                globals.internalDetachCache(cache);
+                if (!proc(cache, context)) {
+                    globals.internalAttachCacheToHead(cache);

[mtklein_C, 2015/07/09 16:46:22]

Thanks, I find this code a lot easier to follow than the old code.

[herb_g, 2015/07/09 18:21:15]

Acknowledged.

+                    cache = NULL;
+                }
+                return cache;
+            }
         }
     }
 
-    /* Release the mutex now, before we create a new entry (which might have
-        side-effects like trying to access the cache/mutex (yikes!)
-    */
-    ac.release();           // release the mutex now
-    insideMutex = false;    // can't use globals anymore
-
     // Check if we can create a scaler-context before creating the glyphcache.
     // If not, we may have exhausted OS/font resources, so try purging the
     // cache once and try again.
     {
         // pass true the first time, to notice if the scalercontext failed,
         // so we can try the purge.
         SkScalerContext* ctx = typeface->createScalerContext(desc, true);
         if (!ctx) {
             getSharedGlobals().purgeAll();
             ctx = typeface->createScalerContext(desc, false);
             SkASSERT(ctx);
         }
         cache = SkNEW_ARGS(SkGlyphCache, (typeface, desc, ctx));
     }
 
-FOUND_IT:
-
     AutoValidate av(cache);
 
     if (!proc(cache, context)) {   // need to reattach
-        if (insideMutex) {
-            globals.internalAttachCacheToHead(cache);
-        } else {
-            globals.attachCacheToHead(cache);
-        }
+        globals.attachCacheToHead(cache);
         cache = NULL;
     }
     return cache;
 }
 
 void SkGlyphCache::AttachCache(SkGlyphCache* cache) {
     SkASSERT(cache);
     SkASSERT(cache->fNext == NULL);
 
     getGlobals().attachCacheToHead(cache);
 }
 
 void SkGlyphCache::Dump() {
     SkGlyphCache_Globals& globals = getGlobals();
-    SkAutoMutexAcquire    ac(globals.fMutex);
+    AutoAcquire           ac(globals.fLock);
     SkGlyphCache*         cache;
 
     globals.validate();
 
     SkDebugf("SkGlyphCache strikes:%d memory:%d\n",
              globals.getCacheCountUsed(), (int)globals.getTotalMemoryUsed());
 
 #ifdef SK_GLYPHCACHE_TRACK_HASH_STATS
     int hitCount = 0;
     int missCount = 0;
 #endif
 
     for (cache = globals.internalGetHead(); cache != NULL; cache = cache->fNext) {
 #ifdef SK_GLYPHCACHE_TRACK_HASH_STATS
         hitCount += cache->fHashHitCount;
         missCount += cache->fHashMissCount;
 #endif
         cache->dump();
     }
 #ifdef SK_GLYPHCACHE_TRACK_HASH_STATS
     SkDebugf("Hash hit percent:%2d\n", 100 * hitCount / (hitCount + missCount));
 #endif
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkGlyphCache_Globals::attachCacheToHead(SkGlyphCache* cache) {
-    SkAutoMutexAcquire    ac(fMutex);
+    AutoAcquire ac(fLock);
 
     this->validate();
     cache->validate();
 
     this->internalAttachCacheToHead(cache);
     this->internalPurge();
 }
 
 SkGlyphCache* SkGlyphCache_Globals::internalGetTail() const {
     SkGlyphCache* cache = fHead;
@@ skipping 108 matching lines @@
     size_t computedBytes = 0;
     int computedCount = 0;
 
     const SkGlyphCache* head = fHead;
     while (head != NULL) {
         computedBytes += head->fMemoryUsed;
         computedCount += 1;
         head = head->fNext;
     }
 
-    SkASSERT(fTotalMemoryUsed == computedBytes);
-    SkASSERT(fCacheCount == computedCount);
+    SkASSERTF(fCacheCount == computedCount, "fCacheCount: %d, computedCount: %d", fCacheCount,
+              computedCount);
+    SkASSERTF(fTotalMemoryUsed == computedBytes, "fTotalMemoryUsed: %d, computedBytes: %d",
+              fTotalMemoryUsed, computedBytes);
 }
 
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "SkTypefaceCache.h"
 
 size_t SkGraphics::GetFontCacheLimit() {
@@ skipping 18 matching lines @@
 
 int SkGraphics::GetFontCacheCountUsed() {
     return getSharedGlobals().getCacheCountUsed();
 }
 
 void SkGraphics::PurgeFontCache() {
     getSharedGlobals().purgeAll();
     SkTypefaceCache::PurgeAll();
 }
 
 size_t SkGraphics::GetTLSFontCacheLimit() {

[mtklein_C, 2015/07/09 16:46:22]

TODO(mtklein): clean up TLS apis ?

[herb_g, 2015/07/09 18:21:15]

Done.

-    const SkGlyphCache_Globals* tls = SkGlyphCache_Globals::FindTLS();
-    return tls ? tls->getCacheSizeLimit() : 0;
+    return 0;
 }
 
 void SkGraphics::SetTLSFontCacheLimit(size_t bytes) {
-    if (0 == bytes) {
-        SkGlyphCache_Globals::DeleteTLS();
-    } else {
-        SkGlyphCache_Globals::GetTLS().setCacheSizeLimit(bytes);
-    }
 }