From b88b188819f1219120c77273a4d497461cd7e5e7 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Fri, 26 Aug 2016 02:47:34 +0000 Subject: [PATCH 1/9] CoreTiming: Cleanup naming conventions --- Source/Core/Core/CoreTiming.cpp | 231 +++++++++--------- Source/Core/Core/CoreTiming.h | 10 +- .../Core/PowerPC/Interpreter/Interpreter.cpp | 2 +- .../PowerPC/Jit64/Jit_SystemRegisters.cpp | 10 +- .../JitArm64/JitArm64_SystemRegisters.cpp | 6 +- 5 files changed, 128 insertions(+), 131 deletions(-) diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp index 0accb2e66c..1f1b711c3a 100644 --- a/Source/Core/Core/CoreTiming.cpp +++ b/Source/Core/Core/CoreTiming.cpp @@ -2,6 +2,7 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include #include #include #include @@ -20,8 +21,6 @@ #include "VideoCommon/Fifo.h" #include "VideoCommon/VideoBackendBase.h" -#define MAX_SLICE_LENGTH 20000 - namespace CoreTiming { struct EventType @@ -30,7 +29,7 @@ struct EventType std::string name; }; -static std::vector event_types; +static std::vector s_event_types; struct BaseEvent { @@ -42,45 +41,45 @@ struct BaseEvent typedef LinkedListItem Event; // STATE_TO_SAVE -static Event* first; -static std::mutex tsWriteLock; -static Common::FifoQueue tsQueue; +static Event* s_first_event; +static std::mutex s_ts_write_lock; +static Common::FifoQueue s_ts_queue; // event pools -static Event* eventPool = nullptr; +static Event* s_event_pool = nullptr; -static float s_lastOCFactor; -float g_lastOCFactor_inverted; -int g_slicelength; -static int maxslicelength = MAX_SLICE_LENGTH; +static float s_last_OC_factor; +float g_last_OC_factor_inverted; +int g_slice_length; +static constexpr int MAX_SLICE_LENGTH = 20000; -static s64 idledCycles; -static u32 fakeDecStartValue; -static u64 fakeDecStartTicks; +static s64 s_idled_cycles; +static u32 s_fake_dec_start_value; +static u64 s_fake_dec_start_ticks; // Are we in a function that has been called from Advance() -static bool globalTimerIsSane; +static bool s_is_global_timer_sane; -s64 g_globalTimer; -u64 g_fakeTBStartValue; -u64 g_fakeTBStartTicks; +s64 g_global_timer; +u64 g_fake_TB_start_value; +u64 g_fake_TB_start_ticks; -static int ev_lost; +static int s_ev_lost; static Event* GetNewEvent() { - if (!eventPool) + if (!s_event_pool) return new Event; - Event* ev = eventPool; - eventPool = ev->next; + Event* ev = s_event_pool; + s_event_pool = ev->next; return ev; } static void FreeEvent(Event* ev) { - ev->next = eventPool; - eventPool = ev; + ev->next = s_event_pool; + s_event_pool = ev; } static void EmptyTimedCallback(u64 userdata, s64 cyclesLate) @@ -96,12 +95,12 @@ static void EmptyTimedCallback(u64 userdata, s64 cyclesLate) // but the effect is largely the same. static int DowncountToCycles(int downcount) { - return (int)(downcount * g_lastOCFactor_inverted); + return static_cast(downcount * g_last_OC_factor_inverted); } static int CyclesToDowncount(int cycles) { - return (int)(cycles * s_lastOCFactor); + return static_cast(cycles * s_last_OC_factor); } int RegisterEvent(const std::string& name, TimedCallback callback) @@ -112,7 +111,7 @@ int RegisterEvent(const std::string& name, TimedCallback callback) // check for existing type with same name. // we want event type names to remain unique so that we can use them for serialization. - for (auto& event_type : event_types) + for (auto& event_type : s_event_types) { if (name == event_type.name) { @@ -127,41 +126,41 @@ int RegisterEvent(const std::string& name, TimedCallback callback) } } - event_types.push_back(type); - return (int)event_types.size() - 1; + s_event_types.push_back(type); + return static_cast(s_event_types.size() - 1); } void UnregisterAllEvents() { - if (first) + if (s_first_event) PanicAlert("Cannot unregister events with events pending"); - event_types.clear(); + s_event_types.clear(); } void Init() { - s_lastOCFactor = SConfig::GetInstance().m_OCEnable ? SConfig::GetInstance().m_OCFactor : 1.0f; - g_lastOCFactor_inverted = 1.0f / s_lastOCFactor; - PowerPC::ppcState.downcount = CyclesToDowncount(maxslicelength); - g_slicelength = maxslicelength; - g_globalTimer = 0; - idledCycles = 0; - globalTimerIsSane = true; + s_last_OC_factor = SConfig::GetInstance().m_OCEnable ? SConfig::GetInstance().m_OCFactor : 1.0f; + g_last_OC_factor_inverted = 1.0f / s_last_OC_factor; + PowerPC::ppcState.downcount = CyclesToDowncount(MAX_SLICE_LENGTH); + g_slice_length = MAX_SLICE_LENGTH; + g_global_timer = 0; + s_idled_cycles = 0; + s_is_global_timer_sane = true; - ev_lost = RegisterEvent("_lost_event", &EmptyTimedCallback); + s_ev_lost = RegisterEvent("_lost_event", &EmptyTimedCallback); } void Shutdown() { - std::lock_guard lk(tsWriteLock); + std::lock_guard lk(s_ts_write_lock); MoveEvents(); ClearPendingEvents(); UnregisterAllEvents(); - while (eventPool) + while (s_event_pool) { - Event* ev = eventPool; - eventPool = ev->next; + Event* ev = s_event_pool; + s_event_pool = ev->next; delete ev; } } @@ -178,15 +177,15 @@ static void EventDoState(PointerWrap& p, BaseEvent* ev) // so, we savestate the event's type's name, and derive ev->type from that when loading. std::string name; if (p.GetMode() != PointerWrap::MODE_READ) - name = event_types[ev->type].name; + name = s_event_types[ev->type].name; p.Do(name); if (p.GetMode() == PointerWrap::MODE_READ) { bool foundMatch = false; - for (unsigned int i = 0; i < event_types.size(); ++i) + for (unsigned int i = 0; i < s_event_types.size(); ++i) { - if (name == event_types[i].name) + if (name == s_event_types[i].name) { ev->type = i; foundMatch = true; @@ -198,30 +197,29 @@ static void EventDoState(PointerWrap& p, BaseEvent* ev) WARN_LOG(POWERPC, "Lost event from savestate because its type, \"%s\", has not been registered.", name.c_str()); - ev->type = ev_lost; + ev->type = s_ev_lost; } } } void DoState(PointerWrap& p) { - std::lock_guard lk(tsWriteLock); - p.Do(g_slicelength); - p.Do(g_globalTimer); - p.Do(idledCycles); - p.Do(fakeDecStartValue); - p.Do(fakeDecStartTicks); - p.Do(g_fakeTBStartValue); - p.Do(g_fakeTBStartTicks); - p.Do(s_lastOCFactor); - if (p.GetMode() == PointerWrap::MODE_READ) - g_lastOCFactor_inverted = 1.0f / s_lastOCFactor; + std::lock_guard lk(s_ts_write_lock); + p.Do(g_slice_length); + p.Do(g_global_timer); + p.Do(s_idled_cycles); + p.Do(s_fake_dec_start_value); + p.Do(s_fake_dec_start_ticks); + p.Do(g_fake_TB_start_value); + p.Do(g_fake_TB_start_ticks); + p.Do(s_last_OC_factor); + g_last_OC_factor_inverted = 1.0f / s_last_OC_factor; p.DoMarker("CoreTimingData"); MoveEvents(); - p.DoLinkedList(first); + p.DoLinkedList(s_first_event); p.DoMarker("CoreTimingEvents"); } @@ -229,34 +227,34 @@ void DoState(PointerWrap& p) // it from any other thread, you are doing something evil u64 GetTicks() { - u64 ticks = (u64)g_globalTimer; - if (!globalTimerIsSane) + u64 ticks = static_cast(g_global_timer); + if (!s_is_global_timer_sane) { int downcount = DowncountToCycles(PowerPC::ppcState.downcount); - ticks += g_slicelength - downcount; + ticks += g_slice_length - downcount; } return ticks; } u64 GetIdleTicks() { - return (u64)idledCycles; + return static_cast(s_idled_cycles); } void ClearPendingEvents() { - while (first) + while (s_first_event) { - Event* e = first->next; - FreeEvent(first); - first = e; + Event* e = s_first_event->next; + FreeEvent(s_first_event); + s_first_event = e; } } static void AddEventToQueue(Event* ne) { Event* prev = nullptr; - Event** pNext = &first; + Event** pNext = &s_first_event; for (;;) { Event*& next = *pNext; @@ -293,7 +291,7 @@ void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThr ne->type = event_type; // If this event needs to be scheduled before the next advance(), force one early - if (!globalTimerIsSane) + if (!s_is_global_timer_sane) ForceExceptionCheck(cycles_into_future); AddEventToQueue(ne); @@ -304,31 +302,31 @@ void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThr { ERROR_LOG(POWERPC, "Someone scheduled an off-thread \"%s\" event while netplay or " "movie play/record was active. This is likely to cause a desync.", - event_types[event_type].name.c_str()); + s_event_types[event_type].name.c_str()); } - std::lock_guard lk(tsWriteLock); + std::lock_guard lk(s_ts_write_lock); Event ne; - ne.time = g_globalTimer + cycles_into_future; + ne.time = g_global_timer + cycles_into_future; ne.type = event_type; ne.userdata = userdata; - tsQueue.Push(ne); + s_ts_queue.Push(ne); } } void RemoveEvent(int event_type) { - while (first && first->type == event_type) + while (s_first_event && s_first_event->type == event_type) { - Event* next = first->next; - FreeEvent(first); - first = next; + Event* next = s_first_event->next; + FreeEvent(s_first_event); + s_first_event = next; } - if (!first) + if (!s_first_event) return; - Event* prev = first; + Event* prev = s_first_event; Event* ptr = prev->next; while (ptr) { @@ -354,20 +352,19 @@ void RemoveAllEvents(int event_type) void ForceExceptionCheck(s64 cycles) { - if (s64(DowncountToCycles(PowerPC::ppcState.downcount)) > cycles) + if (DowncountToCycles(PowerPC::ppcState.downcount) > cycles) { // downcount is always (much) smaller than MAX_INT so we can safely cast cycles to an int here. - g_slicelength -= - (DowncountToCycles(PowerPC::ppcState.downcount) - - (int)cycles); // Account for cycles already executed by adjusting the g_slicelength - PowerPC::ppcState.downcount = CyclesToDowncount((int)cycles); + // Account for cycles already executed by adjusting the g_slice_length + g_slice_length -= (DowncountToCycles(PowerPC::ppcState.downcount) - static_cast(cycles)); + PowerPC::ppcState.downcount = CyclesToDowncount(static_cast(cycles)); } } void MoveEvents() { BaseEvent sevt; - while (tsQueue.Pop(sevt)) + while (s_ts_queue.Pop(sevt)) { Event* evt = GetNewEvent(); evt->time = sevt.time; @@ -381,35 +378,35 @@ void Advance() { MoveEvents(); - int cyclesExecuted = g_slicelength - DowncountToCycles(PowerPC::ppcState.downcount); - g_globalTimer += cyclesExecuted; - s_lastOCFactor = SConfig::GetInstance().m_OCEnable ? SConfig::GetInstance().m_OCFactor : 1.0f; - g_lastOCFactor_inverted = 1.0f / s_lastOCFactor; - g_slicelength = maxslicelength; + int cyclesExecuted = g_slice_length - DowncountToCycles(PowerPC::ppcState.downcount); + g_global_timer += cyclesExecuted; + s_last_OC_factor = SConfig::GetInstance().m_OCEnable ? SConfig::GetInstance().m_OCFactor : 1.0f; + g_last_OC_factor_inverted = 1.0f / s_last_OC_factor; + g_slice_length = MAX_SLICE_LENGTH; - globalTimerIsSane = true; + s_is_global_timer_sane = true; - while (first && first->time <= g_globalTimer) + while (s_first_event && s_first_event->time <= g_global_timer) { // LOG(POWERPC, "[Scheduler] %s (%lld, %lld) ", - // event_types[first->type].name ? event_types[first->type].name : "?", - // (u64)g_globalTimer, (u64)first->time); - Event* evt = first; - first = first->next; - event_types[evt->type].callback(evt->userdata, (int)(g_globalTimer - evt->time)); + // s_event_types[s_first_event->type].name ? s_event_types[s_first_event->type].name + // : "?", + // (u64)g_global_timer, (u64)s_first_event->time); + Event* evt = s_first_event; + s_first_event = s_first_event->next; + s_event_types[evt->type].callback(evt->userdata, g_global_timer - evt->time); FreeEvent(evt); } - globalTimerIsSane = false; + s_is_global_timer_sane = false; - if (first) + if (s_first_event) { - g_slicelength = (int)(first->time - g_globalTimer); - if (g_slicelength > maxslicelength) - g_slicelength = maxslicelength; + g_slice_length = + static_cast(std::min(s_first_event->time - g_global_timer, MAX_SLICE_LENGTH)); } - PowerPC::ppcState.downcount = CyclesToDowncount(g_slicelength); + PowerPC::ppcState.downcount = CyclesToDowncount(g_slice_length); // Check for any external exceptions. // It's important to do this after processing events otherwise any exceptions will be delayed @@ -420,10 +417,10 @@ void Advance() void LogPendingEvents() { - Event* ptr = first; + Event* ptr = s_first_event; while (ptr) { - INFO_LOG(POWERPC, "PENDING: Now: %" PRId64 " Pending: %" PRId64 " Type: %d", g_globalTimer, + INFO_LOG(POWERPC, "PENDING: Now: %" PRId64 " Pending: %" PRId64 " Type: %d", g_global_timer, ptr->time, ptr->type); ptr = ptr->next; } @@ -439,22 +436,22 @@ void Idle() Fifo::FlushGpu(); } - idledCycles += DowncountToCycles(PowerPC::ppcState.downcount); + s_idled_cycles += DowncountToCycles(PowerPC::ppcState.downcount); PowerPC::ppcState.downcount = 0; } std::string GetScheduledEventsSummary() { - Event* ptr = first; + Event* ptr = s_first_event; std::string text = "Scheduled events\n"; text.reserve(1000); while (ptr) { unsigned int t = ptr->type; - if (t >= event_types.size()) + if (t >= s_event_types.size()) PanicAlertT("Invalid event type %i", t); - const std::string& name = event_types[ptr->type].name; + const std::string& name = s_event_types[ptr->type].name; text += StringFromFormat("%s : %" PRIi64 " %016" PRIx64 "\n", name.c_str(), ptr->time, ptr->userdata); @@ -465,42 +462,42 @@ std::string GetScheduledEventsSummary() u32 GetFakeDecStartValue() { - return fakeDecStartValue; + return s_fake_dec_start_value; } void SetFakeDecStartValue(u32 val) { - fakeDecStartValue = val; + s_fake_dec_start_value = val; } u64 GetFakeDecStartTicks() { - return fakeDecStartTicks; + return s_fake_dec_start_ticks; } void SetFakeDecStartTicks(u64 val) { - fakeDecStartTicks = val; + s_fake_dec_start_ticks = val; } u64 GetFakeTBStartValue() { - return g_fakeTBStartValue; + return g_fake_TB_start_value; } void SetFakeTBStartValue(u64 val) { - g_fakeTBStartValue = val; + g_fake_TB_start_value = val; } u64 GetFakeTBStartTicks() { - return g_fakeTBStartTicks; + return g_fake_TB_start_ticks; } void SetFakeTBStartTicks(u64 val) { - g_fakeTBStartTicks = val; + g_fake_TB_start_ticks = val; } } // namespace diff --git a/Source/Core/Core/CoreTiming.h b/Source/Core/Core/CoreTiming.h index 210aa7c5e6..4fd76e4df7 100644 --- a/Source/Core/Core/CoreTiming.h +++ b/Source/Core/Core/CoreTiming.h @@ -25,11 +25,11 @@ class PointerWrap; namespace CoreTiming { // These really shouldn't be global, but jit64 accesses them directly -extern s64 g_globalTimer; -extern u64 g_fakeTBStartValue; -extern u64 g_fakeTBStartTicks; -extern int g_slicelength; -extern float g_lastOCFactor_inverted; +extern s64 g_global_timer; +extern u64 g_fake_TB_start_value; +extern u64 g_fake_TB_start_ticks; +extern int g_slice_length; +extern float g_last_OC_factor_inverted; void Init(); void Shutdown(); diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp index 24337e37d7..336371914e 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp @@ -198,7 +198,7 @@ void Interpreter::SingleStep() { SingleStepInner(); - CoreTiming::g_slicelength = 1; + CoreTiming::g_slice_length = 1; PowerPC::ppcState.downcount = 0; CoreTiming::Advance(); diff --git a/Source/Core/Core/PowerPC/Jit64/Jit_SystemRegisters.cpp b/Source/Core/Core/PowerPC/Jit64/Jit_SystemRegisters.cpp index 78f8e190e2..8a202251e7 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit_SystemRegisters.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit_SystemRegisters.cpp @@ -286,13 +286,13 @@ void Jit64::mfspr(UGeckoInstruction inst) // cost of calling out to C for this is actually significant. // Scale downcount by the CPU overclocking factor. CVTSI2SS(XMM0, PPCSTATE(downcount)); - MULSS(XMM0, M(&CoreTiming::g_lastOCFactor_inverted)); + MULSS(XMM0, M(&CoreTiming::g_last_OC_factor_inverted)); CVTSS2SI(RDX, R(XMM0)); // RDX is downcount scaled by the overclocking factor - MOV(32, R(RAX), M(&CoreTiming::g_slicelength)); + MOV(32, R(RAX), M(&CoreTiming::g_slice_length)); SUB(64, R(RAX), R(RDX)); // cycles since the last CoreTiming::Advance() event is (slicelength - // Scaled_downcount) - ADD(64, R(RAX), M(&CoreTiming::g_globalTimer)); - SUB(64, R(RAX), M(&CoreTiming::g_fakeTBStartTicks)); + ADD(64, R(RAX), M(&CoreTiming::g_global_timer)); + SUB(64, R(RAX), M(&CoreTiming::g_fake_TB_start_ticks)); // It might seem convenient to correct the timer for the block position here for even more // accurate // timing, but as of currently, this can break games. If we end up reading a time *after* the @@ -308,7 +308,7 @@ void Jit64::mfspr(UGeckoInstruction inst) // a / 12 = (a * 0xAAAAAAAAAAAAAAAB) >> 67 MOV(64, R(RDX), Imm64(0xAAAAAAAAAAAAAAABULL)); MUL(64, R(RDX)); - MOV(64, R(RAX), M(&CoreTiming::g_fakeTBStartValue)); + MOV(64, R(RAX), M(&CoreTiming::g_fake_TB_start_value)); SHR(64, R(RDX), Imm8(3)); ADD(64, R(RAX), R(RDX)); MOV(64, PPCSTATE(spr[SPR_TL]), R(RAX)); diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp index 22ecb11405..f5c457a00b 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp @@ -234,9 +234,9 @@ void JitArm64::mfspr(UGeckoInstruction inst) // An inline implementation of CoreTiming::GetFakeTimeBase, since in timer-heavy games the // cost of calling out to C for this is actually significant. - MOVI2R(XA, (u64)&CoreTiming::g_globalTimer); + MOVI2R(XA, (u64)&CoreTiming::g_global_timer); LDR(INDEX_UNSIGNED, XA, XA, 0); - MOVI2R(XB, (u64)&CoreTiming::g_fakeTBStartTicks); + MOVI2R(XB, (u64)&CoreTiming::g_fake_TB_start_ticks); LDR(INDEX_UNSIGNED, XB, XB, 0); SUB(XA, XA, XB); @@ -254,7 +254,7 @@ void JitArm64::mfspr(UGeckoInstruction inst) ADD(XB, XB, 1); UMULH(XA, XA, XB); - MOVI2R(XB, (u64)&CoreTiming::g_fakeTBStartValue); + MOVI2R(XB, (u64)&CoreTiming::g_fake_TB_start_value); LDR(INDEX_UNSIGNED, XB, XB, 0); ADD(XA, XB, XA, ArithOption(XA, ST_LSR, 3)); STR(INDEX_UNSIGNED, XA, PPC_REG, PPCSTATE_OFF(spr[SPR_TL])); From 17c34ae0b1d02614133ebc0e3c7d12ff1ed4d51b Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Thu, 1 Sep 2016 06:42:52 +0000 Subject: [PATCH 2/9] CoreTiming: Data structure cleanup Replace adhoc linked list with a priority heap. Performance characteristics are mostly the same, but is more cache friendly. [Priority Queues have O(log n) push/pop compared to the linked list's O(n) push/O(1) pop but the queue is not big enough for that to matter, so linear is faster over linked. Very slight gains when framelimit is unlimited (Wind Waker), 1900% -> 1950%] --- Source/Core/Common/ChunkFile.h | 85 ++--------- Source/Core/Core/CoreTiming.cpp | 256 ++++++++++++-------------------- Source/Core/Core/State.cpp | 2 +- 3 files changed, 111 insertions(+), 232 deletions(-) diff --git a/Source/Core/Common/ChunkFile.h b/Source/Core/Common/ChunkFile.h index 4253bbf968..64e2a10b50 100644 --- a/Source/Core/Common/ChunkFile.h +++ b/Source/Core/Common/ChunkFile.h @@ -51,12 +51,6 @@ #error No version of is_trivially_copyable #endif -template -struct LinkedListItem : public T -{ - LinkedListItem* next; -}; - // Wrapper class class PointerWrap { @@ -244,67 +238,6 @@ public: } } - // Let's pretend std::list doesn't exist! - template * (*TNew)(), void (*TFree)(LinkedListItem*), - void (*TDo)(PointerWrap&, T*)> - void DoLinkedList(LinkedListItem*& list_start, LinkedListItem** list_end = 0) - { - LinkedListItem* list_cur = list_start; - LinkedListItem* prev = nullptr; - - while (true) - { - u8 shouldExist = !!list_cur; - Do(shouldExist); - if (shouldExist == 1) - { - LinkedListItem* cur = list_cur ? list_cur : TNew(); - TDo(*this, (T*)cur); - if (!list_cur) - { - if (mode == MODE_READ) - { - cur->next = nullptr; - list_cur = cur; - if (prev) - prev->next = cur; - else - list_start = cur; - } - else - { - TFree(cur); - continue; - } - } - } - else - { - if (mode == MODE_READ) - { - if (prev) - prev->next = nullptr; - if (list_end) - *list_end = prev; - if (list_cur) - { - if (list_start == list_cur) - list_start = nullptr; - do - { - LinkedListItem* next = list_cur->next; - TFree(list_cur); - list_cur = next; - } while (list_cur); - } - } - break; - } - prev = list_cur; - list_cur = list_cur->next; - } - } - void DoMarker(const std::string& prevName, u32 arbitraryNumber = 0x42) { u32 cookie = arbitraryNumber; @@ -319,16 +252,22 @@ public: } } + template + void DoEachElement(T& container, Functor member) + { + u32 size = static_cast(container.size()); + Do(size); + container.resize(size); + + for (auto& elem : container) + member(*this, elem); + } + private: template void DoContainer(T& x) { - u32 size = (u32)x.size(); - Do(size); - x.resize(size); - - for (auto& elem : x) - Do(elem); + DoEachElement(x, [](PointerWrap& p, typename T::value_type& elem) { p.Do(elem); }); } __forceinline void DoVoid(void* data, u32 size) diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp index 1f1b711c3a..ad8cb71139 100644 --- a/Source/Core/Core/CoreTiming.cpp +++ b/Source/Core/Core/CoreTiming.cpp @@ -8,8 +8,10 @@ #include #include +#include "Common/Assert.h" #include "Common/ChunkFile.h" #include "Common/FifoQueue.h" +#include "Common/Logging/Log.h" #include "Common/StringUtil.h" #include "Common/Thread.h" @@ -31,22 +33,30 @@ struct EventType static std::vector s_event_types; -struct BaseEvent +struct Event { s64 time; u64 userdata; int type; }; -typedef LinkedListItem Event; +constexpr bool operator>(const Event& left, const Event& right) +{ + return left.time > right.time; +} +constexpr bool operator<(const Event& left, const Event& right) +{ + return left.time < right.time; +} // STATE_TO_SAVE -static Event* s_first_event; +// The queue is a min-heap using std::make_heap/push_heap/pop_heap. +// We don't use std::priority_queue because we need to be able to serialize, unserialize and +// erase arbitrary events (RemoveEvent()) regardless of the queue order. These aren't accomodated +// by the standard adaptor class. +static std::vector s_event_queue; static std::mutex s_ts_write_lock; -static Common::FifoQueue s_ts_queue; - -// event pools -static Event* s_event_pool = nullptr; +static Common::FifoQueue s_ts_queue; static float s_last_OC_factor; float g_last_OC_factor_inverted; @@ -66,22 +76,6 @@ u64 g_fake_TB_start_ticks; static int s_ev_lost; -static Event* GetNewEvent() -{ - if (!s_event_pool) - return new Event; - - Event* ev = s_event_pool; - s_event_pool = ev->next; - return ev; -} - -static void FreeEvent(Event* ev) -{ - ev->next = s_event_pool; - s_event_pool = ev; -} - static void EmptyTimedCallback(u64 userdata, s64 cyclesLate) { } @@ -132,9 +126,9 @@ int RegisterEvent(const std::string& name, TimedCallback callback) void UnregisterAllEvents() { - if (s_first_event) - PanicAlert("Cannot unregister events with events pending"); + _assert_msg_(POWERPC, s_event_queue.empty(), "Cannot unregister events with events pending"); s_event_types.clear(); + s_event_types.shrink_to_fit(); } void Init() @@ -156,50 +150,6 @@ void Shutdown() MoveEvents(); ClearPendingEvents(); UnregisterAllEvents(); - - while (s_event_pool) - { - Event* ev = s_event_pool; - s_event_pool = ev->next; - delete ev; - } -} - -static void EventDoState(PointerWrap& p, BaseEvent* ev) -{ - p.Do(ev->time); - - // this is why we can't have (nice things) pointers as userdata - p.Do(ev->userdata); - - // we can't savestate ev->type directly because events might not get registered in the same order - // (or at all) every time. - // so, we savestate the event's type's name, and derive ev->type from that when loading. - std::string name; - if (p.GetMode() != PointerWrap::MODE_READ) - name = s_event_types[ev->type].name; - - p.Do(name); - if (p.GetMode() == PointerWrap::MODE_READ) - { - bool foundMatch = false; - for (unsigned int i = 0; i < s_event_types.size(); ++i) - { - if (name == s_event_types[i].name) - { - ev->type = i; - foundMatch = true; - break; - } - } - if (!foundMatch) - { - WARN_LOG(POWERPC, - "Lost event from savestate because its type, \"%s\", has not been registered.", - name.c_str()); - ev->type = s_ev_lost; - } - } } void DoState(PointerWrap& p) @@ -218,9 +168,44 @@ void DoState(PointerWrap& p) p.DoMarker("CoreTimingData"); MoveEvents(); + p.DoEachElement(s_event_queue, [](PointerWrap& pw, Event& ev) { + pw.Do(ev.time); - p.DoLinkedList(s_first_event); + // this is why we can't have (nice things) pointers as userdata + pw.Do(ev.userdata); + + // we can't savestate ev.type directly because events might not get registered in the same + // order (or at all) every time. + // so, we savestate the event's type's name, and derive ev.type from that when loading. + std::string name; + if (pw.GetMode() != PointerWrap::MODE_READ) + name = s_event_types[ev.type].name; + + pw.Do(name); + if (pw.GetMode() == PointerWrap::MODE_READ) + { + auto itr = std::find_if(s_event_types.begin(), s_event_types.end(), + [&](const EventType& evt) { return evt.name == name; }); + if (itr != s_event_types.end()) + { + ev.type = static_cast(itr - s_event_types.begin()); + } + else + { + WARN_LOG(POWERPC, + "Lost event from savestate because its type, \"%s\", has not been registered.", + name.c_str()); + ev.type = s_ev_lost; + } + } + }); p.DoMarker("CoreTimingEvents"); + + // When loading from a save state, we must assume the Event order is random and meaningless. + // The exact layout of the heap in memory is implementation defined, therefore it is platform + // and library version specific. + if (p.GetMode() == PointerWrap::MODE_READ) + std::make_heap(s_event_queue.begin(), s_event_queue.end(), std::greater()); } // This should only be called from the CPU thread. If you are calling @@ -243,30 +228,7 @@ u64 GetIdleTicks() void ClearPendingEvents() { - while (s_first_event) - { - Event* e = s_first_event->next; - FreeEvent(s_first_event); - s_first_event = e; - } -} - -static void AddEventToQueue(Event* ne) -{ - Event* prev = nullptr; - Event** pNext = &s_first_event; - for (;;) - { - Event*& next = *pNext; - if (!next || ne->time < next->time) - { - ne->next = next; - next = ne; - break; - } - prev = next; - pNext = &prev->next; - } + s_event_queue.clear(); } void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThread from) @@ -285,16 +247,14 @@ void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThr if (from_cpu_thread) { - Event* ne = GetNewEvent(); - ne->time = GetTicks() + cycles_into_future; - ne->userdata = userdata; - ne->type = event_type; + s64 timeout = GetTicks() + cycles_into_future; // If this event needs to be scheduled before the next advance(), force one early if (!s_is_global_timer_sane) ForceExceptionCheck(cycles_into_future); - AddEventToQueue(ne); + s_event_queue.emplace_back(Event{timeout, userdata, event_type}); + std::push_heap(s_event_queue.begin(), s_event_queue.end(), std::greater()); } else { @@ -306,41 +266,20 @@ void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThr } std::lock_guard lk(s_ts_write_lock); - Event ne; - ne.time = g_global_timer + cycles_into_future; - ne.type = event_type; - ne.userdata = userdata; - s_ts_queue.Push(ne); + s_ts_queue.Push(Event{g_global_timer + cycles_into_future, userdata, event_type}); } } void RemoveEvent(int event_type) { - while (s_first_event && s_first_event->type == event_type) - { - Event* next = s_first_event->next; - FreeEvent(s_first_event); - s_first_event = next; - } + auto itr = std::remove_if(s_event_queue.begin(), s_event_queue.end(), + [&](const Event& e) { return e.type == event_type; }); - if (!s_first_event) - return; - - Event* prev = s_first_event; - Event* ptr = prev->next; - while (ptr) + // Removing random items breaks the invariant so we have to re-establish it. + if (itr != s_event_queue.end()) { - if (ptr->type == event_type) - { - prev->next = ptr->next; - FreeEvent(ptr); - ptr = prev->next; - } - else - { - prev = ptr; - ptr = ptr->next; - } + s_event_queue.erase(itr, s_event_queue.end()); + std::make_heap(s_event_queue.begin(), s_event_queue.end(), std::greater()); } } @@ -363,14 +302,10 @@ void ForceExceptionCheck(s64 cycles) void MoveEvents() { - BaseEvent sevt; - while (s_ts_queue.Pop(sevt)) + for (Event ev; s_ts_queue.Pop(ev);) { - Event* evt = GetNewEvent(); - evt->time = sevt.time; - evt->userdata = sevt.userdata; - evt->type = sevt.type; - AddEventToQueue(evt); + s_event_queue.emplace_back(std::move(ev)); + std::push_heap(s_event_queue.begin(), s_event_queue.end(), std::greater()); } } @@ -386,24 +321,23 @@ void Advance() s_is_global_timer_sane = true; - while (s_first_event && s_first_event->time <= g_global_timer) + while (!s_event_queue.empty() && s_event_queue.front().time <= g_global_timer) { - // LOG(POWERPC, "[Scheduler] %s (%lld, %lld) ", - // s_event_types[s_first_event->type].name ? s_event_types[s_first_event->type].name - // : "?", - // (u64)g_global_timer, (u64)s_first_event->time); - Event* evt = s_first_event; - s_first_event = s_first_event->next; - s_event_types[evt->type].callback(evt->userdata, g_global_timer - evt->time); - FreeEvent(evt); + Event evt = std::move(s_event_queue.front()); + std::pop_heap(s_event_queue.begin(), s_event_queue.end(), std::greater()); + s_event_queue.pop_back(); + // NOTICE_LOG(POWERPC, "[Scheduler] %-20s (%lld, %lld)", evt.type->name->c_str(), + // g_global_timer, evt.time); + s_event_types[evt.type].callback(evt.userdata, g_global_timer - evt.time); } s_is_global_timer_sane = false; - if (s_first_event) + // Still events left (scheduled in the future) + if (!s_event_queue.empty()) { - g_slice_length = - static_cast(std::min(s_first_event->time - g_global_timer, MAX_SLICE_LENGTH)); + g_slice_length = static_cast( + std::min(s_event_queue.front().time - g_global_timer, MAX_SLICE_LENGTH)); } PowerPC::ppcState.downcount = CyclesToDowncount(g_slice_length); @@ -417,12 +351,14 @@ void Advance() void LogPendingEvents() { - Event* ptr = s_first_event; - while (ptr) + auto clone = s_event_queue; + std::sort(clone.begin(), clone.end()); + for (const Event& ev : clone) { - INFO_LOG(POWERPC, "PENDING: Now: %" PRId64 " Pending: %" PRId64 " Type: %d", g_global_timer, - ptr->time, ptr->type); - ptr = ptr->next; + INFO_LOG(POWERPC, "PENDING: Now: %" PRId64 " Pending: %" PRId64 " Type: %s (%d)", + g_global_timer, ev.time, + ev.type < s_event_types.size() ? s_event_types[ev.type].name.c_str() : "", + ev.type); } } @@ -442,20 +378,24 @@ void Idle() std::string GetScheduledEventsSummary() { - Event* ptr = s_first_event; std::string text = "Scheduled events\n"; text.reserve(1000); - while (ptr) + + auto clone = s_event_queue; + std::sort(clone.begin(), clone.end()); + for (const Event& ev : clone) { - unsigned int t = ptr->type; + unsigned int t = ev.type; if (t >= s_event_types.size()) + { PanicAlertT("Invalid event type %i", t); + continue; + } - const std::string& name = s_event_types[ptr->type].name; + const std::string& name = s_event_types[ev.type].name; - text += StringFromFormat("%s : %" PRIi64 " %016" PRIx64 "\n", name.c_str(), ptr->time, - ptr->userdata); - ptr = ptr->next; + text += + StringFromFormat("%s : %" PRIi64 " %016" PRIx64 "\n", name.c_str(), ev.time, ev.userdata); } return text; } diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index feee9730f5..117e16b935 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -70,7 +70,7 @@ static Common::Event g_compressAndDumpStateSyncEvent; static std::thread g_save_thread; // Don't forget to increase this after doing changes on the savestate system -static const u32 STATE_VERSION = 55; +static const u32 STATE_VERSION = 56; // Maps savestate versions to Dolphin versions. // Versions after 42 don't need to be added to this list, From f15e4fb35e5f2b7ed01d2f23b26719b7230c5b90 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Sat, 3 Sep 2016 04:54:34 +0000 Subject: [PATCH 3/9] WII_IPC: Fix reregistering CoreTiming callback multiple times. Separate state reset from Init(). --- Source/Core/Core/HW/WII_IPC.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index 646abb778d..ff24dd3447 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -113,7 +113,7 @@ void DoState(PointerWrap& p) p.Do(sensorbar_power); } -void Init() +static void InitState() { ctrl = CtrlRegister(); ppc_msg = 0; @@ -127,14 +127,18 @@ void Init() sensorbar_power = 0; ppc_irq_masks |= INT_CAUSE_IPC_BROADWAY; +} +void Init() +{ + InitState(); updateInterrupts = CoreTiming::RegisterEvent("IPCInterrupt", UpdateInterrupts); } void Reset() { INFO_LOG(WII_IPC, "Resetting ..."); - Init(); + InitState(); WII_IPC_HLE_Interface::Reset(); } From aa1628251637e2040a85146c7c8578832b8489fb Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Thu, 1 Sep 2016 10:54:18 +0000 Subject: [PATCH 4/9] Core: Change CoreTiming event key from int to EventType* Replace 'int' keys with something that carries type information. Performance is neutral. --- Source/Core/Core/CoreTiming.cpp | 82 ++++++++------------ Source/Core/Core/CoreTiming.h | 10 ++- Source/Core/Core/HW/AudioInterface.cpp | 2 +- Source/Core/Core/HW/DSP.cpp | 4 +- Source/Core/Core/HW/DVDInterface.cpp | 8 +- Source/Core/Core/HW/DVDThread.cpp | 2 +- Source/Core/Core/HW/EXI.cpp | 4 +- Source/Core/Core/HW/EXI_DeviceMemoryCard.h | 7 +- Source/Core/Core/HW/ProcessorInterface.cpp | 4 +- Source/Core/Core/HW/SI.cpp | 4 +- Source/Core/Core/HW/SystemTimers.cpp | 15 ++-- Source/Core/Core/HW/WII_IPC.cpp | 2 +- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 4 +- Source/Core/Core/MemoryWatcher.cpp | 2 +- Source/Core/Core/PowerPC/PowerPC.cpp | 2 +- Source/Core/VideoCommon/CommandProcessor.cpp | 2 +- Source/Core/VideoCommon/Fifo.cpp | 2 +- Source/Core/VideoCommon/PixelEngine.cpp | 2 +- 18 files changed, 73 insertions(+), 85 deletions(-) diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp index ad8cb71139..329f63a7ae 100644 --- a/Source/Core/Core/CoreTiming.cpp +++ b/Source/Core/Core/CoreTiming.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include "Common/Assert.h" @@ -28,16 +29,14 @@ namespace CoreTiming struct EventType { TimedCallback callback; - std::string name; + const std::string* name; }; -static std::vector s_event_types; - struct Event { s64 time; u64 userdata; - int type; + EventType* type; }; constexpr bool operator>(const Event& left, const Event& right) @@ -49,6 +48,10 @@ constexpr bool operator<(const Event& left, const Event& right) return left.time < right.time; } +// unordered_map stores each element separately as a linked list node so pointers to elements +// remain stable regardless of rehashes/resizing. +static std::unordered_map s_event_types; + // STATE_TO_SAVE // The queue is a min-heap using std::make_heap/push_heap/pop_heap. // We don't use std::priority_queue because we need to be able to serialize, unserialize and @@ -74,7 +77,7 @@ s64 g_global_timer; u64 g_fake_TB_start_value; u64 g_fake_TB_start_ticks; -static int s_ev_lost; +static EventType* s_ev_lost = nullptr; static void EmptyTimedCallback(u64 userdata, s64 cyclesLate) { @@ -97,38 +100,25 @@ static int CyclesToDowncount(int cycles) return static_cast(cycles * s_last_OC_factor); } -int RegisterEvent(const std::string& name, TimedCallback callback) +EventType* RegisterEvent(const std::string& name, TimedCallback callback) { - EventType type; - type.name = name; - type.callback = callback; - // check for existing type with same name. // we want event type names to remain unique so that we can use them for serialization. - for (auto& event_type : s_event_types) - { - if (name == event_type.name) - { - WARN_LOG( - POWERPC, - "Discarded old event type \"%s\" because a new type with the same name was registered.", - name.c_str()); - // we don't know if someone might be holding on to the type index, - // so we gut the old event type instead of actually removing it. - event_type.name = "_discarded_event"; - event_type.callback = &EmptyTimedCallback; - } - } + _assert_msg_(POWERPC, s_event_types.find(name) == s_event_types.end(), + "CoreTiming Event \"%s\" is already registered. Events should only be registered " + "during Init to avoid breaking save states.", + name.c_str()); - s_event_types.push_back(type); - return static_cast(s_event_types.size() - 1); + auto info = s_event_types.emplace(name, EventType{callback, nullptr}); + EventType* event_type = &info.first->second; + event_type->name = &info.first->first; + return event_type; } void UnregisterAllEvents() { _assert_msg_(POWERPC, s_event_queue.empty(), "Cannot unregister events with events pending"); s_event_types.clear(); - s_event_types.shrink_to_fit(); } void Init() @@ -179,16 +169,15 @@ void DoState(PointerWrap& p) // so, we savestate the event's type's name, and derive ev.type from that when loading. std::string name; if (pw.GetMode() != PointerWrap::MODE_READ) - name = s_event_types[ev.type].name; + name = *ev.type->name; pw.Do(name); if (pw.GetMode() == PointerWrap::MODE_READ) { - auto itr = std::find_if(s_event_types.begin(), s_event_types.end(), - [&](const EventType& evt) { return evt.name == name; }); + auto itr = s_event_types.find(name); if (itr != s_event_types.end()) { - ev.type = static_cast(itr - s_event_types.begin()); + ev.type = &itr->second; } else { @@ -231,8 +220,10 @@ void ClearPendingEvents() s_event_queue.clear(); } -void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThread from) +void ScheduleEvent(s64 cycles_into_future, EventType* event_type, u64 userdata, FromThread from) { + _assert_msg_(POWERPC, event_type, "Event type is nullptr, will crash now."); + bool from_cpu_thread; if (from == FromThread::ANY) { @@ -262,7 +253,7 @@ void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThr { ERROR_LOG(POWERPC, "Someone scheduled an off-thread \"%s\" event while netplay or " "movie play/record was active. This is likely to cause a desync.", - s_event_types[event_type].name.c_str()); + event_type->name->c_str()); } std::lock_guard lk(s_ts_write_lock); @@ -270,7 +261,7 @@ void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThr } } -void RemoveEvent(int event_type) +void RemoveEvent(EventType* event_type) { auto itr = std::remove_if(s_event_queue.begin(), s_event_queue.end(), [&](const Event& e) { return e.type == event_type; }); @@ -283,7 +274,7 @@ void RemoveEvent(int event_type) } } -void RemoveAllEvents(int event_type) +void RemoveAllEvents(EventType* event_type) { MoveEvents(); RemoveEvent(event_type); @@ -328,7 +319,7 @@ void Advance() s_event_queue.pop_back(); // NOTICE_LOG(POWERPC, "[Scheduler] %-20s (%lld, %lld)", evt.type->name->c_str(), // g_global_timer, evt.time); - s_event_types[evt.type].callback(evt.userdata, g_global_timer - evt.time); + evt.type->callback(evt.userdata, g_global_timer - evt.time); } s_is_global_timer_sane = false; @@ -355,10 +346,8 @@ void LogPendingEvents() std::sort(clone.begin(), clone.end()); for (const Event& ev : clone) { - INFO_LOG(POWERPC, "PENDING: Now: %" PRId64 " Pending: %" PRId64 " Type: %s (%d)", - g_global_timer, ev.time, - ev.type < s_event_types.size() ? s_event_types[ev.type].name.c_str() : "", - ev.type); + INFO_LOG(POWERPC, "PENDING: Now: %" PRId64 " Pending: %" PRId64 " Type: %s", g_global_timer, + ev.time, ev.type->name->c_str()); } } @@ -385,17 +374,8 @@ std::string GetScheduledEventsSummary() std::sort(clone.begin(), clone.end()); for (const Event& ev : clone) { - unsigned int t = ev.type; - if (t >= s_event_types.size()) - { - PanicAlertT("Invalid event type %i", t); - continue; - } - - const std::string& name = s_event_types[ev.type].name; - - text += - StringFromFormat("%s : %" PRIi64 " %016" PRIx64 "\n", name.c_str(), ev.time, ev.userdata); + text += StringFromFormat("%s : %" PRIi64 " %016" PRIx64 "\n", ev.type->name->c_str(), ev.time, + ev.userdata); } return text; } diff --git a/Source/Core/Core/CoreTiming.h b/Source/Core/Core/CoreTiming.h index 4fd76e4df7..4f209c97bd 100644 --- a/Source/Core/Core/CoreTiming.h +++ b/Source/Core/Core/CoreTiming.h @@ -43,9 +43,11 @@ u64 GetIdleTicks(); void DoState(PointerWrap& p); +struct EventType; + // Returns the event_type identifier. if name is not unique, an existing event_type will be // discarded. -int RegisterEvent(const std::string& name, TimedCallback callback); +EventType* RegisterEvent(const std::string& name, TimedCallback callback); void UnregisterAllEvents(); enum class FromThread @@ -58,12 +60,12 @@ enum class FromThread }; // userdata MAY NOT CONTAIN POINTERS. userdata might get written and reloaded from savestates. -void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata = 0, +void ScheduleEvent(s64 cycles_into_future, EventType* event_type, u64 userdata = 0, FromThread from = FromThread::CPU); // We only permit one event of each type in the queue at a time. -void RemoveEvent(int event_type); -void RemoveAllEvents(int event_type); +void RemoveEvent(EventType* event_type); +void RemoveAllEvents(EventType* event_type); void Advance(); void MoveEvents(); diff --git a/Source/Core/Core/HW/AudioInterface.cpp b/Source/Core/Core/HW/AudioInterface.cpp index de062df4aa..92e0e1610b 100644 --- a/Source/Core/Core/HW/AudioInterface.cpp +++ b/Source/Core/Core/HW/AudioInterface.cpp @@ -130,7 +130,7 @@ static void GenerateAudioInterrupt(); static void UpdateInterrupts(); static void IncreaseSampleCount(const u32 _uAmount); static int GetAIPeriod(); -static int et_AI; +static CoreTiming::EventType* et_AI; static void Update(u64 userdata, s64 cyclesLate); void Init() diff --git a/Source/Core/Core/HW/DSP.cpp b/Source/Core/Core/HW/DSP.cpp index 9ad397901c..9fab707330 100644 --- a/Source/Core/Core/HW/DSP.cpp +++ b/Source/Core/Core/HW/DSP.cpp @@ -188,8 +188,8 @@ static void UpdateInterrupts(); static void Do_ARAM_DMA(); static void GenerateDSPInterrupt(u64 DSPIntType, s64 cyclesLate = 0); -static int et_GenerateDSPInterrupt; -static int et_CompleteARAM; +static CoreTiming::EventType* et_GenerateDSPInterrupt; +static CoreTiming::EventType* et_CompleteARAM; static void CompleteARAM(u64 userdata, s64 cyclesLate) { diff --git a/Source/Core/Core/HW/DVDInterface.cpp b/Source/Core/Core/HW/DVDInterface.cpp index 1fdef1a313..ad3236d1b0 100644 --- a/Source/Core/Core/HW/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVDInterface.cpp @@ -238,14 +238,14 @@ static u32 s_error_code = 0; static bool s_disc_inside = false; static bool s_stream = false; static bool s_stop_at_track_end = false; -static int s_finish_executing_command = 0; -static int s_dtk = 0; +static CoreTiming::EventType* s_finish_executing_command; +static CoreTiming::EventType* s_dtk; static u64 s_last_read_offset; static u64 s_last_read_time; -static int s_eject_disc; -static int s_insert_disc; +static CoreTiming::EventType* s_eject_disc; +static CoreTiming::EventType* s_insert_disc; static void EjectDiscCallback(u64 userdata, s64 cyclesLate); static void InsertDiscCallback(u64 userdata, s64 cyclesLate); diff --git a/Source/Core/Core/HW/DVDThread.cpp b/Source/Core/Core/HW/DVDThread.cpp index 3601f5bafd..e79de14019 100644 --- a/Source/Core/Core/HW/DVDThread.cpp +++ b/Source/Core/Core/HW/DVDThread.cpp @@ -30,7 +30,7 @@ namespace DVDThread static void DVDThread(); static void FinishRead(u64 userdata, s64 cycles_late); -static int s_finish_read; +static CoreTiming::EventType* s_finish_read; static std::thread s_dvd_thread; static Common::Event s_dvd_thread_start_working; diff --git a/Source/Core/Core/HW/EXI.cpp b/Source/Core/Core/HW/EXI.cpp index 9db0a83382..32c6ce80a7 100644 --- a/Source/Core/Core/HW/EXI.cpp +++ b/Source/Core/Core/HW/EXI.cpp @@ -23,8 +23,8 @@ bool g_SRAM_netplay_initialized = false; namespace ExpansionInterface { -static int changeDevice; -static int updateInterrupts; +static CoreTiming::EventType* changeDevice; +static CoreTiming::EventType* updateInterrupts; static std::array, MAX_EXI_CHANNELS> g_Channels; diff --git a/Source/Core/Core/HW/EXI_DeviceMemoryCard.h b/Source/Core/Core/HW/EXI_DeviceMemoryCard.h index 53763e7e5f..5cafcfce29 100644 --- a/Source/Core/Core/HW/EXI_DeviceMemoryCard.h +++ b/Source/Core/Core/HW/EXI_DeviceMemoryCard.h @@ -9,6 +9,10 @@ #include "Core/HW/EXI_Device.h" +namespace CoreTiming +{ +struct EventType; +} class MemoryCardBase; class PointerWrap; @@ -67,7 +71,8 @@ private: }; int card_index; - int et_cmd_done, et_transfer_complete; + CoreTiming::EventType* et_cmd_done = nullptr; + CoreTiming::EventType* et_transfer_complete = nullptr; //! memory card state // STATE_TO_SAVE diff --git a/Source/Core/Core/HW/ProcessorInterface.cpp b/Source/Core/Core/HW/ProcessorInterface.cpp index 59ce6a4569..04fec4cd07 100644 --- a/Source/Core/Core/HW/ProcessorInterface.cpp +++ b/Source/Core/Core/HW/ProcessorInterface.cpp @@ -31,10 +31,10 @@ static u32 m_FlipperRev; static u32 m_Unknown; // ID and callback for scheduling reset button presses/releases -static int toggleResetButton; +static CoreTiming::EventType* toggleResetButton; static void ToggleResetButtonCallback(u64 userdata, s64 cyclesLate); -static int iosNotifyResetButton; +static CoreTiming::EventType* iosNotifyResetButton; static void IOSNotifyResetButtonCallback(u64 userdata, s64 cyclesLate); // Let the PPC know that an external exception is set/cleared diff --git a/Source/Core/Core/HW/SI.cpp b/Source/Core/Core/HW/SI.cpp index 55c9c06904..917f70d7e4 100644 --- a/Source/Core/Core/HW/SI.cpp +++ b/Source/Core/Core/HW/SI.cpp @@ -23,8 +23,8 @@ namespace SerialInterface { -static int changeDevice; -static int et_transfer_pending; +static CoreTiming::EventType* changeDevice; +static CoreTiming::EventType* et_transfer_pending; static void RunSIBuffer(u64 userdata, s64 cyclesLate); static void UpdateInterrupts(); diff --git a/Source/Core/Core/HW/SystemTimers.cpp b/Source/Core/Core/HW/SystemTimers.cpp index 6736d191b7..3e8b96381f 100644 --- a/Source/Core/Core/HW/SystemTimers.cpp +++ b/Source/Core/Core/HW/SystemTimers.cpp @@ -64,13 +64,14 @@ IPC_HLE_PERIOD: For the Wiimote this is the call schedule: namespace SystemTimers { -static int et_Dec; -static int et_VI; -static int et_AudioDMA; -static int et_DSP; -static int et_IPC_HLE; -static int et_PatchEngine; // PatchEngine updates every 1/60th of a second by default -static int et_Throttle; +static CoreTiming::EventType* et_Dec; +static CoreTiming::EventType* et_VI; +static CoreTiming::EventType* et_AudioDMA; +static CoreTiming::EventType* et_DSP; +static CoreTiming::EventType* et_IPC_HLE; +// PatchEngine updates every 1/60th of a second by default +static CoreTiming::EventType* et_PatchEngine; +static CoreTiming::EventType* et_Throttle; static u32 s_cpu_core_clock = 486000000u; // 486 mhz (its not 485, stop bugging me!) diff --git a/Source/Core/Core/HW/WII_IPC.cpp b/Source/Core/Core/HW/WII_IPC.cpp index ff24dd3447..67da12be96 100644 --- a/Source/Core/Core/HW/WII_IPC.cpp +++ b/Source/Core/Core/HW/WII_IPC.cpp @@ -98,7 +98,7 @@ static u32 arm_irq_masks; static u32 sensorbar_power; // do we need to care about this? -static int updateInterrupts; +static CoreTiming::EventType* updateInterrupts; static void UpdateInterrupts(u64 = 0, s64 cyclesLate = 0); void DoState(PointerWrap& p) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index 1720790b75..0934c79f59 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -76,8 +76,8 @@ static ipc_msg_queue request_queue; // ppc -> arm static ipc_msg_queue reply_queue; // arm -> ppc static ipc_msg_queue ack_queue; // arm -> ppc -static int event_enqueue; -static int event_sdio_notify; +static CoreTiming::EventType* event_enqueue; +static CoreTiming::EventType* event_sdio_notify; static u64 last_reply_time; diff --git a/Source/Core/Core/MemoryWatcher.cpp b/Source/Core/Core/MemoryWatcher.cpp index a0fccb973b..3aea80a6b6 100644 --- a/Source/Core/Core/MemoryWatcher.cpp +++ b/Source/Core/Core/MemoryWatcher.cpp @@ -15,7 +15,7 @@ #include "Core/MemoryWatcher.h" static std::unique_ptr s_memory_watcher; -static int s_event; +static CoreTiming::EventType* s_event; static const int MW_RATE = 600; // Steps per second static void MWCallback(u64 userdata, s64 cyclesLate) diff --git a/Source/Core/Core/PowerPC/PowerPC.cpp b/Source/Core/Core/PowerPC/PowerPC.cpp index 68b21ab0e0..e7d9e8617d 100644 --- a/Source/Core/Core/PowerPC/PowerPC.cpp +++ b/Source/Core/Core/PowerPC/PowerPC.cpp @@ -36,7 +36,7 @@ BreakPoints breakpoints; MemChecks memchecks; PPCDebugInterface debug_interface; -static int s_invalidate_cache_thread_safe; +static CoreTiming::EventType* s_invalidate_cache_thread_safe; static void InvalidateCacheThreadSafe(u64 userdata, s64 cyclesLate) { ppcState.iCache.Invalidate(static_cast(userdata)); diff --git a/Source/Core/VideoCommon/CommandProcessor.cpp b/Source/Core/VideoCommon/CommandProcessor.cpp index 287d93d28f..32095d57b2 100644 --- a/Source/Core/VideoCommon/CommandProcessor.cpp +++ b/Source/Core/VideoCommon/CommandProcessor.cpp @@ -21,7 +21,7 @@ namespace CommandProcessor { -static int et_UpdateInterrupts; +static CoreTiming::EventType* et_UpdateInterrupts; // TODO(ector): Warn on bbox read/write diff --git a/Source/Core/VideoCommon/Fifo.cpp b/Source/Core/VideoCommon/Fifo.cpp index 263d21193c..97cb8753e0 100644 --- a/Source/Core/VideoCommon/Fifo.cpp +++ b/Source/Core/VideoCommon/Fifo.cpp @@ -48,7 +48,7 @@ static u8* s_fifo_aux_read_ptr; static bool s_use_deterministic_gpu_thread; static u64 s_last_sync_gpu_tick; -static int s_event_sync_gpu; +static CoreTiming::EventType* s_event_sync_gpu; // STATE_TO_SAVE static u8* s_video_buffer; diff --git a/Source/Core/VideoCommon/PixelEngine.cpp b/Source/Core/VideoCommon/PixelEngine.cpp index f38eb71125..5271f11d9a 100644 --- a/Source/Core/VideoCommon/PixelEngine.cpp +++ b/Source/Core/VideoCommon/PixelEngine.cpp @@ -100,7 +100,7 @@ static bool s_event_raised; static bool s_signal_token_interrupt; static bool s_signal_finish_interrupt; -static int et_SetTokenFinishOnMainThread; +static CoreTiming::EventType* et_SetTokenFinishOnMainThread; enum { From ac63e54473db5cc3fc3f3e2aedaaac530b9177d5 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Fri, 2 Sep 2016 10:11:08 +0000 Subject: [PATCH 5/9] [UnitTests] Add CoreTimingTest --- Source/UnitTests/Core/CMakeLists.txt | 1 + Source/UnitTests/Core/CoreTimingTest.cpp | 313 +++++++++++++++++++++++ 2 files changed, 314 insertions(+) create mode 100644 Source/UnitTests/Core/CoreTimingTest.cpp diff --git a/Source/UnitTests/Core/CMakeLists.txt b/Source/UnitTests/Core/CMakeLists.txt index 5bb809d9df..604318361a 100644 --- a/Source/UnitTests/Core/CMakeLists.txt +++ b/Source/UnitTests/Core/CMakeLists.txt @@ -1,2 +1,3 @@ add_dolphin_test(MMIOTest MMIOTest.cpp) add_dolphin_test(PageFaultTest PageFaultTest.cpp) +add_dolphin_test(CoreTimingTest CoreTimingTest.cpp) diff --git a/Source/UnitTests/Core/CoreTimingTest.cpp b/Source/UnitTests/Core/CoreTimingTest.cpp new file mode 100644 index 0000000000..305232e8a3 --- /dev/null +++ b/Source/UnitTests/Core/CoreTimingTest.cpp @@ -0,0 +1,313 @@ +// Copyright 2016 Dolphin Emulator Project +// Licensed under GPLv2+ +// Refer to the license.txt file included. + +#include + +#include +#include + +#include "Core/ConfigManager.h" +#include "Core/Core.h" +#include "Core/CoreTiming.h" +#include "Core/PowerPC/PowerPC.h" + +// Numbers are chosen randomly to make sure the correct one is given. +static constexpr std::array CB_IDS{{42, 144, 93, 1026, UINT64_C(0xFFFF7FFFF7FFFF)}}; +static constexpr int MAX_SLICE_LENGTH = 20000; // Copied from CoreTiming internals + +static std::bitset s_callbacks_ran_flags; +static u64 s_expected_callback = 0; +static s64 s_lateness = 0; + +template +void CallbackTemplate(u64 userdata, s64 lateness) +{ + static_assert(IDX < CB_IDS.size(), "IDX out of range"); + s_callbacks_ran_flags.set(IDX); + EXPECT_EQ(CB_IDS[IDX], userdata); + if (s_expected_callback) // In SharedSlot, we don't care about this + EXPECT_EQ(CB_IDS[IDX], s_expected_callback); + EXPECT_EQ(s_lateness, lateness); +} + +class ScopeInit final +{ +public: + ScopeInit() + { + Core::DeclareAsCPUThread(); + SConfig::Init(); + PowerPC::Init(PowerPC::CORE_INTERPRETER); + CoreTiming::Init(); + } + ~ScopeInit() + { + CoreTiming::Shutdown(); + PowerPC::Shutdown(); + SConfig::Shutdown(); + Core::UndeclareAsCPUThread(); + } +}; + +void AdvanceAndCheck(u32 idx, int downcount, int expected_lateness = 0, int cpu_downcount = 0) +{ + s_callbacks_ran_flags = 0; + s_expected_callback = CB_IDS[idx]; + s_lateness = expected_lateness; + + PowerPC::ppcState.downcount = cpu_downcount; // Pretend we executed X cycles of instructions. + CoreTiming::Advance(); + + EXPECT_EQ(decltype(s_callbacks_ran_flags)().set(idx), s_callbacks_ran_flags); + EXPECT_EQ(downcount, PowerPC::ppcState.downcount); +} + +TEST(CoreTiming, BasicOrder) +{ + ScopeInit guard; + + CoreTiming::EventType* cb_a = CoreTiming::RegisterEvent("callbackA", CallbackTemplate<0>); + CoreTiming::EventType* cb_b = CoreTiming::RegisterEvent("callbackB", CallbackTemplate<1>); + CoreTiming::EventType* cb_c = CoreTiming::RegisterEvent("callbackC", CallbackTemplate<2>); + CoreTiming::EventType* cb_d = CoreTiming::RegisterEvent("callbackD", CallbackTemplate<3>); + CoreTiming::EventType* cb_e = CoreTiming::RegisterEvent("callbackE", CallbackTemplate<4>); + + // Enter slice 0 + CoreTiming::Advance(); + + // D -> B -> C -> A -> E + CoreTiming::ScheduleEvent(1000, cb_a, CB_IDS[0]); + EXPECT_EQ(1000, PowerPC::ppcState.downcount); + CoreTiming::ScheduleEvent(500, cb_b, CB_IDS[1]); + EXPECT_EQ(500, PowerPC::ppcState.downcount); + CoreTiming::ScheduleEvent(800, cb_c, CB_IDS[2]); + EXPECT_EQ(500, PowerPC::ppcState.downcount); + CoreTiming::ScheduleEvent(100, cb_d, CB_IDS[3]); + EXPECT_EQ(100, PowerPC::ppcState.downcount); + CoreTiming::ScheduleEvent(1200, cb_e, CB_IDS[4]); + EXPECT_EQ(100, PowerPC::ppcState.downcount); + + AdvanceAndCheck(3, 400); + AdvanceAndCheck(1, 300); + AdvanceAndCheck(2, 200); + AdvanceAndCheck(0, 200); + AdvanceAndCheck(4, MAX_SLICE_LENGTH); +} + +TEST(CoreTiming, SharedSlot) +{ + ScopeInit guard; + + CoreTiming::EventType* cb_a = CoreTiming::RegisterEvent("callbackA", CallbackTemplate<0>); + CoreTiming::EventType* cb_b = CoreTiming::RegisterEvent("callbackB", CallbackTemplate<1>); + CoreTiming::EventType* cb_c = CoreTiming::RegisterEvent("callbackC", CallbackTemplate<2>); + CoreTiming::EventType* cb_d = CoreTiming::RegisterEvent("callbackD", CallbackTemplate<3>); + CoreTiming::EventType* cb_e = CoreTiming::RegisterEvent("callbackE", CallbackTemplate<4>); + + CoreTiming::ScheduleEvent(1000, cb_a, CB_IDS[0]); + CoreTiming::ScheduleEvent(1000, cb_b, CB_IDS[1]); + CoreTiming::ScheduleEvent(1000, cb_c, CB_IDS[2]); + CoreTiming::ScheduleEvent(1000, cb_d, CB_IDS[3]); + CoreTiming::ScheduleEvent(1000, cb_e, CB_IDS[4]); + + // Enter slice 0 + CoreTiming::Advance(); + EXPECT_EQ(1000, PowerPC::ppcState.downcount); + + s_callbacks_ran_flags = 0; + s_lateness = 0; + s_expected_callback = 0; + PowerPC::ppcState.downcount = 0; + CoreTiming::Advance(); + EXPECT_EQ(MAX_SLICE_LENGTH, PowerPC::ppcState.downcount); + EXPECT_EQ(0x1FULL, s_callbacks_ran_flags.to_ullong()); +} + +TEST(CoreTiming, PredictableLateness) +{ + ScopeInit guard; + + CoreTiming::EventType* cb_a = CoreTiming::RegisterEvent("callbackA", CallbackTemplate<0>); + CoreTiming::EventType* cb_b = CoreTiming::RegisterEvent("callbackB", CallbackTemplate<1>); + + // Enter slice 0 + CoreTiming::Advance(); + + CoreTiming::ScheduleEvent(100, cb_a, CB_IDS[0]); + CoreTiming::ScheduleEvent(200, cb_b, CB_IDS[1]); + + AdvanceAndCheck(0, 90, 10, -10); // (100 - 10) + AdvanceAndCheck(1, MAX_SLICE_LENGTH, 50, -50); +} + +namespace ChainSchedulingTest +{ +static int s_reschedules = 0; + +static void RescheduleCallback(u64 userdata, s64 lateness) +{ + --s_reschedules; + EXPECT_TRUE(s_reschedules >= 0); + EXPECT_EQ(s_lateness, lateness); + + if (s_reschedules > 0) + CoreTiming::ScheduleEvent(1000, reinterpret_cast(userdata), userdata); +} +} + +TEST(CoreTiming, ChainScheduling) +{ + using namespace ChainSchedulingTest; + + ScopeInit guard; + + CoreTiming::EventType* cb_a = CoreTiming::RegisterEvent("callbackA", CallbackTemplate<0>); + CoreTiming::EventType* cb_b = CoreTiming::RegisterEvent("callbackB", CallbackTemplate<1>); + CoreTiming::EventType* cb_c = CoreTiming::RegisterEvent("callbackC", CallbackTemplate<2>); + CoreTiming::EventType* cb_rs = + CoreTiming::RegisterEvent("callbackReschedule", RescheduleCallback); + + // Enter slice 0 + CoreTiming::Advance(); + + CoreTiming::ScheduleEvent(800, cb_a, CB_IDS[0]); + CoreTiming::ScheduleEvent(1000, cb_b, CB_IDS[1]); + CoreTiming::ScheduleEvent(2200, cb_c, CB_IDS[2]); + CoreTiming::ScheduleEvent(1000, cb_rs, reinterpret_cast(cb_rs)); + EXPECT_EQ(800, PowerPC::ppcState.downcount); + + s_reschedules = 3; + AdvanceAndCheck(0, 200); // cb_a + AdvanceAndCheck(1, 1000); // cb_b, cb_rs + EXPECT_EQ(2, s_reschedules); + + PowerPC::ppcState.downcount = 0; + CoreTiming::Advance(); // cb_rs + EXPECT_EQ(1, s_reschedules); + EXPECT_EQ(200, PowerPC::ppcState.downcount); + + AdvanceAndCheck(2, 800); // cb_c + + PowerPC::ppcState.downcount = 0; + CoreTiming::Advance(); // cb_rs + EXPECT_EQ(0, s_reschedules); + EXPECT_EQ(MAX_SLICE_LENGTH, PowerPC::ppcState.downcount); +} + +namespace ScheduleIntoPastTest +{ +static CoreTiming::EventType* s_cb_next = nullptr; + +static void ChainCallback(u64 userdata, s64 lateness) +{ + EXPECT_EQ(CB_IDS[0] + 1, userdata); + EXPECT_EQ(0, lateness); + + CoreTiming::ScheduleEvent(-1000, s_cb_next, userdata - 1); +} +} + +// This can happen when scheduling from outside the CPU Thread. +// Also, if the callback is very late, it may reschedule itself for the next period which +// is also in the past. +TEST(CoreTiming, ScheduleIntoPast) +{ + using namespace ScheduleIntoPastTest; + + ScopeInit guard; + + s_cb_next = CoreTiming::RegisterEvent("callbackA", CallbackTemplate<0>); + CoreTiming::EventType* cb_b = CoreTiming::RegisterEvent("callbackB", CallbackTemplate<1>); + CoreTiming::EventType* cb_chain = CoreTiming::RegisterEvent("callbackChain", ChainCallback); + + // Enter slice 0 + CoreTiming::Advance(); + + CoreTiming::ScheduleEvent(1000, cb_chain, CB_IDS[0] + 1); + EXPECT_EQ(1000, PowerPC::ppcState.downcount); + + AdvanceAndCheck(0, MAX_SLICE_LENGTH, 1000); // Run cb_chain into late cb_a + + // Schedule late from wrong thread + // The problem with scheduling CPU events from outside the CPU Thread is that g_global_timer + // is not reliable outside the CPU Thread. It's possible for the other thread to sample the + // global timer right before the timer is updated by Advance() then submit a new event using + // the stale value, i.e. effectively half-way through the previous slice. + // NOTE: We're only testing that the scheduler doesn't break, not whether this makes sense. + Core::UndeclareAsCPUThread(); + CoreTiming::g_global_timer -= 1000; + CoreTiming::ScheduleEvent(0, cb_b, CB_IDS[1], CoreTiming::FromThread::NON_CPU); + CoreTiming::g_global_timer += 1000; + Core::DeclareAsCPUThread(); + AdvanceAndCheck(1, MAX_SLICE_LENGTH, MAX_SLICE_LENGTH + 1000); +} + +TEST(CoreTiming, Overclocking) +{ + ScopeInit guard; + + CoreTiming::EventType* cb_a = CoreTiming::RegisterEvent("callbackA", CallbackTemplate<0>); + CoreTiming::EventType* cb_b = CoreTiming::RegisterEvent("callbackB", CallbackTemplate<1>); + CoreTiming::EventType* cb_c = CoreTiming::RegisterEvent("callbackC", CallbackTemplate<2>); + CoreTiming::EventType* cb_d = CoreTiming::RegisterEvent("callbackD", CallbackTemplate<3>); + CoreTiming::EventType* cb_e = CoreTiming::RegisterEvent("callbackE", CallbackTemplate<4>); + + // Overclock + SConfig::GetInstance().m_OCEnable = true; + SConfig::GetInstance().m_OCFactor = 2.0; + + // Enter slice 0 + // Updates s_last_OC_factor. + CoreTiming::Advance(); + + CoreTiming::ScheduleEvent(100, cb_a, CB_IDS[0]); + CoreTiming::ScheduleEvent(200, cb_b, CB_IDS[1]); + CoreTiming::ScheduleEvent(400, cb_c, CB_IDS[2]); + CoreTiming::ScheduleEvent(800, cb_d, CB_IDS[3]); + CoreTiming::ScheduleEvent(1600, cb_e, CB_IDS[4]); + EXPECT_EQ(200, PowerPC::ppcState.downcount); + + AdvanceAndCheck(0, 200); // (200 - 100) * 2 + AdvanceAndCheck(1, 400); // (400 - 200) * 2 + AdvanceAndCheck(2, 800); // (800 - 400) * 2 + AdvanceAndCheck(3, 1600); // (1600 - 800) * 2 + AdvanceAndCheck(4, MAX_SLICE_LENGTH * 2); + + // Underclock + SConfig::GetInstance().m_OCFactor = 0.5; + CoreTiming::Advance(); + + CoreTiming::ScheduleEvent(100, cb_a, CB_IDS[0]); + CoreTiming::ScheduleEvent(200, cb_b, CB_IDS[1]); + CoreTiming::ScheduleEvent(400, cb_c, CB_IDS[2]); + CoreTiming::ScheduleEvent(800, cb_d, CB_IDS[3]); + CoreTiming::ScheduleEvent(1600, cb_e, CB_IDS[4]); + EXPECT_EQ(50, PowerPC::ppcState.downcount); + + AdvanceAndCheck(0, 50); // (200 - 100) / 2 + AdvanceAndCheck(1, 100); // (400 - 200) / 2 + AdvanceAndCheck(2, 200); // (800 - 400) / 2 + AdvanceAndCheck(3, 400); // (1600 - 800) / 2 + AdvanceAndCheck(4, MAX_SLICE_LENGTH / 2); + + // Try switching the clock mid-emulation + SConfig::GetInstance().m_OCFactor = 1.0; + CoreTiming::Advance(); + + CoreTiming::ScheduleEvent(100, cb_a, CB_IDS[0]); + CoreTiming::ScheduleEvent(200, cb_b, CB_IDS[1]); + CoreTiming::ScheduleEvent(400, cb_c, CB_IDS[2]); + CoreTiming::ScheduleEvent(800, cb_d, CB_IDS[3]); + CoreTiming::ScheduleEvent(1600, cb_e, CB_IDS[4]); + EXPECT_EQ(100, PowerPC::ppcState.downcount); + + AdvanceAndCheck(0, 100); // (200 - 100) + SConfig::GetInstance().m_OCFactor = 2.0; + AdvanceAndCheck(1, 400); // (400 - 200) * 2 + AdvanceAndCheck(2, 800); // (800 - 400) * 2 + SConfig::GetInstance().m_OCFactor = 0.1f; + AdvanceAndCheck(3, 80); // (1600 - 800) / 10 + SConfig::GetInstance().m_OCFactor = 1.0; + AdvanceAndCheck(4, MAX_SLICE_LENGTH); +} From 59465911d741ce9ee7c7481927fd5a5648d1fe50 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Fri, 2 Sep 2016 02:18:14 +0000 Subject: [PATCH 6/9] CoreTiming: Fix scheduling into the past ForceExceptionCheck messes up the downcount and slice length if the callback is scheduled into the past (g_slice_length becomes negative) --- Source/Core/Core/CoreTiming.cpp | 3 ++- Source/UnitTests/Core/CoreTimingTest.cpp | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp index 329f63a7ae..50747cf59a 100644 --- a/Source/Core/Core/CoreTiming.cpp +++ b/Source/Core/Core/CoreTiming.cpp @@ -282,11 +282,12 @@ void RemoveAllEvents(EventType* event_type) void ForceExceptionCheck(s64 cycles) { + cycles = std::max(0, cycles); if (DowncountToCycles(PowerPC::ppcState.downcount) > cycles) { // downcount is always (much) smaller than MAX_INT so we can safely cast cycles to an int here. // Account for cycles already executed by adjusting the g_slice_length - g_slice_length -= (DowncountToCycles(PowerPC::ppcState.downcount) - static_cast(cycles)); + g_slice_length -= DowncountToCycles(PowerPC::ppcState.downcount) - static_cast(cycles); PowerPC::ppcState.downcount = CyclesToDowncount(static_cast(cycles)); } } diff --git a/Source/UnitTests/Core/CoreTimingTest.cpp b/Source/UnitTests/Core/CoreTimingTest.cpp index 305232e8a3..f87e4a7b21 100644 --- a/Source/UnitTests/Core/CoreTimingTest.cpp +++ b/Source/UnitTests/Core/CoreTimingTest.cpp @@ -241,6 +241,13 @@ TEST(CoreTiming, ScheduleIntoPast) CoreTiming::g_global_timer += 1000; Core::DeclareAsCPUThread(); AdvanceAndCheck(1, MAX_SLICE_LENGTH, MAX_SLICE_LENGTH + 1000); + + // Schedule directly into the past from the CPU. + // This shouldn't happen in practice, but it's best if we don't mess up the slice length and + // downcount if we do. + CoreTiming::ScheduleEvent(-1000, s_cb_next, CB_IDS[0]); + EXPECT_EQ(0, PowerPC::ppcState.downcount); + AdvanceAndCheck(0, MAX_SLICE_LENGTH, 1000); } TEST(CoreTiming, Overclocking) From fb5537213a2c3184c9962e9d5f7c1f5985c373e4 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Thu, 1 Sep 2016 11:39:05 +0000 Subject: [PATCH 7/9] CoreTiming: Document initial startup behavior Events don't update the downcount until after the first Advance(), thus Advance() must be called once before scheduling works normally. --- Source/Core/Core/CoreTiming.cpp | 5 +++++ Source/Core/Core/CoreTiming.h | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp index 50747cf59a..e2ab497e86 100644 --- a/Source/Core/Core/CoreTiming.cpp +++ b/Source/Core/Core/CoreTiming.cpp @@ -129,6 +129,11 @@ void Init() g_slice_length = MAX_SLICE_LENGTH; g_global_timer = 0; s_idled_cycles = 0; + + // The time between CoreTiming being intialized and the first call to Advance() is considered + // the slice boundary between slice -1 and slice 0. Dispatcher loops must call Advance() before + // executing the first PPC cycle of each slice to prepare the slice length and downcount for + // that slice. s_is_global_timer_sane = true; s_ev_lost = RegisterEvent("_lost_event", &EmptyTimedCallback); diff --git a/Source/Core/Core/CoreTiming.h b/Source/Core/Core/CoreTiming.h index 4f209c97bd..28263be94c 100644 --- a/Source/Core/Core/CoreTiming.h +++ b/Source/Core/Core/CoreTiming.h @@ -31,6 +31,8 @@ extern u64 g_fake_TB_start_ticks; extern int g_slice_length; extern float g_last_OC_factor_inverted; +// CoreTiming begins at the boundary of timing slice -1. An initial call to Advance() is +// required to end slice -1 and start slice 0 before the first cycle of code is executed. void Init(); void Shutdown(); @@ -60,12 +62,22 @@ enum class FromThread }; // userdata MAY NOT CONTAIN POINTERS. userdata might get written and reloaded from savestates. +// After the first Advance, the slice lengths and the downcount will be reduced whenever an event +// is scheduled earlier than the current values (when scheduled from the CPU Thread only). +// Scheduling from a callback will not update the downcount until the Advance() completes. void ScheduleEvent(s64 cycles_into_future, EventType* event_type, u64 userdata = 0, FromThread from = FromThread::CPU); // We only permit one event of each type in the queue at a time. void RemoveEvent(EventType* event_type); void RemoveAllEvents(EventType* event_type); + +// Advance must be called at the beginning of dispatcher loops, not the end. Advance() ends +// the previous timing slice and begins the next one, you must Advance from the previous +// slice to the current one before executing any cycles. CoreTiming starts in slice -1 so an +// Advance() is required to initialize the slice length before the first cycle of emulated +// instructions is executed. +// NOTE: Advance updates the PowerPC downcount and performs a PPC external exception check. void Advance(); void MoveEvents(); From 3a85aa9817b56d2c289a28c05790480357bb2224 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Sat, 3 Sep 2016 02:43:20 +0000 Subject: [PATCH 8/9] EXI_DeviceMemoryCard: Use CoreTiming/DoState correctly CoreTiming gets restored before ExpansionInterface so CoreTiming events need to already be registered before the save state loading begins. This means that the callbacks must be registered unconditionally instead of on-demand. --- Source/Core/Core/HW/EXI.cpp | 4 ++ Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp | 60 +++++++++++++------- Source/Core/Core/HW/EXI_DeviceMemoryCard.h | 12 ++-- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/Source/Core/Core/HW/EXI.cpp b/Source/Core/Core/HW/EXI.cpp index 32c6ce80a7..1b18c566c3 100644 --- a/Source/Core/Core/HW/EXI.cpp +++ b/Source/Core/Core/HW/EXI.cpp @@ -12,6 +12,7 @@ #include "Core/CoreTiming.h" #include "Core/HW/EXI.h" #include "Core/HW/EXI_Channel.h" +#include "Core/HW/EXI_DeviceMemoryCard.h" #include "Core/HW/MMIO.h" #include "Core/HW/ProcessorInterface.h" #include "Core/HW/Sram.h" @@ -38,6 +39,7 @@ void Init() InitSRAM(); } + CEXIMemoryCard::Init(); for (u32 i = 0; i < MAX_EXI_CHANNELS; i++) g_Channels[i] = std::make_unique(i); @@ -65,6 +67,8 @@ void Shutdown() { for (auto& channel : g_Channels) channel.reset(); + + CEXIMemoryCard::Shutdown(); } void DoState(PointerWrap& p) diff --git a/Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp b/Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp index 3ca0ddcfcf..fdef3181fb 100644 --- a/Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp +++ b/Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp @@ -2,6 +2,7 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include #include #include #include @@ -41,6 +42,9 @@ static const u32 MC_TRANSFER_RATE_READ = 512 * 1024; static const u32 MC_TRANSFER_RATE_WRITE = (u32)(96.125f * 1024.0f); +static std::array s_et_cmd_done; +static std::array s_et_transfer_complete; + // Takes care of the nasty recovery of the 'this' pointer from card_index, // stored in the userdata parameter of the CoreTiming event. void CEXIMemoryCard::EventCompleteFindInstance(u64 userdata, @@ -70,25 +74,37 @@ void CEXIMemoryCard::TransferCompleteCallback(u64 userdata, s64 cyclesLate) [](CEXIMemoryCard* instance) { instance->TransferComplete(); }); } +void CEXIMemoryCard::Init() +{ + static constexpr char DONE_PREFIX[] = "memcardDone"; + static constexpr char TRANSFER_COMPLETE_PREFIX[] = "memcardTransferComplete"; + + static_assert(s_et_cmd_done.size() == s_et_transfer_complete.size(), "Event array size differs"); + for (unsigned int i = 0; i < s_et_cmd_done.size(); ++i) + { + std::string name = DONE_PREFIX; + name += static_cast('A' + i); + s_et_cmd_done[i] = CoreTiming::RegisterEvent(name, CmdDoneCallback); + + name = TRANSFER_COMPLETE_PREFIX; + name += static_cast('A' + i); + s_et_transfer_complete[i] = CoreTiming::RegisterEvent(name, TransferCompleteCallback); + } +} + +void CEXIMemoryCard::Shutdown() +{ + s_et_cmd_done.fill(nullptr); + s_et_transfer_complete.fill(nullptr); +} + CEXIMemoryCard::CEXIMemoryCard(const int index, bool gciFolder) : card_index(index) { - struct - { - const char* done; - const char* transfer_complete; - } const event_names[] = { - {"memcardDoneA", "memcardTransferCompleteA"}, {"memcardDoneB", "memcardTransferCompleteB"}, - }; + _assert_msg_(EXPANSIONINTERFACE, static_cast(index) < s_et_cmd_done.size(), + "Trying to create invalid memory card index %d.", index); - if ((size_t)index >= ArraySize(event_names)) - { - PanicAlertT("Trying to create invalid memory card index."); - } - // we're potentially leaking events here, since there's no RemoveEvent - // until emu shutdown, but I guess it's inconsequential - et_cmd_done = CoreTiming::RegisterEvent(event_names[index].done, CmdDoneCallback); - et_transfer_complete = - CoreTiming::RegisterEvent(event_names[index].transfer_complete, TransferCompleteCallback); + // NOTE: When loading a save state, DMA completion callbacks (s_et_transfer_complete) and such + // may have been restored, we need to anticipate those arriving. interruptSwitch = 0; m_bInterruptSet = 0; @@ -248,8 +264,8 @@ void CEXIMemoryCard::SetupRawMemcard(u16 sizeMb) CEXIMemoryCard::~CEXIMemoryCard() { - CoreTiming::RemoveEvent(et_cmd_done); - CoreTiming::RemoveEvent(et_transfer_complete); + CoreTiming::RemoveEvent(s_et_cmd_done[card_index]); + CoreTiming::RemoveEvent(s_et_transfer_complete[card_index]); } bool CEXIMemoryCard::UseDelayedTransferCompletion() const @@ -279,8 +295,8 @@ void CEXIMemoryCard::TransferComplete() void CEXIMemoryCard::CmdDoneLater(u64 cycles) { - CoreTiming::RemoveEvent(et_cmd_done); - CoreTiming::ScheduleEvent((int)cycles, et_cmd_done, (u64)card_index); + CoreTiming::RemoveEvent(s_et_cmd_done[card_index]); + CoreTiming::ScheduleEvent((int)cycles, s_et_cmd_done[card_index], (u64)card_index); } void CEXIMemoryCard::SetCS(int cs) @@ -547,7 +563,7 @@ void CEXIMemoryCard::DMARead(u32 _uAddr, u32 _uSize) // Schedule transfer complete later based on read speed CoreTiming::ScheduleEvent(_uSize * (SystemTimers::GetTicksPerSecond() / MC_TRANSFER_RATE_READ), - et_transfer_complete, (u64)card_index); + s_et_transfer_complete[card_index], (u64)card_index); } // DMA write are preceded by all of the necessary setup via IMMWrite @@ -563,5 +579,5 @@ void CEXIMemoryCard::DMAWrite(u32 _uAddr, u32 _uSize) // Schedule transfer complete later based on write speed CoreTiming::ScheduleEvent(_uSize * (SystemTimers::GetTicksPerSecond() / MC_TRANSFER_RATE_WRITE), - et_transfer_complete, (u64)card_index); + s_et_transfer_complete[card_index], (u64)card_index); } diff --git a/Source/Core/Core/HW/EXI_DeviceMemoryCard.h b/Source/Core/Core/HW/EXI_DeviceMemoryCard.h index 5cafcfce29..95c0dd1e1f 100644 --- a/Source/Core/Core/HW/EXI_DeviceMemoryCard.h +++ b/Source/Core/Core/HW/EXI_DeviceMemoryCard.h @@ -9,10 +9,6 @@ #include "Core/HW/EXI_Device.h" -namespace CoreTiming -{ -struct EventType; -} class MemoryCardBase; class PointerWrap; @@ -30,6 +26,12 @@ public: void DMARead(u32 _uAddr, u32 _uSize) override; void DMAWrite(u32 _uAddr, u32 _uSize) override; + // CoreTiming events need to be registered during boot since CoreTiming is DoState()-ed + // before ExpansionInterface so we'll lose the save stated events if the callbacks are + // not already registered first. + static void Init(); + static void Shutdown(); + private: void SetupGciFolder(u16 sizeMb); void SetupRawMemcard(u16 sizeMb); @@ -71,8 +73,6 @@ private: }; int card_index; - CoreTiming::EventType* et_cmd_done = nullptr; - CoreTiming::EventType* et_transfer_complete = nullptr; //! memory card state // STATE_TO_SAVE From 9d8f37301624681685c479e976d26f3d266c1129 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Sat, 3 Sep 2016 05:12:41 +0000 Subject: [PATCH 9/9] VideoSoftware: Don't Init the PixelEngine twice PixelEngine is initialized by InitializeShared() --- Source/Core/VideoBackends/Software/SWmain.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/Core/VideoBackends/Software/SWmain.cpp b/Source/Core/VideoBackends/Software/SWmain.cpp index 7d7ca38361..ab1910fd1e 100644 --- a/Source/Core/VideoBackends/Software/SWmain.cpp +++ b/Source/Core/VideoBackends/Software/SWmain.cpp @@ -142,7 +142,6 @@ bool VideoSoftware::Initialize(void* window_handle) SWOGLWindow::Init(window_handle); - PixelEngine::Init(); Clipper::Init(); Rasterizer::Init(); SWRenderer::Init();