From 871b01a5d91f2812f98b6ed58f024713e3b1ebe1 Mon Sep 17 00:00:00 2001 From: nyanpasu64 Date: Sun, 27 Mar 2022 22:25:40 -0700 Subject: [PATCH 1/3] Remove unnecessary atomic usage in GCAdapter.cpp You can safely read or write non-atomic integers on multiple threads, as long as every thread reading or writing it holds the same mutex while doing so (here, s_mutex). Removing the atomic accesses makes the code faster, but the actual performance difference is probably negligible. --- Source/Core/InputCommon/GCAdapter.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Source/Core/InputCommon/GCAdapter.cpp b/Source/Core/InputCommon/GCAdapter.cpp index 973d1a1d5f..059b17a5e0 100644 --- a/Source/Core/InputCommon/GCAdapter.cpp +++ b/Source/Core/InputCommon/GCAdapter.cpp @@ -51,7 +51,8 @@ static std::mutex s_mutex; static u8 s_controller_payload[37]; static u8 s_controller_payload_swap[37]; -static std::atomic s_controller_payload_size = {0}; +// Only access with s_mutex held! +static int s_controller_payload_size = {0}; static std::thread s_adapter_input_thread; static std::thread s_adapter_output_thread; @@ -101,7 +102,7 @@ static void Read() { std::lock_guard lk(s_mutex); std::swap(s_controller_payload_swap, s_controller_payload); - s_controller_payload_size.store(payload_size); + s_controller_payload_size = payload_size; } Common::YieldCPU(); @@ -459,7 +460,7 @@ GCPadStatus Input(int chan) std::lock_guard lk(s_mutex); std::copy(std::begin(s_controller_payload), std::end(s_controller_payload), std::begin(controller_payload_copy)); - payload_size = s_controller_payload_size.load(); + payload_size = s_controller_payload_size; } GCPadStatus pad = {}; From 76160276848bd2bbfa6be5335c9cd374ead49a58 Mon Sep 17 00:00:00 2001 From: nyanpasu64 Date: Sun, 27 Mar 2022 22:27:44 -0700 Subject: [PATCH 2/3] Remove unnecessary atomic usage in GCAdapter_Android.cpp s_controller_write_payload_size needs to remain an atomic because Read() loads and stores without holding a mutex, Output() stores while holding s_write_mutex, and ResetRumble() stores while holding s_read_mutex! I'm pretty sure this code is wrong, specifically ResetRumble(). --- Source/Core/InputCommon/GCAdapter_Android.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/InputCommon/GCAdapter_Android.cpp b/Source/Core/InputCommon/GCAdapter_Android.cpp index 1e58733236..b129eaaf0f 100644 --- a/Source/Core/InputCommon/GCAdapter_Android.cpp +++ b/Source/Core/InputCommon/GCAdapter_Android.cpp @@ -41,7 +41,7 @@ static u8 s_controller_rumble[4]; // Input handling static std::mutex s_read_mutex; static std::array s_controller_payload; -static std::atomic s_controller_payload_size{0}; +static int s_controller_payload_size{0}; // Output handling static std::mutex s_write_mutex; @@ -164,7 +164,7 @@ static void Read() { std::lock_guard lk(s_read_mutex); std::copy(java_data, java_data + s_controller_payload.size(), s_controller_payload.begin()); - s_controller_payload_size.store(read_size); + s_controller_payload_size = read_size; } env->ReleaseByteArrayElements(*java_controller_payload, java_data, 0); @@ -284,7 +284,7 @@ GCPadStatus Input(int chan) { std::lock_guard lk(s_read_mutex); controller_payload_copy = s_controller_payload; - payload_size = s_controller_payload_size.load(); + payload_size = s_controller_payload_size; } GCPadStatus pad = {}; From b5a7ae52b56abd1b3aaa37786debcd95cc4e6057 Mon Sep 17 00:00:00 2001 From: nyanpasu64 Date: Sun, 27 Mar 2022 22:37:43 -0700 Subject: [PATCH 3/3] Fix locking the wrong mutex in GCAdapter_Android.cpp ResetRumble() I am not confident there are no race conditions between s_write_mutex, s_controller_write_payload_size, and s_controller_write_payload. But this code should be safer than before. --- Source/Core/InputCommon/GCAdapter_Android.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/InputCommon/GCAdapter_Android.cpp b/Source/Core/InputCommon/GCAdapter_Android.cpp index b129eaaf0f..499a9ece30 100644 --- a/Source/Core/InputCommon/GCAdapter_Android.cpp +++ b/Source/Core/InputCommon/GCAdapter_Android.cpp @@ -406,7 +406,7 @@ void ResetRumble() { unsigned char rumble[5] = {0x11, 0, 0, 0, 0}; { - std::lock_guard lk(s_read_mutex); + std::lock_guard lk(s_write_mutex); memcpy(s_controller_write_payload, rumble, 5); s_controller_write_payload_size.store(5); }