From 9a2e5674e8d6b520865fb7320247ee6dd52e4e46 Mon Sep 17 00:00:00 2001
From: Jamie Reece Wilson <me@reece.sx>
Date: Sun, 30 Jul 2023 11:10:08 +0100
Subject: [PATCH] [*] RWLock improvements

---
 Source/Threading/Primitives/AuRWLock.cpp | 95 ++++++++++++++----------
 Source/Threading/Primitives/AuRWLock.hpp |  5 +-
 2 files changed, 58 insertions(+), 42 deletions(-)

diff --git a/Source/Threading/Primitives/AuRWLock.cpp b/Source/Threading/Primitives/AuRWLock.cpp
index 8887b96e..f242caf5 100644
--- a/Source/Threading/Primitives/AuRWLock.cpp
+++ b/Source/Threading/Primitives/AuRWLock.cpp
@@ -18,6 +18,8 @@ namespace Aurora::Threading::Primitives
     #define ViewParent ((T *)(((char *)this) - (bIsReadView ? RWLockImpl<true>::kOffsetOfRead : RWLockImpl<true>::kOffsetOfWrite)))
 #endif
 
+    static const auto kRWThreadWriterHardContextSwitchBias = 15;
+
     template<bool bIsReadView, typename T>
     void RWLockAccessView<bIsReadView, T>::Unlock()
     {
@@ -152,12 +154,6 @@ namespace Aurora::Threading::Primitives
                     {
                         return false;
                     }
-
-                    if (this->writersPending_)
-                    {
-                        this->GetConditionWriter().Broadcast();
-                        continue;
-                    }
                 }
             }
         }
@@ -198,8 +194,9 @@ namespace Aurora::Threading::Primitives
             }
         }
 
+        AuAtomicAdd(&this->writersPending_, 1);
+
         AU_LOCK_GUARD(this->mutex_);
-        this->writersPending_++;
 
         AuInt64 uEndTime = uTimeout ? AuTime::SteadyClockNS() + uTimeout : 0;
 
@@ -215,7 +212,7 @@ namespace Aurora::Threading::Primitives
 
                     if (uSecondTimeout <= 0)
                     {
-                        this->writersPending_--;
+                        AuAtomicSub(&this->writersPending_, 1);
                         return false;
                     }
                 }
@@ -226,7 +223,7 @@ namespace Aurora::Threading::Primitives
                 if (!this->GetConditionWriter().WaitForSignalNsEx(AuUnsafeRaiiToShared(&this->mutex_), uSecondTimeout))
             #endif
                 {
-                    this->writersPending_--;
+                    AuAtomicSub(&this->writersPending_, 1);
                     return false;
                 }
             }
@@ -234,12 +231,13 @@ namespace Aurora::Threading::Primitives
             if (AuAtomicCompareExchange(&this->state_, -1, 0) == 0)
             {
                 this->reentrantWriteLockHandle_ = GetThreadCookie();
-                this->writersPending_--;
+                AuAtomicSub(&this->writersPending_, 1);
                 return true;
             }
             else
             {
                 this->GetConditionWriter().Broadcast();
+                this->GetCondition().Broadcast();
             }
         }
 
