WaitSynch: Removed unused variables and reduced SharedPtr copies.

Define a variable with the value of the sync timeout error code.

Use a boost::flat_map instead of an unordered_map to hold the equivalence of objects and wait indices in a WaitSynchN call.

src/citra_qt/debugger/wait_tree.cpp

@@ -230,7 +230,7 @@ std::vector<std::unique_ptr<WaitTreeItem>> WaitTreeThread::GetChildren() const {
         list.push_back(std::make_unique<WaitTreeMutexList>(thread.held_mutexes));
     }
     if (thread.status == THREADSTATUS_WAIT_SYNCH) {
-        list.push_back(std::make_unique<WaitTreeObjectList>(thread.wait_objects, thread.IsWaitingAll()));
+        list.push_back(std::make_unique<WaitTreeObjectList>(thread.wait_objects, thread.IsSleepingOnWaitAll()));
     }
 
     return list;

src/core/hle/kernel/kernel.cpp

@@ -34,14 +34,11 @@ void WaitObject::RemoveWaitingThread(Thread* thread) {
 
 SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
     // Remove the threads that are ready or already running from our waitlist
-    boost::range::remove_erase_if(waiting_threads, [](const SharedPtr<Thread>& thread) -> bool {
+    boost::range::remove_erase_if(waiting_threads, [](const SharedPtr<Thread>& thread) {
         return thread->status == THREADSTATUS_RUNNING || thread->status == THREADSTATUS_READY;
     });
 
-    if (waiting_threads.empty())
-        return nullptr;
-
-    SharedPtr<Thread> candidate = nullptr;
+    Thread* candidate = nullptr;
     s32 candidate_priority = THREADPRIO_LOWEST + 1;
 
     for (const auto& thread : waiting_threads) {
@@ -52,7 +49,7 @@ SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
             return object->ShouldWait();
         });
         if (ready_to_run) {
-            candidate = thread;
+            candidate = thread.get();
             candidate_priority = thread->current_priority;
         }
     }
@@ -61,9 +58,8 @@ SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
 }
 
 void WaitObject::WakeupAllWaitingThreads() {
-    // Wake up all threads that can be awoken, in priority order
     while (auto thread = GetHighestPriorityReadyThread()) {
-        if (thread->wait_objects.empty()) {
+        if (!thread->IsSleepingOnWaitAll()) {
             Acquire();
             // Set the output index of the WaitSynchronizationN call to the index of this object.
             if (thread->wait_set_output) {
@@ -73,7 +69,6 @@ void WaitObject::WakeupAllWaitingThreads() {
         } else {
             for (auto object : thread->wait_objects) {
                 object->Acquire();
-                // Remove the thread from the object's waitlist
                 object->RemoveWaitingThread(thread.get());
             }
             // Note: This case doesn't update the output index of WaitSynchronizationN.
@@ -81,7 +76,6 @@ void WaitObject::WakeupAllWaitingThreads() {
             thread->wait_objects.clear();
         }
 
-        // Set the result of the call to WaitSynchronization to RESULT_SUCCESS
         thread->SetWaitSynchronizationResult(RESULT_SUCCESS);
         thread->ResumeFromWait();
         // Note: Removing the thread from the object's waitlist will be done by GetHighestPriorityReadyThread

src/core/hle/kernel/kernel.h

@@ -152,7 +152,10 @@ public:
      */
     void RemoveWaitingThread(Thread* thread);
 
-    /// Wake up all threads waiting on this object
+    /**
+     * Wake up all threads waiting on this object that can be awoken, in priority order,
+     * and set the synchronization result and output of the thread.
+     */
     void WakeupAllWaitingThreads();
 
     /// Obtains the highest priority thread that is ready to run from this object's waiting list.

src/core/hle/kernel/thread.h

@@ -7,6 +7,7 @@
 #include <string>
 #include <unordered_map>
 #include <vector>
+#include <boost/container/flat_map.hpp>
 #include <boost/container/flat_set.hpp>
 #include "common/common_types.h"
 #include "core/core.h"
@@ -153,7 +154,7 @@ public:
      * its wait list to become ready, as a result of a WaitSynchronizationN call
      * with wait_all = true, or a ReplyAndReceive call.
      */
-    bool IsWaitingAll() const {
+    bool IsSleepingOnWaitAll() const {
         return !wait_objects.empty();
     }
 
@@ -183,7 +184,7 @@ public:
     /// This is only populated when the thread should wait for all the objects to become ready.
     std::vector<SharedPtr<WaitObject>> wait_objects;
 
-    std::unordered_map<int, s32> wait_objects_index; ///< Mapping of Object ids to their position in the last waitlist that this object waited on.
+    boost::container::flat_map<int, s32> wait_objects_index; ///< Mapping of Object ids to their position in the last waitlist that this object waited on.
 
     VAddr wait_address;   ///< If waiting on an AddressArbiter, this is the arbitration address
 

src/core/hle/svc.cpp

@@ -41,6 +41,9 @@ const ResultCode ERR_PORT_NAME_TOO_LONG(ErrorDescription(30), ErrorModule::OS,
                                         ErrorSummary::InvalidArgument,
                                         ErrorLevel::Usage); // 0xE0E0181E
 
+const ResultCode ERR_SYNC_TIMEOUT(ErrorDescription::Timeout, ErrorModule::OS,
+                                  ErrorSummary::StatusChanged, ErrorLevel::Info);
+
 const ResultCode ERR_MISALIGNED_ADDRESS{// 0xE0E01BF1
                                         ErrorDescription::MisalignedAddress, ErrorModule::OS,
                                         ErrorSummary::InvalidArgument, ErrorLevel::Usage};
@@ -257,11 +260,8 @@ static ResultCode WaitSynchronization1(Handle handle, s64 nano_seconds) {
 
     if (object->ShouldWait()) {
 
-        if (nano_seconds == 0) {
-            return ResultCode(ErrorDescription::Timeout, ErrorModule::OS,
-                              ErrorSummary::StatusChanged,
-                              ErrorLevel::Info);
-        }
+        if (nano_seconds == 0)
+            return ERR_SYNC_TIMEOUT;
 
         object->AddWaitingThread(thread);
         // TODO(Subv): Perform things like update the mutex lock owner's priority to prevent priority inversion.
@@ -273,9 +273,7 @@ static ResultCode WaitSynchronization1(Handle handle, s64 nano_seconds) {
 
         // Note: The output of this SVC will be set to RESULT_SUCCESS if the thread resumes due to a signal in its wait objects.
         // Otherwise we retain the default value of timeout.
-        return ResultCode(ErrorDescription::Timeout, ErrorModule::OS,
-                               ErrorSummary::StatusChanged,
-                               ErrorLevel::Info);
+        return ERR_SYNC_TIMEOUT;
     }
 
     object->Acquire();
@@ -286,8 +284,6 @@ static ResultCode WaitSynchronization1(Handle handle, s64 nano_seconds) {
 /// Wait for the given handles to synchronize, timeout after the specified nanoseconds
 static ResultCode WaitSynchronizationN(s32* out, Handle* handles, s32 handle_count, bool wait_all,
                                        s64 nano_seconds) {
-    bool wait_thread = !wait_all;
-    int handle_index = 0;
     Kernel::Thread* thread = Kernel::GetCurrentThread();
 
     // Check if 'handles' is invalid
@@ -305,7 +301,6 @@ static ResultCode WaitSynchronizationN(s32* out, Handle* handles, s32 handle_cou
                           ErrorSummary::InvalidArgument, ErrorLevel::Usage);
 
     using ObjectPtr = Kernel::SharedPtr<Kernel::WaitObject>;
-
     std::vector<ObjectPtr> objects(handle_count);
 
     for (int i = 0; i < handle_count; ++i) {
@@ -320,100 +315,90 @@ static ResultCode WaitSynchronizationN(s32* out, Handle* handles, s32 handle_cou
     // It will be repopulated later in the wait_all = false case.
     thread->wait_objects_index.clear();
 
-    if (!wait_all) {
-        // Find the first object that is acquireable in the provided list of objects
-        auto itr = std::find_if(objects.begin(), objects.end(), [](const ObjectPtr& object) {
+    if (wait_all) {
+        bool all_available = std::all_of(objects.begin(), objects.end(), [](const ObjectPtr& object) {
             return !object->ShouldWait();
         });
-
-        if (itr != objects.end()) {
-            // We found a ready object, acquire it and set the result value
-            ObjectPtr object = *itr;
-            object->Acquire();
-            *out = std::distance(objects.begin(), itr);
+        if (all_available) {
+            // We can acquire all objects right now, do so.
+            for (auto object : objects)
+                object->Acquire();
+            // Note: In this case, the `out` parameter is not set, and retains whatever value it had before.
             return RESULT_SUCCESS;
         }
 
-        // No objects were ready to be acquired, prepare to suspend the thread.
+        // Not all objects were available right now, prepare to suspend the thread.
 
         // If a timeout value of 0 was provided, just return the Timeout error code instead of suspending the thread.
-        if (nano_seconds == 0) {
-            return ResultCode(ErrorDescription::Timeout, ErrorModule::OS,
-                              ErrorSummary::StatusChanged,
-                              ErrorLevel::Info);
-        }
+        if (nano_seconds == 0)
+            return ERR_SYNC_TIMEOUT;
 
         // Put the thread to sleep
         thread->status = THREADSTATUS_WAIT_SYNCH;
 
-        // Clear the thread's waitlist, we won't use it for wait_all = false
-        thread->wait_objects.clear();
-
         // Add the thread to each of the objects' waiting threads.
-        for (size_t i = 0; i < objects.size(); ++i) {
-            ObjectPtr object = objects[i];
-            // Set the index of this object in the mapping of Objects -> index for this thread.
-            thread->wait_objects_index[object->GetObjectId()] = static_cast<int>(i);
+        for (auto& object : objects) {
             object->AddWaitingThread(thread);
             // TODO(Subv): Perform things like update the mutex lock owner's priority to prevent priority inversion.
             // Currently this is done in Mutex::ShouldWait, but it should be moved to a function that is called from here.
         }
 
-        // Note: If no handles and no timeout were given, then the thread will deadlock, this is consistent with hardware behavior.
+        // Set the thread's waitlist to the list of objects passed to WaitSynchronizationN
+        thread->wait_objects = std::move(objects);
 
         // Create an event to wake the thread up after the specified nanosecond delay has passed
         thread->WakeAfterDelay(nano_seconds);
 
-        // Note: The output of this SVC will be set to RESULT_SUCCESS if the thread resumes due to a signal in one of its wait objects.
-        // Otherwise we retain the default value of timeout, and -1 in the out parameter
-        thread->wait_set_output = true;
+        // This value gets set to -1 by default in this case, it is not modified after this.
         *out = -1;
-        return ResultCode(ErrorDescription::Timeout, ErrorModule::OS,
-                          ErrorSummary::StatusChanged,
-                          ErrorLevel::Info);
+        // Note: The output of this SVC will be set to RESULT_SUCCESS if the thread resumes due to a signal in one of its wait objects.
+        return ERR_SYNC_TIMEOUT;
     } else {
-        bool all_available = std::all_of(objects.begin(), objects.end(), [](const ObjectPtr& object) {
+        // Find the first object that is acquirable in the provided list of objects
+        auto itr = std::find_if(objects.begin(), objects.end(), [](const ObjectPtr& object) {
             return !object->ShouldWait();
         });
-        if (all_available) {
-            // We can acquire all objects right now, do so.
-            for (auto object : objects)
-                object->Acquire();
-            // Note: In this case, the `out` parameter is not set, and retains whatever value it had before.
+
+        if (itr != objects.end()) {
+            // We found a ready object, acquire it and set the result value
+            Kernel::WaitObject* object = itr->get();
+            object->Acquire();
+            *out = std::distance(objects.begin(), itr);
             return RESULT_SUCCESS;
         }
 
-        // Not all objects were available right now, prepare to suspend the thread.
+        // No objects were ready to be acquired, prepare to suspend the thread.
 
         // If a timeout value of 0 was provided, just return the Timeout error code instead of suspending the thread.
-        if (nano_seconds == 0) {
-            return ResultCode(ErrorDescription::Timeout, ErrorModule::OS,
-                              ErrorSummary::StatusChanged,
-                              ErrorLevel::Info);
-        }
+        if (nano_seconds == 0)
+            return ERR_SYNC_TIMEOUT;
 
         // Put the thread to sleep
         thread->status = THREADSTATUS_WAIT_SYNCH;
 
-        // Set the thread's waitlist to the list of objects passed to WaitSynchronizationN
-        thread->wait_objects = objects;
+        // Clear the thread's waitlist, we won't use it for wait_all = false
+        thread->wait_objects.clear();
 
         // Add the thread to each of the objects' waiting threads.
-        for (auto object : objects) {
+        for (size_t i = 0; i < objects.size(); ++i) {
+            Kernel::WaitObject* object = objects[i].get();
+            // Set the index of this object in the mapping of Objects -> index for this thread.
+            thread->wait_objects_index[object->GetObjectId()] = static_cast<int>(i);
             object->AddWaitingThread(thread);
             // TODO(Subv): Perform things like update the mutex lock owner's priority to prevent priority inversion.
             // Currently this is done in Mutex::ShouldWait, but it should be moved to a function that is called from here.
         }
 
+        // Note: If no handles and no timeout were given, then the thread will deadlock, this is consistent with hardware behavior.
+
         // Create an event to wake the thread up after the specified nanosecond delay has passed
         thread->WakeAfterDelay(nano_seconds);
 
-        // This value gets set to -1 by default in this case, it is not modified after this.
-        *out = -1;
         // Note: The output of this SVC will be set to RESULT_SUCCESS if the thread resumes due to a signal in one of its wait objects.
-        return ResultCode(ErrorDescription::Timeout, ErrorModule::OS,
-                          ErrorSummary::StatusChanged,
-                          ErrorLevel::Info);
+        // Otherwise we retain the default value of timeout, and -1 in the out parameter
+        thread->wait_set_output = true;
+        *out = -1;
+        return ERR_SYNC_TIMEOUT;
     }
 }