SkGlyphCache_Globals: SkMutex -> SkSpinlock

include/core/SkMutex.h

+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
 #ifndef SkMutex_DEFINED
 #define SkMutex_DEFINED
 
 // This file is not part of the public Skia API.
 #include "SkTypes.h"
 
 #if defined(SK_BUILD_FOR_WIN)
     #include "../ports/SkMutex_win.h"
 #else
     #include "../ports/SkMutex_pthread.h"
 #endif
 
-class SkAutoMutexAcquire : SkNoncopyable {
+template <typename Lock>
+class SkScopedLock : SkNoncopyable {

[mtklein, 2015/07/08 19:33:47]

Why not just use SkAutoLockAcquire<Lock> ?

[reed1, 2015/07/08 19:36:01]

For now, I suggest we keep the current skia name convention of Auto

SkAutoTAcquire?

Happy to consider switching auto->scoped, but lets do that more deliberately.

[herb_g, 2015/07/08 20:21:57]

I agree. Regressed back to google3 naming.

[herb_g, 2015/07/08 20:21:57]

I don't want to take the hit of checking for NULL all the time. This is
obviously a hot path and will be blowing out the branch predictors. I measure a
performance improvement by using the simpler locking system.

There is a similar system in SkTaskGroup called AutoLock.

 public:
-    explicit SkAutoMutexAcquire(SkBaseMutex& mutex) : fMutex(&mutex) {
+    explicit SkScopedLock(Lock& lock) : fLock(lock) { fLock.acquire(); }
+    ~SkScopedLock() { fLock.release(); }
+
+private:
+    Lock& fLock;
+};
+
+template <typename Lock>
+class SkAutoLockAcquire : SkNoncopyable {

[reed1, 2015/07/08 19:36:01]

Do these two just differ by * -vs- & ? If so, that's sad (and confusing)

[mtklein, 2015/07/08 19:36:34]

yep

[herb_g, 2015/07/08 20:21:57]

These two differ by checking for NULL all the time. With Lock& there is no need
to check for NULL.

+public:
+    explicit SkAutoLockAcquire(Lock& mutex) : fMutex(&mutex) {
         SkASSERT(fMutex != NULL);
         mutex.acquire();
     }
 
-    explicit SkAutoMutexAcquire(SkBaseMutex* mutex) : fMutex(mutex) {
+    explicit SkAutoLockAcquire(Lock* mutex) : fMutex(mutex) {
         if (mutex) {
             mutex->acquire();
         }
     }
 
     /** If the mutex has not been released, release it now. */
-    ~SkAutoMutexAcquire() {
+    ~SkAutoLockAcquire() {
         if (fMutex) {
             fMutex->release();
         }
     }
 
     /** If the mutex has not been released, release it now. */
     void release() {
         if (fMutex) {
             fMutex->release();
             fMutex = NULL;
         }
     }
 
     /** Assert that we're holding the mutex. */
     void assertHeld() {
         SkASSERT(fMutex);
         fMutex->assertHeld();
     }
 
 private:
-    SkBaseMutex* fMutex;
+    Lock* fMutex;
 };
+
+using SkAutoMutexAcquire = SkAutoLockAcquire<SkBaseMutex>;
+
 #define SkAutoMutexAcquire(...) SK_REQUIRE_LOCAL_VAR(SkAutoMutexAcquire)
 
 
 #endif//SkMutex_DEFINED