From f34d3d7e84a1396d4d4f07ec1434b06b1f39bb8a Mon Sep 17 00:00:00 2001 From: Liam Date: Sat, 23 Dec 2023 13:58:09 -0500 Subject: [PATCH 1/4] core_timing: remove user data value --- src/audio_core/device/device_session.cpp | 4 +- src/core/core_timing.cpp | 32 +++++--------- src/core/core_timing.h | 16 +++---- src/core/hle/kernel/k_hardware_timer.cpp | 18 ++++---- src/core/hle/kernel/kernel.cpp | 2 +- src/core/hle/service/hid/hidbus.cpp | 8 ++-- src/core/hle/service/hid/hidbus.h | 2 +- src/core/hle/service/hid/resource_manager.cpp | 42 +++++++++---------- src/core/hle/service/hid/resource_manager.h | 8 ++-- .../hle/service/nvnflinger/nvnflinger.cpp | 8 ++-- src/core/memory/cheat_engine.cpp | 8 ++-- src/core/memory/cheat_engine.h | 2 +- src/core/tools/freezer.cpp | 17 ++++---- src/core/tools/freezer.h | 2 +- src/tests/core/core_timing.cpp | 14 +++---- 15 files changed, 79 insertions(+), 104 deletions(-) diff --git a/src/audio_core/device/device_session.cpp b/src/audio_core/device/device_session.cpp index c41d9d1eae..ee42ae529d 100644 --- a/src/audio_core/device/device_session.cpp +++ b/src/audio_core/device/device_session.cpp @@ -18,9 +18,7 @@ constexpr auto INCREMENT_TIME{5ms}; DeviceSession::DeviceSession(Core::System& system_) : system{system_}, thread_event{Core::Timing::CreateEvent( "AudioOutSampleTick", - [this](std::uintptr_t, s64 time, std::chrono::nanoseconds) { - return ThreadFunc(); - })} {} + [this](s64 time, std::chrono::nanoseconds) { return ThreadFunc(); })} {} DeviceSession::~DeviceSession() { Finalize(); diff --git a/src/core/core_timing.cpp b/src/core/core_timing.cpp index d6b5abc68e..3b7b0aa45f 100644 --- a/src/core/core_timing.cpp +++ b/src/core/core_timing.cpp @@ -29,7 +29,6 @@ std::shared_ptr CreateEvent(std::string name, TimedCallback&& callbac struct CoreTiming::Event { s64 time; u64 fifo_order; - std::uintptr_t user_data; std::weak_ptr type; s64 reschedule_time; heap_t::handle_type handle{}; @@ -67,9 +66,6 @@ void CoreTiming::Initialize(std::function&& on_thread_init_) { event_fifo_id = 0; shutting_down = false; cpu_ticks = 0; - const auto empty_timed_callback = [](std::uintptr_t, u64, std::chrono::nanoseconds) - -> std::optional { return std::nullopt; }; - ev_lost = CreateEvent("_lost_event", empty_timed_callback); if (is_multicore) { timer_thread = std::make_unique(ThreadEntry, std::ref(*this)); } @@ -119,14 +115,12 @@ bool CoreTiming::HasPendingEvents() const { } void CoreTiming::ScheduleEvent(std::chrono::nanoseconds ns_into_future, - const std::shared_ptr& event_type, - std::uintptr_t user_data, bool absolute_time) { + const std::shared_ptr& event_type, bool absolute_time) { { std::scoped_lock scope{basic_lock}; const auto next_time{absolute_time ? ns_into_future : GetGlobalTimeNs() + ns_into_future}; - auto h{event_queue.emplace( - Event{next_time.count(), event_fifo_id++, user_data, event_type, 0})}; + auto h{event_queue.emplace(Event{next_time.count(), event_fifo_id++, event_type, 0})}; (*h).handle = h; } @@ -136,28 +130,27 @@ void CoreTiming::ScheduleEvent(std::chrono::nanoseconds ns_into_future, void CoreTiming::ScheduleLoopingEvent(std::chrono::nanoseconds start_time, std::chrono::nanoseconds resched_time, const std::shared_ptr& event_type, - std::uintptr_t user_data, bool absolute_time) { + bool absolute_time) { { std::scoped_lock scope{basic_lock}; const auto next_time{absolute_time ? start_time : GetGlobalTimeNs() + start_time}; - auto h{event_queue.emplace(Event{next_time.count(), event_fifo_id++, user_data, event_type, - resched_time.count()})}; + auto h{event_queue.emplace( + Event{next_time.count(), event_fifo_id++, event_type, resched_time.count()})}; (*h).handle = h; } event.Set(); } -void CoreTiming::UnscheduleEvent(const std::shared_ptr& event_type, - std::uintptr_t user_data, bool wait) { +void CoreTiming::UnscheduleEvent(const std::shared_ptr& event_type, bool wait) { { std::scoped_lock lk{basic_lock}; std::vector to_remove; for (auto itr = event_queue.begin(); itr != event_queue.end(); itr++) { const Event& e = *itr; - if (e.type.lock().get() == event_type.get() && e.user_data == user_data) { + if (e.type.lock().get() == event_type.get()) { to_remove.push_back(itr->handle); } } @@ -209,7 +202,6 @@ std::optional CoreTiming::Advance() { if (const auto event_type{evt.type.lock()}) { if (evt.reschedule_time == 0) { - const auto evt_user_data = evt.user_data; const auto evt_time = evt.time; event_queue.pop(); @@ -217,16 +209,14 @@ std::optional CoreTiming::Advance() { basic_lock.unlock(); event_type->callback( - evt_user_data, evt_time, - std::chrono::nanoseconds{GetGlobalTimeNs().count() - evt_time}); + evt_time, std::chrono::nanoseconds{GetGlobalTimeNs().count() - evt_time}); basic_lock.lock(); } else { basic_lock.unlock(); const auto new_schedule_time{event_type->callback( - evt.user_data, evt.time, - std::chrono::nanoseconds{GetGlobalTimeNs().count() - evt.time})}; + evt.time, std::chrono::nanoseconds{GetGlobalTimeNs().count() - evt.time})}; basic_lock.lock(); @@ -241,8 +231,8 @@ std::optional CoreTiming::Advance() { next_time = pause_end_time + next_schedule_time; } - event_queue.update(evt.handle, Event{next_time, event_fifo_id++, evt.user_data, - evt.type, next_schedule_time, evt.handle}); + event_queue.update(evt.handle, Event{next_time, event_fifo_id++, evt.type, + next_schedule_time, evt.handle}); } } diff --git a/src/core/core_timing.h b/src/core/core_timing.h index 21548f0a9b..d86337cdc9 100644 --- a/src/core/core_timing.h +++ b/src/core/core_timing.h @@ -22,7 +22,7 @@ namespace Core::Timing { /// A callback that may be scheduled for a particular core timing event. using TimedCallback = std::function( - std::uintptr_t user_data, s64 time, std::chrono::nanoseconds ns_late)>; + s64 time, std::chrono::nanoseconds ns_late)>; /// Contains the characteristics of a particular event. struct EventType { @@ -89,22 +89,19 @@ public: /// Schedules an event in core timing void ScheduleEvent(std::chrono::nanoseconds ns_into_future, - const std::shared_ptr& event_type, std::uintptr_t user_data = 0, - bool absolute_time = false); + const std::shared_ptr& event_type, bool absolute_time = false); /// Schedules an event which will automatically re-schedule itself with the given time, until /// unscheduled void ScheduleLoopingEvent(std::chrono::nanoseconds start_time, std::chrono::nanoseconds resched_time, const std::shared_ptr& event_type, - std::uintptr_t user_data = 0, bool absolute_time = false); + bool absolute_time = false); - void UnscheduleEvent(const std::shared_ptr& event_type, std::uintptr_t user_data, - bool wait = true); + void UnscheduleEvent(const std::shared_ptr& event_type, bool wait = true); - void UnscheduleEventWithoutWait(const std::shared_ptr& event_type, - std::uintptr_t user_data) { - UnscheduleEvent(event_type, user_data, false); + void UnscheduleEventWithoutWait(const std::shared_ptr& event_type) { + UnscheduleEvent(event_type, false); } void AddTicks(u64 ticks_to_add); @@ -158,7 +155,6 @@ private: heap_t event_queue; u64 event_fifo_id = 0; - std::shared_ptr ev_lost; Common::Event event{}; Common::Event pause_event{}; mutable std::mutex basic_lock; diff --git a/src/core/hle/kernel/k_hardware_timer.cpp b/src/core/hle/kernel/k_hardware_timer.cpp index 8e2e40307d..2a29a487c1 100644 --- a/src/core/hle/kernel/k_hardware_timer.cpp +++ b/src/core/hle/kernel/k_hardware_timer.cpp @@ -10,15 +10,15 @@ namespace Kernel { void KHardwareTimer::Initialize() { // Create the timing callback to register with CoreTiming. - m_event_type = Core::Timing::CreateEvent( - "KHardwareTimer::Callback", [](std::uintptr_t timer_handle, s64, std::chrono::nanoseconds) { - reinterpret_cast(timer_handle)->DoTask(); - return std::nullopt; - }); + m_event_type = Core::Timing::CreateEvent("KHardwareTimer::Callback", + [this](s64, std::chrono::nanoseconds) { + this->DoTask(); + return std::nullopt; + }); } void KHardwareTimer::Finalize() { - m_kernel.System().CoreTiming().UnscheduleEvent(m_event_type, reinterpret_cast(this)); + m_kernel.System().CoreTiming().UnscheduleEvent(m_event_type); m_wakeup_time = std::numeric_limits::max(); m_event_type.reset(); } @@ -57,13 +57,11 @@ void KHardwareTimer::EnableInterrupt(s64 wakeup_time) { m_wakeup_time = wakeup_time; m_kernel.System().CoreTiming().ScheduleEvent(std::chrono::nanoseconds{m_wakeup_time}, - m_event_type, reinterpret_cast(this), - true); + m_event_type, true); } void KHardwareTimer::DisableInterrupt() { - m_kernel.System().CoreTiming().UnscheduleEventWithoutWait(m_event_type, - reinterpret_cast(this)); + m_kernel.System().CoreTiming().UnscheduleEventWithoutWait(m_event_type); m_wakeup_time = std::numeric_limits::max(); } diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index e479dacde1..b515f6a18c 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -247,7 +247,7 @@ struct KernelCore::Impl { void InitializePreemption(KernelCore& kernel) { preemption_event = Core::Timing::CreateEvent( "PreemptionCallback", - [this, &kernel](std::uintptr_t, s64 time, + [this, &kernel](s64 time, std::chrono::nanoseconds) -> std::optional { { KScopedSchedulerLock lock(kernel); diff --git a/src/core/hle/service/hid/hidbus.cpp b/src/core/hle/service/hid/hidbus.cpp index 80aac221b8..854de2fe2f 100644 --- a/src/core/hle/service/hid/hidbus.cpp +++ b/src/core/hle/service/hid/hidbus.cpp @@ -49,10 +49,10 @@ HidBus::HidBus(Core::System& system_) // Register update callbacks hidbus_update_event = Core::Timing::CreateEvent( "Hidbus::UpdateCallback", - [this](std::uintptr_t user_data, s64 time, + [this](s64 time, std::chrono::nanoseconds ns_late) -> std::optional { const auto guard = LockService(); - UpdateHidbus(user_data, ns_late); + UpdateHidbus(ns_late); return std::nullopt; }); @@ -61,10 +61,10 @@ HidBus::HidBus(Core::System& system_) } HidBus::~HidBus() { - system.CoreTiming().UnscheduleEvent(hidbus_update_event, 0); + system.CoreTiming().UnscheduleEvent(hidbus_update_event); } -void HidBus::UpdateHidbus(std::uintptr_t user_data, std::chrono::nanoseconds ns_late) { +void HidBus::UpdateHidbus(std::chrono::nanoseconds ns_late) { if (is_hidbus_enabled) { for (std::size_t i = 0; i < devices.size(); ++i) { if (!devices[i].is_device_initializated) { diff --git a/src/core/hle/service/hid/hidbus.h b/src/core/hle/service/hid/hidbus.h index c29b5e8823..85a1df1334 100644 --- a/src/core/hle/service/hid/hidbus.h +++ b/src/core/hle/service/hid/hidbus.h @@ -108,7 +108,7 @@ private: void DisableJoyPollingReceiveMode(HLERequestContext& ctx); void SetStatusManagerType(HLERequestContext& ctx); - void UpdateHidbus(std::uintptr_t user_data, std::chrono::nanoseconds ns_late); + void UpdateHidbus(std::chrono::nanoseconds ns_late); std::optional GetDeviceIndexFromHandle(BusHandle handle) const; template diff --git a/src/core/hle/service/hid/resource_manager.cpp b/src/core/hle/service/hid/resource_manager.cpp index 6c6cbd8027..afc61f70dd 100644 --- a/src/core/hle/service/hid/resource_manager.cpp +++ b/src/core/hle/service/hid/resource_manager.cpp @@ -227,8 +227,7 @@ void ResourceManager::EnableTouchScreen(u64 aruid, bool is_enabled) { applet_resource->EnableTouchScreen(aruid, is_enabled); } -void ResourceManager::UpdateControllers(std::uintptr_t user_data, - std::chrono::nanoseconds ns_late) { +void ResourceManager::UpdateControllers(std::chrono::nanoseconds ns_late) { auto& core_timing = system.CoreTiming(); debug_pad->OnUpdate(core_timing); digitizer->OnUpdate(core_timing); @@ -241,20 +240,19 @@ void ResourceManager::UpdateControllers(std::uintptr_t user_data, capture_button->OnUpdate(core_timing); } -void ResourceManager::UpdateNpad(std::uintptr_t user_data, std::chrono::nanoseconds ns_late) { +void ResourceManager::UpdateNpad(std::chrono::nanoseconds ns_late) { auto& core_timing = system.CoreTiming(); npad->OnUpdate(core_timing); } -void ResourceManager::UpdateMouseKeyboard(std::uintptr_t user_data, - std::chrono::nanoseconds ns_late) { +void ResourceManager::UpdateMouseKeyboard(std::chrono::nanoseconds ns_late) { auto& core_timing = system.CoreTiming(); mouse->OnUpdate(core_timing); debug_mouse->OnUpdate(core_timing); keyboard->OnUpdate(core_timing); } -void ResourceManager::UpdateMotion(std::uintptr_t user_data, std::chrono::nanoseconds ns_late) { +void ResourceManager::UpdateMotion(std::chrono::nanoseconds ns_late) { auto& core_timing = system.CoreTiming(); six_axis->OnUpdate(core_timing); seven_six_axis->OnUpdate(core_timing); @@ -273,34 +271,34 @@ IAppletResource::IAppletResource(Core::System& system_, std::shared_ptr std::optional { + [this, resource]( + s64 time, std::chrono::nanoseconds ns_late) -> std::optional { const auto guard = LockService(); - resource->UpdateNpad(user_data, ns_late); + resource->UpdateNpad(ns_late); return std::nullopt; }); default_update_event = Core::Timing::CreateEvent( "HID::UpdateDefaultCallback", - [this, resource](std::uintptr_t user_data, s64 time, std::chrono::nanoseconds ns_late) - -> std::optional { + [this, resource]( + s64 time, std::chrono::nanoseconds ns_late) -> std::optional { const auto guard = LockService(); - resource->UpdateControllers(user_data, ns_late); + resource->UpdateControllers(ns_late); return std::nullopt; }); mouse_keyboard_update_event = Core::Timing::CreateEvent( "HID::UpdateMouseKeyboardCallback", - [this, resource](std::uintptr_t user_data, s64 time, std::chrono::nanoseconds ns_late) - -> std::optional { + [this, resource]( + s64 time, std::chrono::nanoseconds ns_late) -> std::optional { const auto guard = LockService(); - resource->UpdateMouseKeyboard(user_data, ns_late); + resource->UpdateMouseKeyboard(ns_late); return std::nullopt; }); motion_update_event = Core::Timing::CreateEvent( "HID::UpdateMotionCallback", - [this, resource](std::uintptr_t user_data, s64 time, std::chrono::nanoseconds ns_late) - -> std::optional { + [this, resource]( + s64 time, std::chrono::nanoseconds ns_late) -> std::optional { const auto guard = LockService(); - resource->UpdateMotion(user_data, ns_late); + resource->UpdateMotion(ns_late); return std::nullopt; }); @@ -314,10 +312,10 @@ IAppletResource::IAppletResource(Core::System& system_, std::shared_ptrFreeAppletResourceId(aruid); } diff --git a/src/core/hle/service/hid/resource_manager.h b/src/core/hle/service/hid/resource_manager.h index 5ad7cb5649..5a6596099f 100644 --- a/src/core/hle/service/hid/resource_manager.h +++ b/src/core/hle/service/hid/resource_manager.h @@ -81,10 +81,10 @@ public: void EnablePadInput(u64 aruid, bool is_enabled); void EnableTouchScreen(u64 aruid, bool is_enabled); - void UpdateControllers(std::uintptr_t user_data, std::chrono::nanoseconds ns_late); - void UpdateNpad(std::uintptr_t user_data, std::chrono::nanoseconds ns_late); - void UpdateMouseKeyboard(std::uintptr_t user_data, std::chrono::nanoseconds ns_late); - void UpdateMotion(std::uintptr_t user_data, std::chrono::nanoseconds ns_late); + void UpdateControllers(std::chrono::nanoseconds ns_late); + void UpdateNpad(std::chrono::nanoseconds ns_late); + void UpdateMouseKeyboard(std::chrono::nanoseconds ns_late); + void UpdateMotion(std::chrono::nanoseconds ns_late); private: Result CreateAppletResourceImpl(u64 aruid); diff --git a/src/core/hle/service/nvnflinger/nvnflinger.cpp b/src/core/hle/service/nvnflinger/nvnflinger.cpp index 6352b09a90..aa8aaa2d97 100644 --- a/src/core/hle/service/nvnflinger/nvnflinger.cpp +++ b/src/core/hle/service/nvnflinger/nvnflinger.cpp @@ -67,7 +67,7 @@ Nvnflinger::Nvnflinger(Core::System& system_, HosBinderDriverServer& hos_binder_ // Schedule the screen composition events multi_composition_event = Core::Timing::CreateEvent( "ScreenComposition", - [this](std::uintptr_t, s64 time, + [this](s64 time, std::chrono::nanoseconds ns_late) -> std::optional { vsync_signal.Set(); return std::chrono::nanoseconds(GetNextTicks()); @@ -75,7 +75,7 @@ Nvnflinger::Nvnflinger(Core::System& system_, HosBinderDriverServer& hos_binder_ single_composition_event = Core::Timing::CreateEvent( "ScreenComposition", - [this](std::uintptr_t, s64 time, + [this](s64 time, std::chrono::nanoseconds ns_late) -> std::optional { const auto lock_guard = Lock(); Compose(); @@ -93,11 +93,11 @@ Nvnflinger::Nvnflinger(Core::System& system_, HosBinderDriverServer& hos_binder_ Nvnflinger::~Nvnflinger() { if (system.IsMulticore()) { - system.CoreTiming().UnscheduleEvent(multi_composition_event, {}); + system.CoreTiming().UnscheduleEvent(multi_composition_event); vsync_thread.request_stop(); vsync_signal.Set(); } else { - system.CoreTiming().UnscheduleEvent(single_composition_event, {}); + system.CoreTiming().UnscheduleEvent(single_composition_event); } ShutdownLayers(); diff --git a/src/core/memory/cheat_engine.cpp b/src/core/memory/cheat_engine.cpp index 3fc4024dc7..7bc5b5ae50 100644 --- a/src/core/memory/cheat_engine.cpp +++ b/src/core/memory/cheat_engine.cpp @@ -190,15 +190,15 @@ CheatEngine::CheatEngine(System& system_, std::vector cheats_, } CheatEngine::~CheatEngine() { - core_timing.UnscheduleEvent(event, 0); + core_timing.UnscheduleEvent(event); } void CheatEngine::Initialize() { event = Core::Timing::CreateEvent( "CheatEngine::FrameCallback::" + Common::HexToString(metadata.main_nso_build_id), - [this](std::uintptr_t user_data, s64 time, + [this](s64 time, std::chrono::nanoseconds ns_late) -> std::optional { - FrameCallback(user_data, ns_late); + FrameCallback(ns_late); return std::nullopt; }); core_timing.ScheduleLoopingEvent(CHEAT_ENGINE_NS, CHEAT_ENGINE_NS, event); @@ -239,7 +239,7 @@ void CheatEngine::Reload(std::vector reload_cheats) { MICROPROFILE_DEFINE(Cheat_Engine, "Add-Ons", "Cheat Engine", MP_RGB(70, 200, 70)); -void CheatEngine::FrameCallback(std::uintptr_t, std::chrono::nanoseconds ns_late) { +void CheatEngine::FrameCallback(std::chrono::nanoseconds ns_late) { if (is_pending_reload.exchange(false)) { vm.LoadProgram(cheats); } diff --git a/src/core/memory/cheat_engine.h b/src/core/memory/cheat_engine.h index 284abdd287..ced2168d13 100644 --- a/src/core/memory/cheat_engine.h +++ b/src/core/memory/cheat_engine.h @@ -70,7 +70,7 @@ public: void Reload(std::vector reload_cheats); private: - void FrameCallback(std::uintptr_t user_data, std::chrono::nanoseconds ns_late); + void FrameCallback(std::chrono::nanoseconds ns_late); DmntCheatVm vm; CheatProcessMetadata metadata; diff --git a/src/core/tools/freezer.cpp b/src/core/tools/freezer.cpp index 98ebbbf329..9d42c726e4 100644 --- a/src/core/tools/freezer.cpp +++ b/src/core/tools/freezer.cpp @@ -51,18 +51,17 @@ void MemoryWriteWidth(Core::Memory::Memory& memory, u32 width, VAddr addr, u64 v Freezer::Freezer(Core::Timing::CoreTiming& core_timing_, Core::Memory::Memory& memory_) : core_timing{core_timing_}, memory{memory_} { - event = Core::Timing::CreateEvent( - "MemoryFreezer::FrameCallback", - [this](std::uintptr_t user_data, s64 time, - std::chrono::nanoseconds ns_late) -> std::optional { - FrameCallback(user_data, ns_late); - return std::nullopt; - }); + event = Core::Timing::CreateEvent("MemoryFreezer::FrameCallback", + [this](s64 time, std::chrono::nanoseconds ns_late) + -> std::optional { + FrameCallback(ns_late); + return std::nullopt; + }); core_timing.ScheduleEvent(memory_freezer_ns, event); } Freezer::~Freezer() { - core_timing.UnscheduleEvent(event, 0); + core_timing.UnscheduleEvent(event); } void Freezer::SetActive(bool is_active) { @@ -159,7 +158,7 @@ Freezer::Entries::const_iterator Freezer::FindEntry(VAddr address) const { [address](const Entry& entry) { return entry.address == address; }); } -void Freezer::FrameCallback(std::uintptr_t, std::chrono::nanoseconds ns_late) { +void Freezer::FrameCallback(std::chrono::nanoseconds ns_late) { if (!IsActive()) { LOG_DEBUG(Common_Memory, "Memory freezer has been deactivated, ending callback events."); return; diff --git a/src/core/tools/freezer.h b/src/core/tools/freezer.h index 0d6df5217e..2efbc11f38 100644 --- a/src/core/tools/freezer.h +++ b/src/core/tools/freezer.h @@ -77,7 +77,7 @@ private: Entries::iterator FindEntry(VAddr address); Entries::const_iterator FindEntry(VAddr address) const; - void FrameCallback(std::uintptr_t user_data, std::chrono::nanoseconds ns_late); + void FrameCallback(std::chrono::nanoseconds ns_late); void FillEntryReads(); std::atomic_bool active{false}; diff --git a/src/tests/core/core_timing.cpp b/src/tests/core/core_timing.cpp index f08afbf9ab..81898a1d38 100644 --- a/src/tests/core/core_timing.cpp +++ b/src/tests/core/core_timing.cpp @@ -16,20 +16,16 @@ namespace { // Numbers are chosen randomly to make sure the correct one is given. -constexpr std::array CB_IDS{{42, 144, 93, 1026, UINT64_C(0xFFFF7FFFF7FFFF)}}; constexpr std::array calls_order{{2, 0, 1, 4, 3}}; std::array delays{}; - -std::bitset callbacks_ran_flags; +std::bitset<5> callbacks_ran_flags; u64 expected_callback = 0; template -std::optional HostCallbackTemplate(std::uintptr_t user_data, s64 time, +std::optional HostCallbackTemplate(s64 time, std::chrono::nanoseconds ns_late) { - static_assert(IDX < CB_IDS.size(), "IDX out of range"); + static_assert(IDX < callbacks_ran_flags.size(), "IDX out of range"); callbacks_ran_flags.set(IDX); - REQUIRE(CB_IDS[IDX] == user_data); - REQUIRE(CB_IDS[IDX] == CB_IDS[calls_order[expected_callback]]); delays[IDX] = ns_late.count(); ++expected_callback; return std::nullopt; @@ -76,7 +72,7 @@ TEST_CASE("CoreTiming[BasicOrder]", "[core]") { const u64 order = calls_order[i]; const auto future_ns = std::chrono::nanoseconds{static_cast(i * one_micro + 100)}; - core_timing.ScheduleEvent(future_ns, events[order], CB_IDS[order]); + core_timing.ScheduleEvent(future_ns, events[order]); } /// test pause REQUIRE(callbacks_ran_flags.none()); @@ -118,7 +114,7 @@ TEST_CASE("CoreTiming[BasicOrderNoPausing]", "[core]") { for (std::size_t i = 0; i < events.size(); i++) { const u64 order = calls_order[i]; const auto future_ns = std::chrono::nanoseconds{static_cast(i * one_micro + 100)}; - core_timing.ScheduleEvent(future_ns, events[order], CB_IDS[order]); + core_timing.ScheduleEvent(future_ns, events[order]); } const u64 end = core_timing.GetGlobalTimeNs().count(); From 575db0417278eef0c854900885734fd068b97430 Mon Sep 17 00:00:00 2001 From: Liam Date: Sat, 23 Dec 2023 14:06:41 -0500 Subject: [PATCH 2/4] core_timing: use static typing for no-wait unschedule --- src/core/core_timing.cpp | 5 +++-- src/core/core_timing.h | 12 +++++++----- src/core/hle/kernel/k_hardware_timer.cpp | 3 ++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/core/core_timing.cpp b/src/core/core_timing.cpp index 3b7b0aa45f..d08c007bbe 100644 --- a/src/core/core_timing.cpp +++ b/src/core/core_timing.cpp @@ -143,7 +143,8 @@ void CoreTiming::ScheduleLoopingEvent(std::chrono::nanoseconds start_time, event.Set(); } -void CoreTiming::UnscheduleEvent(const std::shared_ptr& event_type, bool wait) { +void CoreTiming::UnscheduleEvent(const std::shared_ptr& event_type, + UnscheduleEventType type) { { std::scoped_lock lk{basic_lock}; @@ -161,7 +162,7 @@ void CoreTiming::UnscheduleEvent(const std::shared_ptr& event_type, b } // Force any in-progress events to finish - if (wait) { + if (type == UnscheduleEventType::Wait) { std::scoped_lock lk{advance_lock}; } } diff --git a/src/core/core_timing.h b/src/core/core_timing.h index d86337cdc9..d8cd599ee7 100644 --- a/src/core/core_timing.h +++ b/src/core/core_timing.h @@ -35,6 +35,11 @@ struct EventType { const std::string name; }; +enum class UnscheduleEventType { + Wait, + NoWait, +}; + /** * This is a system to schedule events into the emulated machine's future. Time is measured * in main CPU clock cycles. @@ -98,11 +103,8 @@ public: const std::shared_ptr& event_type, bool absolute_time = false); - void UnscheduleEvent(const std::shared_ptr& event_type, bool wait = true); - - void UnscheduleEventWithoutWait(const std::shared_ptr& event_type) { - UnscheduleEvent(event_type, false); - } + void UnscheduleEvent(const std::shared_ptr& event_type, + UnscheduleEventType type = UnscheduleEventType::Wait); void AddTicks(u64 ticks_to_add); diff --git a/src/core/hle/kernel/k_hardware_timer.cpp b/src/core/hle/kernel/k_hardware_timer.cpp index 2a29a487c1..4e947dd6bc 100644 --- a/src/core/hle/kernel/k_hardware_timer.cpp +++ b/src/core/hle/kernel/k_hardware_timer.cpp @@ -61,7 +61,8 @@ void KHardwareTimer::EnableInterrupt(s64 wakeup_time) { } void KHardwareTimer::DisableInterrupt() { - m_kernel.System().CoreTiming().UnscheduleEventWithoutWait(m_event_type); + m_kernel.System().CoreTiming().UnscheduleEvent(m_event_type, + Core::Timing::UnscheduleEventType::NoWait); m_wakeup_time = std::numeric_limits::max(); } From 05eda56e668f3a04c1f2600cd1a887ecd6ee1961 Mon Sep 17 00:00:00 2001 From: Liam Date: Sat, 23 Dec 2023 14:20:10 -0500 Subject: [PATCH 3/4] core_timing: handle concurrent unscheduling of looping events --- src/core/core_timing.cpp | 14 +++++++++++--- src/core/core_timing.h | 5 ++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/core/core_timing.cpp b/src/core/core_timing.cpp index d08c007bbe..c85590d4c5 100644 --- a/src/core/core_timing.cpp +++ b/src/core/core_timing.cpp @@ -159,6 +159,8 @@ void CoreTiming::UnscheduleEvent(const std::shared_ptr& event_type, for (auto h : to_remove) { event_queue.erase(h); } + + event_type->sequence_number++; } // Force any in-progress events to finish @@ -202,9 +204,10 @@ std::optional CoreTiming::Advance() { const Event& evt = event_queue.top(); if (const auto event_type{evt.type.lock()}) { - if (evt.reschedule_time == 0) { - const auto evt_time = evt.time; + const auto evt_time = evt.time; + const auto evt_sequence_num = event_type->sequence_number; + if (evt.reschedule_time == 0) { event_queue.pop(); basic_lock.unlock(); @@ -217,10 +220,15 @@ std::optional CoreTiming::Advance() { basic_lock.unlock(); const auto new_schedule_time{event_type->callback( - evt.time, std::chrono::nanoseconds{GetGlobalTimeNs().count() - evt.time})}; + evt_time, std::chrono::nanoseconds{GetGlobalTimeNs().count() - evt_time})}; basic_lock.lock(); + if (evt_sequence_num != event_type->sequence_number) { + // Heap handle is invalidated after external modification. + continue; + } + const auto next_schedule_time{new_schedule_time.has_value() ? new_schedule_time.value().count() : evt.reschedule_time}; diff --git a/src/core/core_timing.h b/src/core/core_timing.h index d8cd599ee7..7e4dff7f3d 100644 --- a/src/core/core_timing.h +++ b/src/core/core_timing.h @@ -27,12 +27,15 @@ using TimedCallback = std::function( /// Contains the characteristics of a particular event. struct EventType { explicit EventType(TimedCallback&& callback_, std::string&& name_) - : callback{std::move(callback_)}, name{std::move(name_)} {} + : callback{std::move(callback_)}, name{std::move(name_)}, sequence_number{0} {} /// The event's callback function. TimedCallback callback; /// A pointer to the name of the event. const std::string name; + /// A monotonic sequence number, incremented when this event is + /// changed externally. + size_t sequence_number; }; enum class UnscheduleEventType { From 3a4e7d45f1b2dfc1a47c00679e99c2034861e73a Mon Sep 17 00:00:00 2001 From: Liam Date: Sat, 23 Dec 2023 14:25:51 -0500 Subject: [PATCH 4/4] core_timing: block advance thread while clearing and signal after --- src/core/core_timing.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/core_timing.cpp b/src/core/core_timing.cpp index c85590d4c5..fc536413b7 100644 --- a/src/core/core_timing.cpp +++ b/src/core/core_timing.cpp @@ -72,8 +72,9 @@ void CoreTiming::Initialize(std::function&& on_thread_init_) { } void CoreTiming::ClearPendingEvents() { - std::scoped_lock lock{basic_lock}; + std::scoped_lock lock{advance_lock, basic_lock}; event_queue.clear(); + event.Set(); } void CoreTiming::Pause(bool is_paused) {