From 1f7dd36499786d373b143a4437d4c32e077a32aa Mon Sep 17 00:00:00 2001
From: Fernando Sahmkow <fsahmkow27@gmail.com>
Date: Mon, 10 Feb 2020 14:45:08 -0400
Subject: [PATCH] Common/Tests: Address Feedback

---
 src/common/fiber.cpp           |  5 ++---
 src/common/fiber.h             |  8 ++++----
 src/common/spin_lock.cpp       |  3 ++-
 src/core/core_timing_util.cpp  | 14 ++++++++++++--
 src/core/core_timing_util.h    |  2 ++
 src/core/host_timing.cpp       |  4 ++--
 src/core/host_timing.h         |  6 +++---
 src/tests/common/fibers.cpp    | 20 ++++++++++----------
 src/tests/core/host_timing.cpp | 28 ++++++++++++++--------------
 9 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/src/common/fiber.cpp b/src/common/fiber.cpp
index 050c93acba..1220eddf05 100644
--- a/src/common/fiber.cpp
+++ b/src/common/fiber.cpp
@@ -32,13 +32,12 @@ void __stdcall Fiber::FiberStartFunc(void* fiber_parameter) {
 }
 
 Fiber::Fiber(std::function<void(void*)>&& entry_point_func, void* start_parameter)
