Refactor Mutex.hpp (#4594)

* Remove `LockGuard` in favour of `std::lock_guard`
* Rename `SimpleMutex` to `SpinLock`

kotlin-native/runtime/src/legacymm/cpp/Memory.cpp

@@ -18,6 +18,7 @@
 #include <stdio.h>
 
 #include <cstddef> // for offsetof
+#include <mutex>
 
 // Allow concurrent global cycle collector.
 #define USE_CYCLIC_GC 0
@@ -218,14 +219,14 @@ class CycleDetector : private kotlin::Pinned {
   }
 
   void insertCandidate(KRef candidate) {
-    LockGuard<SimpleMutex> guard(lock_);
+    std::lock_guard<kotlin::SpinLock> guard(lock_);
 
     auto it = candidateList_.insert(candidateList_.begin(), candidate);
     candidateInList_.emplace(candidate, it);
   }
 
   void removeCandidate(KRef candidate) {
-    LockGuard<SimpleMutex> guard(lock_);
+    std::lock_guard<kotlin::SpinLock> guard(lock_);
 
     auto it = candidateInList_.find(candidate);
     if (it == candidateInList_.end())
@@ -234,7 +235,7 @@ class CycleDetector : private kotlin::Pinned {
     candidateInList_.erase(it);
   }
 
-  SimpleMutex lock_;
+  kotlin::SpinLock lock_;
   using CandidateList = KStdList<KRef>;
   CandidateList candidateList_;
   KStdUnorderedMap<KRef, CandidateList::iterator> candidateInList_;
@@ -2990,7 +2991,7 @@ ScopedRefHolder::~ScopedRefHolder() {
 CycleDetectorRootset CycleDetector::collectRootset() {
   auto& detector = instance();
   CycleDetectorRootset rootset;
-  LockGuard<SimpleMutex> guard(detector.lock_);
+  std::lock_guard<kotlin::SpinLock> guard(detector.lock_);
   for (auto* candidate: detector.candidateList_) {
     // Only frozen candidates are to be analyzed.
     if (!isPermanentOrFrozen(candidate))

kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp

@@ -65,7 +65,7 @@ public:
             for (auto& node : queue_) {
                 node.owner_ = nullptr;
             }
-            std::lock_guard<SimpleMutex> guard(owner_.mutex_);
+            std::lock_guard<SpinLock> guard(owner_.mutex_);
             owner_.queue_.splice(owner_.queue_.end(), queue_);
             owner_.deletionQueue_.splice(owner_.deletionQueue_.end(), deletionQueue_);
         }
@@ -108,7 +108,7 @@ public:
         explicit Iterable(MultiSourceQueue& owner) noexcept : owner_(owner), guard_(owner_.mutex_) {}
 
         MultiSourceQueue& owner_; // weak
-        std::unique_lock<SimpleMutex> guard_;
+        std::unique_lock<SpinLock> guard_;
     };
 
     // Lock `MultiSourceQueue` for safe iteration. If element was scheduled for deletion,
@@ -117,7 +117,7 @@ public:
 
     // Lock `MultiSourceQueue` and apply deletions. Only deletes elements that were published.
     void ApplyDeletions() noexcept {
-        std::lock_guard<SimpleMutex> guard(mutex_);
+        std::lock_guard<SpinLock> guard(mutex_);
         std::list<Node*> remainingDeletions;
 
         auto it = deletionQueue_.begin();
@@ -143,7 +143,7 @@ private:
     // which is important for GC mark phase.
     std::list<Node> queue_;
     std::list<Node*> deletionQueue_;
-    SimpleMutex mutex_;
+    SpinLock mutex_;
 };
 
 } // namespace kotlin

kotlin-native/runtime/src/main/cpp/Mutex.hpp

@@ -18,41 +18,31 @@
 #define RUNTIME_MUTEX_H
 
 #include <cstdint>
+
 #include "KAssert.h"
 #include "Utils.hpp"
 
-class SimpleMutex {
+namespace kotlin {
- private:
-  int32_t atomicInt = 0;
 
- public:
+class SpinLock : private Pinned {
-  void lock() {
+public:
-    while (!__sync_bool_compare_and_swap(&atomicInt, 0, 1)) {
+    void lock() noexcept {
-      // TODO: yield.
+        while (!__sync_bool_compare_and_swap(&atomicInt, 0, 1)) {
+            // TODO: yield.
+        }
     }
-  }
 
-  void unlock() {
+    void unlock() noexcept {
-    if (!__sync_bool_compare_and_swap(&atomicInt, 1, 0)) {
+        if (!__sync_bool_compare_and_swap(&atomicInt, 1, 0)) {
-      RuntimeAssert(false, "Unable to unlock");
+            RuntimeAssert(false, "Unable to unlock");
+        }
     }
-  }
+
+private:
+    // TODO: Consider using `std::atomic_flag`.
+    int32_t atomicInt = 0;
 };
 
-// TODO: use std::lock_guard instead?
+} // namespace kotlin
-template <class Mutex>
-class LockGuard : private kotlin::Pinned {
- public:
-  explicit LockGuard(Mutex& mutex_) : mutex(mutex_) {
-    mutex.lock();
-  }
-
-  ~LockGuard() {
-    mutex.unlock();
-  }
-
- private:
-  Mutex& mutex;
-};
 
 #endif // RUNTIME_MUTEX_H

kotlin-native/runtime/src/main/cpp/ObjCExport.mm

@@ -21,6 +21,8 @@
 
 #if KONAN_OBJC_INTEROP
 
+#import <mutex>
+
 #import <Foundation/NSObject.h>
 #import <Foundation/NSValue.h>
 #import <Foundation/NSString.h>
@@ -959,7 +961,7 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType, co
   return result;
 }
 
-static SimpleMutex typeInfoCreationMutex;
+static kotlin::SpinLock typeInfoCreationMutex;
 
 static const TypeInfo* getOrCreateTypeInfo(Class clazz) {
   const TypeInfo* result = Kotlin_ObjCExport_getAssociatedTypeInfo(clazz);
@@ -973,7 +975,7 @@ static const TypeInfo* getOrCreateTypeInfo(Class clazz) {
     theForeignObjCObjectTypeInfo :
     getOrCreateTypeInfo(superClass);
 
-  LockGuard<SimpleMutex> lockGuard(typeInfoCreationMutex);
+  std::lock_guard<kotlin::SpinLock> lockGuard(typeInfoCreationMutex);
 
   result = Kotlin_ObjCExport_getAssociatedTypeInfo(clazz); // double-checking.
   if (result == nullptr) {
@@ -993,7 +995,7 @@ const TypeInfo* Kotlin_ObjCExport_createTypeInfoWithKotlinFieldsFrom(Class clazz
   return createTypeInfo(clazz, superType, fieldsInfo);
 }
 
-static SimpleMutex classCreationMutex;
+static kotlin::SpinLock classCreationMutex;
 static int anonymousClassNextId = 0;
 
 static void addVirtualAdapters(Class clazz, const ObjCTypeAdapter* typeAdapter) {
@@ -1067,7 +1069,7 @@ static Class getOrCreateClass(const TypeInfo* typeInfo) {
   } else {
     Class superClass = getOrCreateClass(typeInfo->superType_);
 
-    LockGuard<SimpleMutex> lockGuard(classCreationMutex); // Note: non-recursive
+    std::lock_guard<kotlin::SpinLock> lockGuard(classCreationMutex); // Note: non-recursive
 
     result = typeInfo->writableInfo_->objCExport.objCClass; // double-checking.
     if (result == nullptr) {

kotlin-native/runtime/src/main/cpp/ObjCInterop.mm

@@ -24,6 +24,7 @@
 #include <objc/message.h>
 #include <cstdio>
 #include <cstdint>
+#include <mutex>
 
 #include "Memory.h"
 #include "MemorySharedRefs.hpp"
@@ -240,7 +241,7 @@ static void AddMethods(Class clazz, const struct ObjCMethodDescription* methods,
   }
 }
 
-static SimpleMutex classCreationMutex;
+static kotlin::SpinLock classCreationMutex;
 static int anonymousClassNextId = 0;
 
 static Class allocateClass(const KotlinObjCClassInfo* info) {
@@ -271,7 +272,7 @@ static Class allocateClass(const KotlinObjCClassInfo* info) {
 }
 
 void* CreateKotlinObjCClass(const KotlinObjCClassInfo* info) {
-  LockGuard<SimpleMutex> lockGuard(classCreationMutex);
+  std::lock_guard<kotlin::SpinLock> lockGuard(classCreationMutex);
 
   void* createdClass = *info->createdClass;
   if (createdClass != nullptr) {

kotlin-native/runtime/src/main/cpp/SingleLockList.hpp

@@ -17,7 +17,7 @@
 namespace kotlin {
 
 // TODO: Consider different locking mechanisms.
-template <typename Value, typename Mutex = SimpleMutex>
+template <typename Value, typename Mutex = SpinLock>
 class SingleLockList : private Pinned {
 public:
     class Node : Pinned {
@@ -71,7 +71,7 @@ public:
     Node* Emplace(Args... args) noexcept {
         auto* nodePtr = new Node(args...);
         std::unique_ptr<Node> node(nodePtr);
-        LockGuard<Mutex> guard(mutex_);
+        std::lock_guard<Mutex> guard(mutex_);
         if (root_) {
             root_->previous = node.get();
         }
@@ -82,7 +82,7 @@ public:
 
     // Using `node` including its referred `Value` after `Erase` is undefined behaviour.
     void Erase(Node* node) noexcept {
-        LockGuard<Mutex> guard(mutex_);
+        std::lock_guard<Mutex> guard(mutex_);
         if (root_.get() == node) {
             root_ = std::move(node->next);
             if (root_) {