@@ -265,7 +263,7 @@ namespace Aurora::Threading::Primitives
     template<bool bIsWriteRecursionAllowed>
     bool RWLockImpl<bIsWriteRecursionAllowed>::TryLockWrite()
     {
-        for (AuUInt i = 0; i < 20; i++)
+        for (AU_ITERATE_N(i, kRWThreadWriterHardContextSwitchBias))
         {
             auto curVal = this->state_;
 
@@ -309,61 +307,82 @@ namespace Aurora::Threading::Primitives
     template<bool bIsWriteRecursionAllowed>
     void RWLockImpl<bIsWriteRecursionAllowed>::UnlockRead()
     {
+        AuInt32 uVal {};
+
         if (this->state_ < 0)
         {
             SysAssertDbg(this->reentrantWriteLockHandle_ == GetThreadCookie());
             return;
         }
 
-        AuInt32 val {};
-        bool bElevation {};
+        uVal = AuAtomicSub(&this->state_, 1);
 
+        if (uVal == 0)
         {
-            AU_LOCK_GUARD(this->mutex_);
-            val = AuAtomicSub(&this->state_, 1);
-            bElevation = this->bElevaterPending_;
-        }
+            bool bElevation {};
+
+            {
+                AU_LOCK_GUARD(this->mutex_);
+                bElevation = this->writersPending_ > 0;
+            }
         
-        if ((val == 0) && (bElevation))
-        {
-            this->GetConditionWriter().Signal();
-        }
-
-        if (val == 0)
-        {
-            this->GetCondition().Signal();
-            this->GetConditionWriter().Signal();
+            if (bElevation)
+            {
+                this->GetConditionWriter().Signal();
+            }
+            else
+            {
+                this->GetCondition().Broadcast();
+            }
         }
     }
 
     template<bool bIsWriteRecursionAllowed>
     void RWLockImpl<bIsWriteRecursionAllowed>::UnlockWrite()
     {
+        bool bElevationPending {};
+
         if constexpr (!bIsWriteRecursionAllowed)
         {
+            this->reentrantWriteLockHandle_ = 0;
+
             {
                 AU_LOCK_GUARD(this->mutex_);
-                this->reentrantWriteLockHandle_ = 0;
                 this->state_ = 0;
+                bElevationPending = this->writersPending_ > 0;
             }
 
-            this->GetConditionWriter().Broadcast();
-            this->GetCondition().Broadcast();
+            if (bElevationPending)
+            {
+                this->GetConditionWriter().Signal();
+            }
+            else
+            {
+                this->GetCondition().Broadcast();
+            }
         }
         else
         {
             AuInt32 val {};
 
-            {
-                AU_LOCK_GUARD(this->mutex_);
-                val = AuAtomicAdd(&this->state_, 1);
-                this->reentrantWriteLockHandle_ = 0;
-            }
+            this->reentrantWriteLockHandle_ = 0;
+            val = AuAtomicAdd(&this->state_, 1);
 
             if (val == 0)
             {
-                this->GetConditionWriter().Broadcast();
-                this->GetCondition().Broadcast();
+                {
+                    AU_LOCK_GUARD(this->mutex_);
+                    bElevationPending = this->writersPending_ > 0;
+                }
+
+                if (bElevationPending)
+                {
+                    this->GetConditionWriter().Signal();
+                }
+                else
+                {
+                    this->GetCondition().Broadcast();
+                }
             }
         }
     }
@@ -375,7 +394,7 @@ namespace Aurora::Threading::Primitives
     
         while (this->state_ != 1)
         {
-            this->bElevaterPending_ = true;
+            AuAtomicAdd(&this->writersPending_, 1);
 
             if (!this->GetConditionWriter().WaitForSignal(timeout))
             {
@@ -383,7 +402,7 @@ namespace Aurora::Threading::Primitives
             }
         }
         
-        this->bElevaterPending_ = false;
+        AuAtomicSub(&this->writersPending_, 1);
         this->reentrantWriteLockHandle_ = GetThreadCookie();
         this->state_ = -1;
         return true;
diff --git a/Source/Threading/Primitives/AuRWLock.hpp b/Source/Threading/Primitives/AuRWLock.hpp
index 97ef5a68..a919d48d 100644
--- a/Source/Threading/Primitives/AuRWLock.hpp
+++ b/Source/Threading/Primitives/AuRWLock.hpp
@@ -93,10 +93,7 @@ namespace Aurora::Threading::Primitives
     #endif
         ThreadCookie_t reentrantWriteLockHandle_ {};
         volatile AuInt32 state_ {};
-        AuInt32 writersPending_ : 31 {};
-        AuInt32 bElevaterPending_ : 1 {};
-        //bool  bElevaterPending_{};
-        //bool reentrantWriteLock_ {true};
+        AuInt32 writersPending_ {};
 
     public:
         cstatic const AuUInt8 kOffsetOfRead = AuOffsetOf(&RWLockImpl::read_);