From c89821e2838bdbb35bd287350bc91f42fa22758e Mon Sep 17 00:00:00 2001 From: "Eunki, Hong" Date: Fri, 26 Jul 2024 14:08:12 +0900 Subject: [PATCH] Lock mutex when animation task check + completed task moving Since VectorAnimationThread::OnTaskCompleted() could be called from various worker threads, we should not change the thread's member value. To avoid this kind of thread issue, - Make mutex during animation tasks control (VectorAnimationThread vs EventThread) - Collect tasks from worker thread and move them at Rasterize() API (VectorAnimationThread vs WorkerThread) Change-Id: Ib3dce6b98f6c66a3bd597a7b46fe8bbac60105b5 Signed-off-by: Eunki, Hong --- .../vector-animation-thread.cpp | 296 ++++++++++++++------- .../vector-animation-thread.h | 24 +- 2 files changed, 215 insertions(+), 105 deletions(-) diff --git a/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.cpp b/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.cpp index 5638ec3..a4992d9 100644 --- a/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.cpp +++ b/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include namespace Dali @@ -37,6 +38,8 @@ namespace Debug::Filter* gVectorAnimationLogFilter = Debug::Filter::New(Debug::NoLogging, false, "LOG_VECTOR_ANIMATION"); #endif +DALI_INIT_TRACE_FILTER(gTraceFilter, DALI_TRACE_IMAGE_PERFORMANCE_MARKER, false); + } // unnamed namespace VectorAnimationThread::VectorAnimationThread() @@ -46,6 +49,8 @@ VectorAnimationThread::VectorAnimationThread() mSleepThread(MakeCallback(this, &VectorAnimationThread::OnAwakeFromSleep)), mConditionalWait(), mEventTriggerMutex(), + mAnimationTasksMutex(), + mTaskCompletedMutex(), mLogFactory(Dali::Adaptor::Get().GetLogFactory()), mTraceFactory(Dali::Adaptor::Get().GetTraceFactory()), mNeedToSleep(false), @@ -59,14 +64,20 @@ VectorAnimationThread::VectorAnimationThread() mEventTrigger = std::unique_ptr(new EventThreadCallback(MakeCallback(this, &VectorAnimationThread::OnEventCallbackTriggered))); } +/// Event thread called VectorAnimationThread::~VectorAnimationThread() { + DALI_TRACE_SCOPE(gTraceFilter, "VECTOR_ANIMATION_THREAD_DESTROY"); + // Stop the thread { ConditionalWait::ScopedLock lock(mConditionalWait); // Wait until some event thread trigger relative job finished. { - Mutex::ScopedLock lock(mEventTriggerMutex); + Mutex::ScopedLock eventTriggerLock(mEventTriggerMutex); + Mutex::ScopedLock taskCompletedLock(mTaskCompletedMutex); + Mutex::ScopedLock animationTasksLock(mAnimationTasksMutex); + DALI_LOG_DEBUG_INFO("Mark VectorAnimationThread destroyed\n"); mDestroyThread = true; } mNeedToSleep = false; @@ -78,82 +89,46 @@ VectorAnimationThread::~VectorAnimationThread() DALI_LOG_INFO(gVectorAnimationLogFilter, Debug::Verbose, "VectorAnimationThread::~VectorAnimationThread: Join [%p]\n", this); + DALI_LOG_DEBUG_INFO("VectorAnimationThread Join request\n"); + Join(); } +/// Event thread called void VectorAnimationThread::AddTask(VectorAnimationTaskPtr task) { ConditionalWait::ScopedLock lock(mConditionalWait); - // Find if the task is already in the list except loading task - auto iter = std::find_if(mAnimationTasks.begin(), mAnimationTasks.end(), [task](VectorAnimationTaskPtr& element) { return (element == task && !element->IsLoadRequested()); }); - if(iter == mAnimationTasks.end()) + // Rasterize as soon as possible + if(MoveTasksToAnimation(task, true)) { - auto currentTime = task->CalculateNextFrameTime(true); // Rasterize as soon as possible - - bool inserted = false; - for(auto iter = mAnimationTasks.begin(); iter != mAnimationTasks.end(); ++iter) - { - auto nextTime = (*iter)->GetNextFrameTime(); - if(nextTime > currentTime) - { - mAnimationTasks.insert(iter, task); - inserted = true; - break; - } - } - - if(!inserted) - { - mAnimationTasks.push_back(task); - } - mNeedToSleep = false; // wake up the animation thread mConditionalWait.Notify(lock); } } +/// Worker thread called void VectorAnimationThread::OnTaskCompleted(VectorAnimationTaskPtr task, bool success, bool keepAnimation) { - if(!mDestroyThread) - { - ConditionalWait::ScopedLock lock(mConditionalWait); - bool needRasterize = false; + ConditionalWait::ScopedLock lock(mConditionalWait); - auto workingTask = std::find(mWorkingTasks.begin(), mWorkingTasks.end(), task); - if(workingTask != mWorkingTasks.end()) - { - mWorkingTasks.erase(workingTask); - } + Mutex::ScopedLock taskCompletedLock(mTaskCompletedMutex); - // Check pending task - if(mAnimationTasks.end() != std::find(mAnimationTasks.begin(), mAnimationTasks.end(), task)) - { - needRasterize = true; - } - - if(keepAnimation && success) - { - if(mCompletedTasks.end() == std::find(mCompletedTasks.begin(), mCompletedTasks.end(), task)) - { - mCompletedTasks.push_back(task); - needRasterize = true; - } - } + if(DALI_LIKELY(!mDestroyThread)) + { + mCompletedTasksQueue.emplace_back(task, success && keepAnimation); - if(needRasterize) - { - mNeedToSleep = false; - // wake up the animation thread - mConditionalWait.Notify(lock); - } + // wake up the animation thread. + // Note that we should not make mNeedToSleep as false now. + mConditionalWait.Notify(lock); } } +/// VectorAnimationThread::SleepThread called void VectorAnimationThread::OnAwakeFromSleep() { - if(!mDestroyThread) + if(DALI_LIKELY(!mDestroyThread)) { mNeedToSleep = false; // wake up the animation thread @@ -161,10 +136,11 @@ void VectorAnimationThread::OnAwakeFromSleep() } } +/// Worker thread called void VectorAnimationThread::AddEventTriggerCallback(CallbackBase* callback, uint32_t argument) { Mutex::ScopedLock lock(mEventTriggerMutex); - if(!mDestroyThread) + if(DALI_LIKELY(!mDestroyThread)) { mTriggerEventCallbacks.emplace_back(callback, argument); @@ -176,20 +152,22 @@ void VectorAnimationThread::AddEventTriggerCallback(CallbackBase* callback, uint } } +/// Event thread called void VectorAnimationThread::RemoveEventTriggerCallbacks(CallbackBase* callback) { Mutex::ScopedLock lock(mEventTriggerMutex); - if(!mDestroyThread) + if(DALI_LIKELY(!mDestroyThread)) { auto iter = std::remove_if(mTriggerEventCallbacks.begin(), mTriggerEventCallbacks.end(), [&callback](std::pair& item) { return item.first == callback; }); mTriggerEventCallbacks.erase(iter, mTriggerEventCallbacks.end()); } } +/// Worker thread called void VectorAnimationThread::RequestForceRenderOnce() { Mutex::ScopedLock lock(mEventTriggerMutex); - if(!mDestroyThread) + if(DALI_LIKELY(!mDestroyThread)) { mForceRenderOnce = true; @@ -201,6 +179,7 @@ void VectorAnimationThread::RequestForceRenderOnce() } } +/// VectorAnimationThread called void VectorAnimationThread::Run() { SetThreadName("VectorAnimationThread"); @@ -213,32 +192,34 @@ void VectorAnimationThread::Run() } } -void VectorAnimationThread::Rasterize() +/// Event & VectorAnimationThread called +bool VectorAnimationThread::MoveTasksToAnimation(VectorAnimationTaskPtr task, bool useCurrentTime) { - // Lock while popping task out from the queue - ConditionalWait::ScopedLock lock(mConditionalWait); + Mutex::ScopedLock animationTasksLock(mAnimationTasksMutex); - // conditional wait - if(mNeedToSleep) - { - mConditionalWait.Wait(lock); - } - - mNeedToSleep = true; - - // Process completed tasks - for(auto&& task : mCompletedTasks) + if(DALI_LIKELY(!mDestroyThread)) { - if(mAnimationTasks.end() == std::find(mAnimationTasks.begin(), mAnimationTasks.end(), task)) + DALI_TRACE_BEGIN_WITH_MESSAGE_GENERATOR(gTraceFilter, "VECTOR_ANIMATION_THREAD_ANIMATION_TASK", [&](std::ostringstream& oss) { + oss << "[" << mAnimationTasks.size() << "," << useCurrentTime << "]"; + }); + + // Find if the task is already in the list except loading task + auto iter = std::find_if(mAnimationTasks.begin(), mAnimationTasks.end(), [task, useCurrentTime](VectorAnimationTaskPtr& element) { + return (element == task) && + (!useCurrentTime || // If we don't need to use current time (i.e. CompletedTasks) + !element->IsLoadRequested()); // Or we need to use current time And loading completed. + }); + + if(iter == mAnimationTasks.end()) { - // Should use the frame rate of the animation file - auto nextFrameTime = task->CalculateNextFrameTime(false); + // Use the frame rate of the animation file, or use current time. + auto nextFrameTime = task->CalculateNextFrameTime(useCurrentTime); bool inserted = false; for(auto iter = mAnimationTasks.begin(); iter != mAnimationTasks.end(); ++iter) { - auto time = (*iter)->GetNextFrameTime(); - if(time > nextFrameTime) + auto nextTime = (*iter)->GetNextFrameTime(); + if(nextTime > nextFrameTime) { mAnimationTasks.insert(iter, task); inserted = true; @@ -250,47 +231,156 @@ void VectorAnimationThread::Rasterize() { mAnimationTasks.push_back(task); } + + DALI_TRACE_END_WITH_MESSAGE_GENERATOR(gTraceFilter, "VECTOR_ANIMATION_THREAD_ANIMATION_TASK", [&](std::ostringstream& oss) { + oss << "[" << mAnimationTasks.size() << "]"; + }); + + return true; } + DALI_TRACE_END(gTraceFilter, "VECTOR_ANIMATION_THREAD_ANIMATION_TASK"); } - mCompletedTasks.clear(); + return false; +} - // pop out the next task from the queue - for(auto it = mAnimationTasks.begin(); it != mAnimationTasks.end();) +/// VectorAnimationThread called +void VectorAnimationThread::MoveTasksToCompleted(VectorAnimationTaskPtr task, bool keepAnimation) +{ + if(DALI_LIKELY(!mDestroyThread)) { - VectorAnimationTaskPtr nextTask = *it; - - auto currentTime = std::chrono::steady_clock::now(); - auto nextFrameTime = nextTask->GetNextFrameTime(); + DALI_TRACE_BEGIN_WITH_MESSAGE_GENERATOR(gTraceFilter, "VECTOR_ANIMATION_THREAD_COMPLETED_TASK", [&](std::ostringstream& oss) { + oss << "[w:" << mWorkingTasks.size() << ",c:" << mCompletedTasks.size() << "]"; + }); + bool needRasterize = false; -#if defined(DEBUG_ENABLED) -// auto duration = std::chrono::duration_cast(nextFrameTime - currentTime); + auto workingTask = std::find(mWorkingTasks.begin(), mWorkingTasks.end(), task); + if(workingTask != mWorkingTasks.end()) + { + mWorkingTasks.erase(workingTask); + } -// DALI_LOG_INFO(gVectorAnimationLogFilter, Debug::Verbose, "VectorAnimationThread::Rasterize: [next time = %lld]\n", duration.count()); -#endif - if(nextFrameTime <= currentTime) + // Check pending task { - // If the task is not in the working list - if(std::find(mWorkingTasks.begin(), mWorkingTasks.end(), nextTask) == mWorkingTasks.end()) + Mutex::ScopedLock animationTasksLock(mAnimationTasksMutex); + if(mAnimationTasks.end() != std::find(mAnimationTasks.begin(), mAnimationTasks.end(), task)) { - it = mAnimationTasks.erase(it); + needRasterize = true; + } + } - // Add it to the working list - mWorkingTasks.push_back(nextTask); - mAsyncTaskManager.AddTask(nextTask); + if(keepAnimation) + { + if(mCompletedTasks.end() == std::find(mCompletedTasks.begin(), mCompletedTasks.end(), task)) + { + mCompletedTasks.push_back(task); + needRasterize = true; } - else + } + + if(needRasterize) + { + mNeedToSleep = false; + } + + DALI_TRACE_END_WITH_MESSAGE_GENERATOR(gTraceFilter, "VECTOR_ANIMATION_THREAD_COMPLETED_TASK", [&](std::ostringstream& oss) { + oss << "[w:" << mWorkingTasks.size() << ",c:" << mCompletedTasks.size() << ",r?" << needRasterize << ",s?" << mNeedToSleep << "]"; + }); + } +} + +/// VectorAnimationThread called +void VectorAnimationThread::Rasterize() +{ + // Lock while popping task out from the queue + ConditionalWait::ScopedLock lock(mConditionalWait); + + // conditional wait + while(DALI_LIKELY(!mDestroyThread) && mNeedToSleep) + { + // ConditionalWait notifyed when sleep done, or task complete. + // If task complete, then we need to re-validate the tasks container, and then sleep again. + decltype(mCompletedTasksQueue) completedTasksQueue; + { + Mutex::ScopedLock taskCompletedLock(mTaskCompletedMutex); + completedTasksQueue.swap(mCompletedTasksQueue); + } + if(completedTasksQueue.empty()) + { + mConditionalWait.Wait(lock); + // Task completed may have been added to the queue while we were waiting. { - it++; + Mutex::ScopedLock taskCompletedLock(mTaskCompletedMutex); + completedTasksQueue.swap(mCompletedTasksQueue); } } - else + + for(auto&& taskPair : completedTasksQueue) { - mSleepThread.SleepUntil(nextFrameTime); - break; + auto& task = taskPair.first; + bool keepAnimation = taskPair.second; + MoveTasksToCompleted(task, keepAnimation); + } + } + + mNeedToSleep = true; + + // Process completed tasks + for(auto&& task : mCompletedTasks) + { + // Should use the frame rate of the animation file + MoveTasksToAnimation(task, false); + } + mCompletedTasks.clear(); + + { + Mutex::ScopedLock animationTasksLock(mAnimationTasksMutex); + if(DALI_LIKELY(!mDestroyThread)) + { + if(mAnimationTasks.size() > 0u) + { + DALI_TRACE_BEGIN_WITH_MESSAGE_GENERATOR(gTraceFilter, "VECTOR_ANIMATION_THREAD_ANIMATION_TASK2", [&](std::ostringstream& oss) { + oss << "[" << mAnimationTasks.size() << "]"; + }); + + // pop out the next task from the queue + for(auto it = mAnimationTasks.begin(); it != mAnimationTasks.end();) + { + VectorAnimationTaskPtr nextTask = *it; + + auto currentTime = std::chrono::steady_clock::now(); + auto nextFrameTime = nextTask->GetNextFrameTime(); + + if(nextFrameTime <= currentTime) + { + // If the task is not in the working list + if(std::find(mWorkingTasks.begin(), mWorkingTasks.end(), nextTask) == mWorkingTasks.end()) + { + it = mAnimationTasks.erase(it); + + // Add it to the working list + mWorkingTasks.push_back(nextTask); + mAsyncTaskManager.AddTask(nextTask); + } + else + { + it++; + } + } + else + { + mSleepThread.SleepUntil(nextFrameTime); + break; + } + } + DALI_TRACE_END_WITH_MESSAGE_GENERATOR(gTraceFilter, "VECTOR_ANIMATION_THREAD_ANIMATION_TASK2", [&](std::ostringstream& oss) { + oss << "[" << mAnimationTasks.size() << "]"; + }); + } } } } +/// Event thread called (Due to mTrigger triggered) void VectorAnimationThread::OnEventCallbackTriggered() { while(true) @@ -316,6 +406,7 @@ void VectorAnimationThread::OnEventCallbackTriggered() } } +/// Event thread called std::pair VectorAnimationThread::GetNextEventCallback() { Mutex::ScopedLock lock(mEventTriggerMutex); @@ -346,16 +437,20 @@ VectorAnimationThread::SleepThread::SleepThread(CallbackBase* callback) VectorAnimationThread::SleepThread::~SleepThread() { + DALI_TRACE_SCOPE(gTraceFilter, "VECTOR_ANIMATION_SLEEP_THREAD_DESTROY"); + // Stop the thread { ConditionalWait::ScopedLock lock(mConditionalWait); mDestroyThread = true; + mAwakeCallback.reset(); mConditionalWait.Notify(lock); } Join(); } +/// VectorAnimationThread called void VectorAnimationThread::SleepThread::SleepUntil(std::chrono::time_point timeToSleepUntil) { ConditionalWait::ScopedLock lock(mConditionalWait); @@ -386,11 +481,7 @@ void VectorAnimationThread::SleepThread::Run() if(needToSleep) { -#if defined(DEBUG_ENABLED) -// auto sleepDuration = std::chrono::duration_cast(mSleepTimePoint - std::chrono::steady_clock::now()); - -// DALI_LOG_INFO(gVectorAnimationLogFilter, Debug::Verbose, "VectorAnimationThread::SleepThread::Run: [sleep duration = %lld]\n", sleepDuration.count()); -#endif + DALI_TRACE_SCOPE(gTraceFilter, "VECTOR_ANIMATION_SLEEP_THREAD"); std::this_thread::sleep_until(sleepTimePoint); @@ -404,6 +495,7 @@ void VectorAnimationThread::SleepThread::Run() ConditionalWait::ScopedLock lock(mConditionalWait); if(!mDestroyThread && !mNeedToSleep) { + DALI_TRACE_SCOPE(gTraceFilter, "VECTOR_ANIMATION_SLEEP_THREAD_WAIT"); mConditionalWait.Wait(lock); } } diff --git a/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.h b/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.h index 8b820cc..ac5053b 100644 --- a/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.h +++ b/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.h @@ -103,6 +103,18 @@ protected: private: /** + * @brief Move given tasks to mAnimationTasks if required. + * @return True if task added. False if not. + */ + bool MoveTasksToAnimation(VectorAnimationTaskPtr task, bool useCurrentTime); + + /** + * @brief Move given tasks to mCompletedTasks if required. + */ + void MoveTasksToCompleted(VectorAnimationTaskPtr task, bool keepAnimation); + +private: + /** * @brief Rasterizes the tasks. */ void Rasterize(); @@ -167,13 +179,19 @@ private: VectorAnimationThread& operator=(const VectorAnimationThread& thread) = delete; private: - std::vector mAnimationTasks; - std::vector mCompletedTasks; - std::vector mWorkingTasks; + std::vector mAnimationTasks; ///< Animation processing tasks, ordered by next frame time. + std::vector mCompletedTasks; ///< Temperal storage for completed tasks. + std::vector mWorkingTasks; + + std::vector> mCompletedTasksQueue; ///< Queue of completed tasks from worker thread. pair of task, and rasterize required. + ///< It will be moved at begin of Rasterize(). + std::vector> mTriggerEventCallbacks{}; // Callbacks are not owned SleepThread mSleepThread; ConditionalWait mConditionalWait; Mutex mEventTriggerMutex; + Mutex mAnimationTasksMutex; ///< Mutex to change + get mAnimationTasks from event thread + Mutex mTaskCompletedMutex; ///< Mutex to collect completed tasks to mCompletedTasksQueue from worker threads std::unique_ptr mEventTrigger{}; const Dali::LogFactoryInterface& mLogFactory; const Dali::TraceFactoryInterface& mTraceFactory; -- 2.7.4