-    : guard{}, entry_point{std::move(entry_point_func)}, start_parameter{start_parameter},
-      previous_fiber{} {
+    : entry_point{std::move(entry_point_func)}, start_parameter{start_parameter} {
     impl = std::make_unique<FiberImpl>();
     impl->handle = CreateFiber(0, &FiberStartFunc, this);
 }
 
-Fiber::Fiber() : guard{}, entry_point{}, start_parameter{}, previous_fiber{} {
+Fiber::Fiber() {
     impl = std::make_unique<FiberImpl>();
 }
 
diff --git a/src/common/fiber.h b/src/common/fiber.h
index 598fe7daaf..7e3b130a46 100644
--- a/src/common/fiber.h
+++ b/src/common/fiber.h
@@ -67,10 +67,10 @@ private:
 
     struct FiberImpl;
 
-    SpinLock guard;
-    std::function<void(void*)> entry_point;
-    void* start_parameter;
-    std::shared_ptr<Fiber> previous_fiber;
+    SpinLock guard{};
+    std::function<void(void*)> entry_point{};
+    void* start_parameter{};
+    std::shared_ptr<Fiber> previous_fiber{};
     std::unique_ptr<FiberImpl> impl;
     bool is_thread_fiber{};
 };
diff --git a/src/common/spin_lock.cpp b/src/common/spin_lock.cpp
index 82a1d39fff..c7b46aac6b 100644
--- a/src/common/spin_lock.cpp
+++ b/src/common/spin_lock.cpp
@@ -35,8 +35,9 @@ void thread_pause() {
 namespace Common {
 
 void SpinLock::lock() {
-    while (lck.test_and_set(std::memory_order_acquire))
+    while (lck.test_and_set(std::memory_order_acquire)) {
         thread_pause();
+    }
 }
 
 void SpinLock::unlock() {
diff --git a/src/core/core_timing_util.cpp b/src/core/core_timing_util.cpp
index f42666b4db..be34b26fe4 100644
--- a/src/core/core_timing_util.cpp
+++ b/src/core/core_timing_util.cpp
@@ -49,9 +49,19 @@ s64 nsToCycles(std::chrono::nanoseconds ns) {
     return (Hardware::BASE_CLOCK_RATE * ns.count()) / 1000000000;
 }
 
+u64 msToClockCycles(std::chrono::milliseconds ns) {
+    const u128 temp = Common::Multiply64Into128(ns.count(), Hardware::CNTFREQ);
+    return Common::Divide128On32(temp, 1000).first;
+}
+
+u64 usToClockCycles(std::chrono::microseconds ns) {
+    const u128 temp = Common::Multiply64Into128(ns.count(), Hardware::CNTFREQ);
+    return Common::Divide128On32(temp, 1000000).first;
+}
+
 u64 nsToClockCycles(std::chrono::nanoseconds ns) {
-    const u128 temporal = Common::Multiply64Into128(ns.count(), CNTFREQ);
-    return Common::Divide128On32(temporal, 1000000000).first;
+    const u128 temp = Common::Multiply64Into128(ns.count(), Hardware::CNTFREQ);
+    return Common::Divide128On32(temp, 1000000000).first;
 }
 
 u64 CpuCyclesToClockCycles(u64 ticks) {
diff --git a/src/core/core_timing_util.h b/src/core/core_timing_util.h
index 65fb7368b6..b3c58447d5 100644
--- a/src/core/core_timing_util.h
+++ b/src/core/core_timing_util.h
@@ -13,6 +13,8 @@ namespace Core::Timing {
 s64 msToCycles(std::chrono::milliseconds ms);
 s64 usToCycles(std::chrono::microseconds us);
 s64 nsToCycles(std::chrono::nanoseconds ns);
+u64 msToClockCycles(std::chrono::milliseconds ns);
+u64 usToClockCycles(std::chrono::microseconds ns);
 u64 nsToClockCycles(std::chrono::nanoseconds ns);
 
 inline std::chrono::milliseconds CyclesToMs(s64 cycles) {
diff --git a/src/core/host_timing.cpp b/src/core/host_timing.cpp
index c734a118e4..be80d9f8ed 100644
--- a/src/core/host_timing.cpp
+++ b/src/core/host_timing.cpp
@@ -76,11 +76,11 @@ void CoreTiming::SyncPause(bool is_paused) {
         ;
 }
 
-bool CoreTiming::IsRunning() {
+bool CoreTiming::IsRunning() const {
     return !paused_set;
 }
 
-bool CoreTiming::HasPendingEvents() {
+bool CoreTiming::HasPendingEvents() const {
     return !(wait_set && event_queue.empty());
 }
 
diff --git a/src/core/host_timing.h b/src/core/host_timing.h
index 15a150904a..679fcf491a 100644
--- a/src/core/host_timing.h
+++ b/src/core/host_timing.h
@@ -72,15 +72,15 @@ public:
     void SyncPause(bool is_paused);
 
     /// Checks if core timing is running.
-    bool IsRunning();
+    bool IsRunning() const;
 
     /// Checks if the timer thread has started.
-    bool HasStarted() {
+    bool HasStarted() const {
         return has_started;
     }
 
     /// Checks if there are any pending time events.
-    bool HasPendingEvents();
+    bool HasPendingEvents() const;
 
     /// Schedules an event in core timing
     void ScheduleEvent(s64 ns_into_future, const std::shared_ptr<EventType>& event_type,
diff --git a/src/tests/common/fibers.cpp b/src/tests/common/fibers.cpp
index d63194dd4c..0d3d5153d6 100644
--- a/src/tests/common/fibers.cpp
+++ b/src/tests/common/fibers.cpp
@@ -34,7 +34,7 @@ public:
 };
 
 static void WorkControl1(void* control) {
-    TestControl1* test_control = static_cast<TestControl1*>(control);
+    auto* test_control = static_cast<TestControl1*>(control);
     test_control->DoWork();
 }
 
@@ -70,8 +70,8 @@ static void ThreadStart1(u32 id, TestControl1& test_control) {
 TEST_CASE("Fibers::Setup", "[common]") {
     constexpr u32 num_threads = 7;
     TestControl1 test_control{};
-    test_control.thread_fibers.resize(num_threads, nullptr);
-    test_control.work_fibers.resize(num_threads, nullptr);
+    test_control.thread_fibers.resize(num_threads);
+    test_control.work_fibers.resize(num_threads);
     test_control.items.resize(num_threads, 0);
     test_control.results.resize(num_threads, 0);
     std::vector<std::thread> threads;
@@ -153,17 +153,17 @@ public:
 };
 
 static void WorkControl2_1(void* control) {
-    TestControl2* test_control = static_cast<TestControl2*>(control);
+    auto* test_control = static_cast<TestControl2*>(control);
     test_control->DoWork1();
 }
 
 static void WorkControl2_2(void* control) {
-    TestControl2* test_control = static_cast<TestControl2*>(control);
+    auto* test_control = static_cast<TestControl2*>(control);
     test_control->DoWork2();
 }
 
 static void WorkControl2_3(void* control) {
-    TestControl2* test_control = static_cast<TestControl2*>(control);
+    auto* test_control = static_cast<TestControl2*>(control);
     test_control->DoWork3();
 }
 
@@ -198,7 +198,7 @@ static void ThreadStart2_2(u32 id, TestControl2& test_control) {
  */
 TEST_CASE("Fibers::InterExchange", "[common]") {
     TestControl2 test_control{};
-    test_control.thread_fibers.resize(2, nullptr);
+    test_control.thread_fibers.resize(2);
     test_control.fiber1 =
         std::make_shared<Fiber>(std::function<void(void*)>{WorkControl2_1}, &test_control);
     test_control.fiber2 =
@@ -261,12 +261,12 @@ public:
 };
 
 static void WorkControl3_1(void* control) {
-    TestControl3* test_control = static_cast<TestControl3*>(control);
+    auto* test_control = static_cast<TestControl3*>(control);
     test_control->DoWork1();
 }
 
 static void WorkControl3_2(void* control) {
-    TestControl3* test_control = static_cast<TestControl3*>(control);
+    auto* test_control = static_cast<TestControl3*>(control);
     test_control->DoWork2();
 }
 
@@ -295,7 +295,7 @@ static void ThreadStart3(u32 id, TestControl3& test_control) {
  */
 TEST_CASE("Fibers::StartRace", "[common]") {
     TestControl3 test_control{};
-    test_control.thread_fibers.resize(2, nullptr);
+    test_control.thread_fibers.resize(2);
     test_control.fiber1 =
         std::make_shared<Fiber>(std::function<void(void*)>{WorkControl3_1}, &test_control);
     test_control.fiber2 =
diff --git a/src/tests/core/host_timing.cpp b/src/tests/core/host_timing.cpp
index 3d0532d02b..ed060be55c 100644
--- a/src/tests/core/host_timing.cpp
+++ b/src/tests/core/host_timing.cpp
@@ -50,13 +50,13 @@ struct ScopeInit final {
 TEST_CASE("HostTiming[BasicOrder]", "[core]") {
     ScopeInit guard;
     auto& core_timing = guard.core_timing;
-    std::vector<std::shared_ptr<Core::HostTiming::EventType>> events;
-    events.resize(5);
-    events[0] = Core::HostTiming::CreateEvent("callbackA", HostCallbackTemplate<0>);
-    events[1] = Core::HostTiming::CreateEvent("callbackB", HostCallbackTemplate<1>);
-    events[2] = Core::HostTiming::CreateEvent("callbackC", HostCallbackTemplate<2>);
-    events[3] = Core::HostTiming::CreateEvent("callbackD", HostCallbackTemplate<3>);
-    events[4] = Core::HostTiming::CreateEvent("callbackE", HostCallbackTemplate<4>);
+    std::vector<std::shared_ptr<Core::HostTiming::EventType>> events{
+        Core::HostTiming::CreateEvent("callbackA", HostCallbackTemplate<0>),
+        Core::HostTiming::CreateEvent("callbackB", HostCallbackTemplate<1>),
+        Core::HostTiming::CreateEvent("callbackC", HostCallbackTemplate<2>),
+        Core::HostTiming::CreateEvent("callbackD", HostCallbackTemplate<3>),
+        Core::HostTiming::CreateEvent("callbackE", HostCallbackTemplate<4>),
+    };
 
     expected_callback = 0;
 
@@ -100,13 +100,13 @@ u64 TestTimerSpeed(Core::HostTiming::CoreTiming& core_timing) {
 TEST_CASE("HostTiming[BasicOrderNoPausing]", "[core]") {
     ScopeInit guard;
     auto& core_timing = guard.core_timing;
-    std::vector<std::shared_ptr<Core::HostTiming::EventType>> events;
-    events.resize(5);
-    events[0] = Core::HostTiming::CreateEvent("callbackA", HostCallbackTemplate<0>);
-    events[1] = Core::HostTiming::CreateEvent("callbackB", HostCallbackTemplate<1>);
-    events[2] = Core::HostTiming::CreateEvent("callbackC", HostCallbackTemplate<2>);
-    events[3] = Core::HostTiming::CreateEvent("callbackD", HostCallbackTemplate<3>);
-    events[4] = Core::HostTiming::CreateEvent("callbackE", HostCallbackTemplate<4>);
+    std::vector<std::shared_ptr<Core::HostTiming::EventType>> events{
+        Core::HostTiming::CreateEvent("callbackA", HostCallbackTemplate<0>),
+        Core::HostTiming::CreateEvent("callbackB", HostCallbackTemplate<1>),
+        Core::HostTiming::CreateEvent("callbackC", HostCallbackTemplate<2>),
+        Core::HostTiming::CreateEvent("callbackD", HostCallbackTemplate<3>),
+        Core::HostTiming::CreateEvent("callbackE", HostCallbackTemplate<4>),
+    };
 
     core_timing.SyncPause(true);
     core_timing.SyncPause(false);