SkGlyphCache_Globals: SkMutex -> SkSpinlock

include/core/SkMutex.h

Index: include/core/SkMutex.h
diff --git a/include/core/SkMutex.h b/include/core/SkMutex.h
index ea7e81726b88c05828f35753c877b129eab2a15d..6c894db94edcc08e924202e9785c598931c171cf 100644
--- a/include/core/SkMutex.h
+++ b/include/core/SkMutex.h
@@ -1,3 +1,10 @@
+/*
+ * 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
 
@@ -10,21 +17,32 @@
     #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 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 SkAutoMutexAcquire(SkBaseMutex& mutex) : fMutex(&mutex) {
+    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();
         }
@@ -45,8 +63,11 @@ public:
     }
 
 private:
-    SkBaseMutex* fMutex;
+    Lock* fMutex;
 };
+
+using SkAutoMutexAcquire = SkAutoLockAcquire<SkBaseMutex>;
+
 #define SkAutoMutexAcquire(...) SK_REQUIRE_LOCAL_VAR(SkAutoMutexAcquire)