From 00eb1ee29a371f98472eda582aa0fd61d361d0ae Mon Sep 17 00:00:00 2001 From: Andrew Cox Date: Mon, 13 Jul 2015 18:26:04 +0100 Subject: [PATCH] Remove boost threading from resource loading Pushing for review. Unit test updates still to come. Change-Id: I0e0ac78954b91cec13491c82a70cec19a8fc8494 Signed-off-by: Andrew Cox --- adaptors/base/conditional-wait.cpp | 37 +++++++++++++-- adaptors/base/conditional-wait.h | 53 +++++++++++++++++++-- .../tizen/resource-loader/resource-thread-base.cpp | 54 ++++++++++++---------- .../tizen/resource-loader/resource-thread-base.h | 38 +++++++-------- .../resource-loader/resource-thread-image.cpp | 7 --- .../tizen/resource-loader/resource-thread-image.h | 5 -- 6 files changed, 129 insertions(+), 65 deletions(-) diff --git a/adaptors/base/conditional-wait.cpp b/adaptors/base/conditional-wait.cpp index 3b7e491..7f90d1c 100644 --- a/adaptors/base/conditional-wait.cpp +++ b/adaptors/base/conditional-wait.cpp @@ -20,6 +20,7 @@ // EXTERNAL INCLUDES #include +#include namespace Dali { @@ -30,10 +31,6 @@ namespace Internal namespace Adaptor { -namespace -{ -} // unnamed namespace - struct ConditionalWait::ConditionalWaitImpl { pthread_mutex_t mutex; @@ -92,6 +89,38 @@ unsigned int ConditionalWait::GetWaitCount() const return mImpl->count; } +ConditionalWait::ScopedLock::ScopedLock( ConditionalWait& wait ) : mWait(wait) +{ + pthread_mutex_lock( &wait.mImpl->mutex ); +} + +ConditionalWait::ScopedLock::~ScopedLock() +{ + ConditionalWait& wait = mWait; + pthread_mutex_unlock( &wait.mImpl->mutex ); +} + +void ConditionalWait::Wait( const ScopedLock& scope ) +{ + // Scope must be locked: + DALI_ASSERT_DEBUG( &scope.GetLockedWait() == this ); + + ++(mImpl->count); + + // pthread_cond_wait may wake up without anyone calling Notify so loop until + // count has been reset in a notify: + do + { + // wait while condition changes + pthread_cond_wait( &mImpl->condition, &mImpl->mutex ); // releases the lock whilst waiting + } + while( 0 != mImpl->count ); + + // We return with our mutex locked safe in the knowledge that the ScopedLock + // passed in will unlock it in the caller. +} + + } // namespace Adaptor } // namespace Internal diff --git a/adaptors/base/conditional-wait.h b/adaptors/base/conditional-wait.h index c6b2b5d..3aa537f 100644 --- a/adaptors/base/conditional-wait.h +++ b/adaptors/base/conditional-wait.h @@ -28,13 +28,48 @@ namespace Adaptor { /** - * Helper class to allow conditional waiting and notifications between multiple threads + * Helper class to allow conditional waiting and notifications between multiple threads. */ class ConditionalWait { public: /** + * @brief Allows client code to synchronize updates to its own state with the + * internal state of a ConditionalWait object. + */ + class ScopedLock + { + public: + /** + * Constructor + * @brief Will acquire the internal mutex of the ConditionalWait object passed in. + * @param[in] wait The ConditionalWait object to lock. + */ + ScopedLock( ConditionalWait& wait ); + + /** + * Destructor + * @brief Will release the internal mutex of the ConditionalWait object passed in. + */ + ~ScopedLock(); + + /** + * Getter for the ConditionalWait locked for this instance's lifetime. + * @return the ConditionalWait object currently locked. + */ + ConditionalWait& GetLockedWait() const { return mWait; } + + private: + + // Not implemented as ScopedLock cannot be copied: + ScopedLock( const ScopedLock& ); + const ScopedLock& operator=( const ScopedLock& ); + + ConditionalWait& mWait; + }; + + /** * @brief Constructor, creates the internal synchronization objects */ ConditionalWait(); @@ -60,6 +95,18 @@ public: void Wait(); /** + * @brief Wait for another thread to notify us when the condition is true and we can continue + * + * Will always block current thread until Notify is called. + * Assumes that the ScopedLock object passed in has already locked the internal state of + * this object. Releases the lock while waiting and re-acquires it when returning + * from the wait. + * param[in] scope A preexisting lock on the internal state of this object. + * @pre scope must have been passed this ConditionalWait during its construction. + */ + void Wait( const ScopedLock& scope ); + + /** * @brief Return the count of threads waiting for this conditional * @return count of waits */ @@ -67,9 +114,9 @@ public: private: - /// Not implemented as ConditionalWait is not copyable + // Not implemented as ConditionalWait is not copyable ConditionalWait( const ConditionalWait& ); - const ConditionalWait& operator= ( const ConditionalWait& ); + const ConditionalWait& operator=( const ConditionalWait& ); struct ConditionalWaitImpl; ConditionalWaitImpl* mImpl; diff --git a/platform-abstractions/tizen/resource-loader/resource-thread-base.cpp b/platform-abstractions/tizen/resource-loader/resource-thread-base.cpp index e3f101f..ffa029f 100644 --- a/platform-abstractions/tizen/resource-loader/resource-thread-base.cpp +++ b/platform-abstractions/tizen/resource-loader/resource-thread-base.cpp @@ -15,14 +15,16 @@ * */ +// EXTERNAL INCLUDES +#include #include + +// INTERNAL INCLUDES #include "resource-thread-base.h" #include "tizen-logging.h" #include "atomics.h" using namespace Dali::Integration; -using boost::mutex; -using boost::unique_lock; namespace Dali { @@ -35,10 +37,11 @@ const Integration::ResourceId NO_REQUEST_CANCELLED = Integration::ResourceId(0) namespace TizenPlatform { +using Internal::Adaptor::ConditionalWait; namespace { -const char * const IDLE_PRIORITY_ENVIRONMENT_VARIABLE_NAME = "DALI_RESOURCE_THREAD_IDLE_PRIORITY"; ///@Todo Move this to somewhere that other environment variables are declared and document it there. +const char * const IDLE_PRIORITY_ENVIRONMENT_VARIABLE_NAME = "DALI_RESOURCE_THREAD_IDLE_PRIORITY"; } // unnamed namespace /** Thrown by InterruptionPoint() to abort a request early. */ @@ -46,6 +49,7 @@ class CancelRequestException {}; ResourceThreadBase::ResourceThreadBase( ResourceLoader& resourceLoader ) : mResourceLoader( resourceLoader ), + mThread( 0 ), mCurrentRequestId( NO_REQUEST_IN_FLIGHT ), mCancelRequestId( NO_REQUEST_CANCELLED ), mPaused( false ) @@ -54,7 +58,8 @@ ResourceThreadBase::ResourceThreadBase( ResourceLoader& resourceLoader ) : mLogFilter = Debug::Filter::New(Debug::Concise, false, "LOG_RESOURCE_THREAD_BASE"); #endif - mThread = new boost::thread(boost::bind(&ResourceThreadBase::ThreadLoop, this)); + int error = pthread_create( &mThread, NULL, InternalThreadEntryFunc, this ); + DALI_ASSERT_ALWAYS( !error && "Error in pthread_create()" ); } ResourceThreadBase::~ResourceThreadBase() @@ -71,13 +76,12 @@ void ResourceThreadBase::TerminateThread() if (mThread) { // wake thread - mCondition.notify_all(); + mCondition.Notify(); + // wait for thread to exit - mThread->join(); - // delete thread instance - delete mThread; - // mark thread terminated - mThread = NULL; + pthread_join( mThread, NULL ); + + mThread = 0; } } @@ -88,7 +92,7 @@ void ResourceThreadBase::AddRequest(const ResourceRequest& request, const Reques { // Lock while adding to the request queue - unique_lock lock( mMutex ); + ConditionalWait::ScopedLock lock( mCondition ); wasEmpty = mQueue.empty(); wasPaused = mPaused; @@ -99,7 +103,7 @@ void ResourceThreadBase::AddRequest(const ResourceRequest& request, const Reques if( wasEmpty && !wasPaused ) { // Wake-up the thread - mCondition.notify_all(); + mCondition.Notify(); } } @@ -112,7 +116,7 @@ void ResourceThreadBase::CancelRequest( const Integration::ResourceId resourceId // Eliminate the cancelled request from the request queue if it is in there: { // Lock while searching and removing from the request queue: - unique_lock lock( mMutex ); + ConditionalWait::ScopedLock lock( mCondition ); for( RequestQueueIter iterator = mQueue.begin(); iterator != mQueue.end(); @@ -149,9 +153,15 @@ void ResourceThreadBase::InterruptionPoint() const } } +void* ResourceThreadBase::InternalThreadEntryFunc( void* This ) +{ + ( static_cast( This ) )->ThreadLoop(); + return NULL; +} + void ResourceThreadBase::Pause() { - unique_lock lock( mMutex ); + ConditionalWait::ScopedLock lock( mCondition ); mPaused = true; } @@ -160,7 +170,7 @@ void ResourceThreadBase::Resume() // Clear the paused flag and if we weren't running already, also wake up the background thread: bool wasPaused = false; { - unique_lock lock( mMutex ); + ConditionalWait::ScopedLock lock( mCondition ); wasPaused = mPaused; mPaused = false; } @@ -169,7 +179,7 @@ void ResourceThreadBase::Resume() // chance to do some work: if( wasPaused ) { - mCondition.notify_all(); + mCondition.Notify(); } } @@ -231,13 +241,13 @@ void ResourceThreadBase::ThreadLoop() void ResourceThreadBase::WaitForRequests() { - unique_lock lock( mMutex ); + ConditionalWait::ScopedLock lock( mCondition ); if( mQueue.empty() || mPaused == true ) { // Waiting for a wake up from resource loader control thread // This will be to process a new request or terminate - mCondition.wait(lock); + mCondition.Wait( lock ); } } @@ -248,7 +258,7 @@ void ResourceThreadBase::ProcessNextRequest() { // lock the queue and extract the next request - unique_lock lock(mMutex); + ConditionalWait::ScopedLock lock( mCondition ); if (!mQueue.empty()) { @@ -283,12 +293,6 @@ void ResourceThreadBase::ProcessNextRequest() Decode(*request); } break; - - case RequestSave: - { - Save(*request); - } - break; } } } diff --git a/platform-abstractions/tizen/resource-loader/resource-thread-base.h b/platform-abstractions/tizen/resource-loader/resource-thread-base.h index b4499fb..2f0ff50 100644 --- a/platform-abstractions/tizen/resource-loader/resource-thread-base.h +++ b/platform-abstractions/tizen/resource-loader/resource-thread-base.h @@ -2,7 +2,7 @@ #define __DALI_TIZEN_PLATFORM_RESOURCE_THREAD_BASE_H__ /* - * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * Copyright (c) 2015 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,12 +21,10 @@ // INTERNAL INCLUDES #include "resource-loader.h" #include "resource-loading-client.h" -#include -#include +#include // EXTERNAL INCLUDES #include -#include namespace Dali { @@ -51,9 +49,7 @@ public: /** Pull a resource from the network. */ RequestDownload, /** Pull a resource out of a memory buffer. */ - RequestDecode, - /** Push a resource's data out to the file system. */ - RequestSave + RequestDecode }; typedef std::pair RequestInfo; @@ -142,28 +138,28 @@ protected: virtual void Decode(const Integration::ResourceRequest& request); /** - * Save a resource - * @param[in] request The requested resource/file url and attributes - */ - virtual void Save(const Integration::ResourceRequest& request) = 0; - - /** * @brief Cancels current resource request if it matches the one latched to be cancelled. * * @copydoc ResourceLoadingClient::InterruptionPoint */ virtual void InterruptionPoint() const; +private: + /** + * Helper for the thread calling the entry function + * @param[in] This A pointer to the current UpdateThread object + */ + static void* InternalThreadEntryFunc( void* This ); + protected: - ResourceLoader& mResourceLoader; - boost::thread* mThread; ///< thread instance - boost::condition_variable mCondition; ///< condition variable - boost::mutex mMutex; ///< used to protect mQueue - RequestQueue mQueue; ///< Request queue + ResourceLoader& mResourceLoader; + pthread_t mThread; ///< thread instance + Internal::Adaptor::ConditionalWait mCondition; ///< condition variable + RequestQueue mQueue; ///< Request queue private: - Integration::ResourceId mCurrentRequestId; ///< Current request, set by worker thread - volatile Integration::ResourceId mCancelRequestId; ///< Request to be cancelled on thread: written by external thread and read by worker. - bool mPaused; ///< Whether to process work in mQueue + Integration::ResourceId mCurrentRequestId; ///< Current request, set by worker thread + volatile Integration::ResourceId mCancelRequestId; ///< Request to be cancelled on thread: written by external thread and read by worker. + bool mPaused; ///< Whether to process work in mQueue #if defined(DEBUG_ENABLED) public: diff --git a/platform-abstractions/tizen/resource-loader/resource-thread-image.cpp b/platform-abstractions/tizen/resource-loader/resource-thread-image.cpp index 5838721..02886c0 100755 --- a/platform-abstractions/tizen/resource-loader/resource-thread-image.cpp +++ b/platform-abstractions/tizen/resource-loader/resource-thread-image.cpp @@ -102,13 +102,6 @@ void ResourceThreadImage::Decode(const ResourceRequest& request) } } -void ResourceThreadImage::Save(const Integration::ResourceRequest& request) -{ - DALI_LOG_TRACE_METHOD( mLogFilter ); - DALI_ASSERT_DEBUG( request.GetType()->id == ResourceBitmap ); - DALI_LOG_WARNING( "Image saving not supported on background resource threads." ); -} - bool ResourceThreadImage::DownloadRemoteImageIntoMemory(const Integration::ResourceRequest& request, Dali::Vector& dataBuffer, size_t& dataSize) { bool ok = Network::DownloadRemoteFileIntoMemory( request.GetPath(), dataBuffer, dataSize, MAXIMUM_DOWNLOAD_IMAGE_SIZE ); diff --git a/platform-abstractions/tizen/resource-loader/resource-thread-image.h b/platform-abstractions/tizen/resource-loader/resource-thread-image.h index 91d5188..6d9a009 100644 --- a/platform-abstractions/tizen/resource-loader/resource-thread-image.h +++ b/platform-abstractions/tizen/resource-loader/resource-thread-image.h @@ -62,11 +62,6 @@ private: virtual void Decode(const Integration::ResourceRequest& request); /** - *@copydoc ResourceThreadBase::Save - */ - virtual void Save(const Integration::ResourceRequest& request); - - /** * Download a requested image into a memory buffer. * @param[in] request The requested resource/file url and attributes * @param[out] dataBuffer A memory buffer object to be written with downloaded image data. -- 2.7.4