From 286ae14a7bb8828e13e89753b3244ffc6dc0d6c0 Mon Sep 17 00:00:00 2001 From: Reece Date: Tue, 17 May 2022 00:41:27 +0100 Subject: [PATCH] [*] Refactor WorkItemHandler EProcessNext -> ETickType [*] AuAsync aue 1 regression --- Include/Aurora/Async/IWorkItemHandler.hpp | 30 +++++---- Include/Aurora/Async/OldTrash.hpp | 8 +-- Include/Aurora/Async/WorkPairImpl.hpp | 2 +- Source/Async/WorkItem.cpp | 75 +++++++++++++++-------- Source/Async/WorkItem.hpp | 5 ++ 5 files changed, 73 insertions(+), 47 deletions(-) diff --git a/Include/Aurora/Async/IWorkItemHandler.hpp b/Include/Aurora/Async/IWorkItemHandler.hpp index dd2da2f1..205283a7 100644 --- a/Include/Aurora/Async/IWorkItemHandler.hpp +++ b/Include/Aurora/Async/IWorkItemHandler.hpp @@ -9,25 +9,24 @@ namespace Aurora::Async { + AUE_DEFINE(ETickType, + ( + eFinished, + eRerun, + eSchedule, + eFailed + )); + struct IWorkItemHandler { - enum class EProcessNext - { - eInvalid = -1, - eFinished = 0, - eRerun, - eSchedule, - eFailed - }; - struct ProcessInfo { - ProcessInfo(bool finished) : type(finished ? EProcessNext::eFinished : EProcessNext::eFailed) {} - ProcessInfo(EProcessNext type) : type(type) {} - ProcessInfo(const AuList> &blockedBy) : type(EProcessNext::eSchedule), waitFor(blockedBy) {} + ProcessInfo(bool finished) : type(finished ? ETickType::eFinished : ETickType::eFailed) {} + ProcessInfo(ETickType type) : type(type) {} + ProcessInfo(const AuList> &blockedBy) : type(ETickType::eSchedule), waitFor(blockedBy) {} // ... - EProcessNext type; + ETickType type; AuList> waitFor; AuUInt32 reschedMs {}; AuUInt64 reschedNs {}; @@ -40,9 +39,8 @@ namespace Aurora::Async virtual void DispatchFrame(ProcessInfo &info) = 0; /// A really terrible name for the overloadable method that serves as the critical failure callback - /// You have a 'shutdown'/free function you can overload, it's called the dtor - /// Don't moan about the shitty naming of this, im not refactoring it - virtual void Shutdown() = 0; + /// This may run from any thread + virtual void OnFailure() {}; virtual void *GetPrivateData() { return nullptr; } }; diff --git a/Include/Aurora/Async/OldTrash.hpp b/Include/Aurora/Async/OldTrash.hpp index f346bc3f..da4f8644 100644 --- a/Include/Aurora/Async/OldTrash.hpp +++ b/Include/Aurora/Async/OldTrash.hpp @@ -22,7 +22,7 @@ namespace Aurora::Async private: #if !defined(_CPPSHARP) - void DispatchFrame(ProcessInfo &info) override + inline void DispatchFrame(ProcessInfo &info) override { try { @@ -34,7 +34,7 @@ namespace Aurora::Async } } - void Shutdown() override + inline void OnFailure() override { try { @@ -66,12 +66,12 @@ namespace Aurora::Async { if (!frame) { - info.type = IWorkItemHandler::EProcessNext::eFinished; + info.type = IWorkItemHandler::ETickType::eFinished; return; } } frame(); - info.type = IWorkItemHandler::EProcessNext::eFinished; + info.type = IWorkItemHandler::ETickType::eFinished; } void Shutdown() override diff --git a/Include/Aurora/Async/WorkPairImpl.hpp b/Include/Aurora/Async/WorkPairImpl.hpp index aa53d84d..4a4e17d5 100644 --- a/Include/Aurora/Async/WorkPairImpl.hpp +++ b/Include/Aurora/Async/WorkPairImpl.hpp @@ -175,7 +175,7 @@ namespace Aurora::Async } } - void Shutdown() override + void OnFailure() override { AU_LOCK_GUARD(this->lock_); ShutdownNoLock(); diff --git a/Source/Async/WorkItem.cpp b/Source/Async/WorkItem.cpp index 27fdb995..bbfee961 100644 --- a/Source/Async/WorkItem.cpp +++ b/Source/Async/WorkItem.cpp @@ -54,29 +54,43 @@ namespace Aurora::Async return AU_SHARED_FROM_THIS; } + bool WorkItem::WaitForLocked(const AuList> &workItems) + { + for (auto &workItem : workItems) + { + auto dependency = AuReinterpretCast(workItem); + AU_LOCK_GUARD(dependency->lock); + + if (dependency->HasFailed()) + { + return false; + } + + if (!AuTryInsert(dependency->waiters_, AuSharedFromThis())) + { + return false; + } + + if (!AuTryInsert(this->waitOn_, workItem)) + { + AuTryRemove(dependency->waiters_, AuSharedFromThis()); + return false; + } + } + + return true; + } + AuSPtr WorkItem::WaitFor(const AuList> &workItems) { bool status {}; { AU_LOCK_GUARD(this->lock); - - for (auto &workItem : workItems) - { - auto dependency = AuReinterpretCast(workItem); - AU_LOCK_GUARD(dependency->lock); - - if (dependency->HasFailed()) - { - status = true; - } - - dependency->waiters_.push_back(shared_from_this()); - this->waitOn_.push_back(workItem); - } + status = WaitForLocked(workItems); } - if (status) + if (!status) { Fail(); } @@ -100,7 +114,7 @@ namespace Aurora::Async AuSPtr WorkItem::SetSchedTimeAbs(AuUInt32 ms) { - this->dispatchTimeNs_ = AuUInt64(ms) * AuUInt64(1000000); + this->dispatchTimeNs_ = AuUInt64(ms) * AuMSToNS(ms); return AU_SHARED_FROM_THIS; } @@ -112,13 +126,13 @@ namespace Aurora::Async AuSPtr WorkItem::SetSchedTime(AuUInt32 ms) { - this->dispatchTimeNs_ = Time::CurrentClockNS() + (AuUInt64(ms) * AuUInt64(1000000)); + this->dispatchTimeNs_ = Time::CurrentClockNS() + AuMSToNS(ms); return AU_SHARED_FROM_THIS; } AuSPtr WorkItem::AddDelayTime(AuUInt32 ms) { - this->delayTimeNs_ += AuUInt64(ms) * AuUInt64(1000000); + this->delayTimeNs_ += AuUInt64(ms) * AuMSToNS(ms); return AU_SHARED_FROM_THIS; } @@ -138,6 +152,11 @@ namespace Aurora::Async { AU_LOCK_GUARD(lock); + DispatchExLocked(check); + } + + void WorkItem::DispatchExLocked(bool check) + { if (check) { if (this->dispatchPending_) @@ -211,17 +230,17 @@ namespace Aurora::Async switch (info.type) { - case IWorkItemHandler::EProcessNext::eFinished: + case ETickType::eFinished: { // do nothing break; } - case IWorkItemHandler::EProcessNext::eInvalid: + case ETickType::eEnumInvalid: { SysPanic("Handle Invalid"); break; } - case IWorkItemHandler::EProcessNext::eSchedule: + case ETickType::eSchedule: { if (info.reschedMs) { @@ -239,16 +258,20 @@ namespace Aurora::Async { SetSchedTimeNsAbs(info.reschedNs); } - WaitFor(info.waitFor); + + if (!WaitForLocked(info.waitFor)) + { + Fail(); + } } [[fallthrough]]; - case IWorkItemHandler::EProcessNext::eRerun: + case ETickType::eRerun: { - DispatchEx(false); + DispatchExLocked(false); return; } - case IWorkItemHandler::EProcessNext::eFailed: + case ETickType::eFailed: { Fail(); return; @@ -273,7 +296,7 @@ namespace Aurora::Async if (auto task_ = AuExchange(this->task_, {})) { - task_->Shutdown(); + task_->OnFailure(); } for (auto &waiter : this->waiters_) diff --git a/Source/Async/WorkItem.hpp b/Source/Async/WorkItem.hpp index 2f2cac25..5c74c093 100644 --- a/Source/Async/WorkItem.hpp +++ b/Source/Async/WorkItem.hpp @@ -46,7 +46,12 @@ namespace Aurora::Async void SetPrio(float val) override; private: + + bool WaitForLocked(const AuList> &workItem); + void DispatchEx(bool check); + void DispatchExLocked(bool check); + AuSPtr task_; WorkerId_t worker_; float prio_ = 0.5f;