From c659ac010b256880530f33e657876ffbee95b769 Mon Sep 17 00:00:00 2001 From: Kimmo Hoikka Date: Thu, 17 Aug 2017 14:51:33 +0100 Subject: [PATCH] Change Dali::Thread, Dali::Mutex and Dali::ConditionalWait to use std::thread from C++ 11 instead of pthread APIs Change-Id: I249a75bcf5bda7c4a747182cfb95710db244477f --- build/tizen/dali-core/Makefile.am | 3 +- dali/devel-api/threading/conditional-wait.cpp | 103 +++++++++------------ dali/devel-api/threading/conditional-wait.h | 11 ++- dali/devel-api/threading/mutex.cpp | 22 ++--- dali/devel-api/threading/mutex.h | 2 +- dali/devel-api/threading/thread.cpp | 42 +++++---- dali/devel-api/threading/thread.h | 8 +- .../common/{mutex-impl.cpp => mutex-trace.cpp} | 20 +--- .../common/{mutex-impl.h => mutex-trace.h} | 31 ++----- dali/internal/file.list | 2 +- packaging/dali.spec | 2 +- 11 files changed, 105 insertions(+), 141 deletions(-) rename dali/internal/common/{mutex-impl.cpp => mutex-trace.cpp} (89%) rename dali/internal/common/{mutex-impl.h => mutex-trace.h} (65%) diff --git a/build/tizen/dali-core/Makefile.am b/build/tizen/dali-core/Makefile.am index ad081d5..137b6f6 100644 --- a/build/tizen/dali-core/Makefile.am +++ b/build/tizen/dali-core/Makefile.am @@ -53,8 +53,7 @@ libdali_core_la_CXXFLAGS = -DDALI_COMPILATION \ $(dali_core_includes) \ $(DALI_CFLAGS) -libdali_core_la_LIBADD = $(DALI_LDFLAGS) \ - -lpthread +libdali_core_la_LIBADD = $(DALI_LDFLAGS) # Install headers under the correct subdirectories platformabstractiondir = $(includedir)/dali/integration-api diff --git a/dali/devel-api/threading/conditional-wait.cpp b/dali/devel-api/threading/conditional-wait.cpp index adc5760..fa0a86c 100644 --- a/dali/devel-api/threading/conditional-wait.cpp +++ b/dali/devel-api/threading/conditional-wait.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 Samsung Electronics Co., Ltd. + * Copyright (c) 2017 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. @@ -19,77 +19,78 @@ #include // EXTERNAL INCLUDES -#include +#include +#include // INTERNAL INCLUDES #include -#include +#include namespace Dali { - +/** + * Data members for ConditionalWait + */ struct ConditionalWait::ConditionalWaitImpl { - pthread_mutex_t mutex; - pthread_cond_t condition; + std::mutex mutex; + std::condition_variable condition; volatile unsigned int count; }; +/** + * Data members for ConditionalWait::ScopedLock + */ +struct ConditionalWait::ScopedLock::ScopedLockImpl +{ + ScopedLockImpl( ConditionalWait& wait ) + : wait( wait ), + lock( wait.mImpl->mutex ) // locks for the lifecycle of this object + { } + ConditionalWait& wait; + std::unique_lock lock; +}; -ConditionalWait::ScopedLock::ScopedLock( ConditionalWait& wait ) : mWait(wait) +ConditionalWait::ScopedLock::ScopedLock( ConditionalWait& wait ) +: mImpl( new ConditionalWait::ScopedLock::ScopedLockImpl( wait ) ) { - Internal::Mutex::Lock( &wait.mImpl->mutex ); + Internal::MutexTrace::Lock(); // matching sequence in mutex.cpp } ConditionalWait::ScopedLock::~ScopedLock() { - ConditionalWait& wait = mWait; - Internal::Mutex::Unlock( &wait.mImpl->mutex ); + Internal::MutexTrace::Unlock(); + delete mImpl; +} + +ConditionalWait& ConditionalWait::ScopedLock::GetLockedWait() const +{ + return mImpl->wait; // mImpl can never be NULL } ConditionalWait::ConditionalWait() : mImpl( new ConditionalWaitImpl ) { - if( pthread_mutex_init( &mImpl->mutex, NULL ) ) - { - DALI_LOG_ERROR( "Unable to initialise mutex in ConditionalWait\n" ); - } - if( pthread_cond_init( &mImpl->condition, NULL ) ) - { - DALI_LOG_ERROR( "Unable to initialise conditional\n" ); - } mImpl->count = 0; } ConditionalWait::~ConditionalWait() { - if( pthread_cond_destroy( &mImpl->condition ) ) - { - DALI_LOG_ERROR( "Unable to destroy conditional\n" ); - } - if( pthread_mutex_destroy( &mImpl->mutex ) ) - { - DALI_LOG_ERROR( "Unable to destroy mutex in ConditionalWait\n" ); - } delete mImpl; } void ConditionalWait::Notify() { - // pthread_cond_wait requires a lock to be held - Internal::Mutex::Lock( &mImpl->mutex ); + // conditional wait requires a lock to be held + ScopedLock lock( *this ); volatile unsigned int previousCount = mImpl->count; mImpl->count = 0; // change state before broadcast as that may wake clients immediately - // broadcast does nothing if the thread is not waiting but still has a system call overhead - // broadcast all threads to continue + // notify does nothing if the thread is not waiting but still has a system call overhead + // notify all threads to continue if( 0 != previousCount ) { - if( pthread_cond_broadcast( &mImpl->condition ) ) - { - DALI_LOG_ERROR( "Error calling pthread_cond_broadcast\n" ); - } + mImpl->condition.notify_all(); // no return value } - Internal::Mutex::Unlock( &mImpl->mutex ); } void ConditionalWait::Notify( const ScopedLock& scope ) @@ -99,35 +100,26 @@ void ConditionalWait::Notify( const ScopedLock& scope ) volatile unsigned int previousCount = mImpl->count; mImpl->count = 0; // change state before broadcast as that may wake clients immediately - // broadcast does nothing if the thread is not waiting but still has a system call overhead - // broadcast all threads to continue + // notify does nothing if the thread is not waiting but still has a system call overhead + // notify all threads to continue if( 0 != previousCount ) { - if( pthread_cond_broadcast( &mImpl->condition ) ) - { - DALI_LOG_ERROR( "Error calling pthread_cond_broadcast\n" ); - } + mImpl->condition.notify_all(); // no return value } } void ConditionalWait::Wait() { - // pthread_cond_wait requires a lock to be held - Internal::Mutex::Lock( &mImpl->mutex ); + // conditional wait requires a lock to be held + ScopedLock scope( *this ); ++(mImpl->count); - // pthread_cond_wait may wake up without anyone calling Notify + // conditional wait may wake up without anyone calling Notify do { // wait while condition changes - if( pthread_cond_wait( &mImpl->condition, &mImpl->mutex ) ) // releases the lock whilst waiting - { - DALI_LOG_ERROR( "Error calling pthread_cond_wait\n" ); - break; - } + mImpl->condition.wait( scope.mImpl->lock ); // need to pass in the std::unique_lock } while( 0 != mImpl->count ); - // when condition returns the mutex is locked so release the lock - Internal::Mutex::Unlock( &mImpl->mutex ); } void ConditionalWait::Wait( const ScopedLock& scope ) @@ -137,16 +129,11 @@ void ConditionalWait::Wait( const ScopedLock& scope ) ++(mImpl->count); - // pthread_cond_wait may wake up without anyone calling Notify so loop until - // count has been reset in a notify: + // conditional wait may wake up without anyone calling Notify do { // wait while condition changes - if( pthread_cond_wait( &mImpl->condition, &mImpl->mutex ) ) // releases the lock whilst waiting - { - DALI_LOG_ERROR( "Error calling pthread_cond_wait\n" ); - break; - } + mImpl->condition.wait( scope.mImpl->lock ); // need to pass in the std::unique_lock } while( 0 != mImpl->count ); diff --git a/dali/devel-api/threading/conditional-wait.h b/dali/devel-api/threading/conditional-wait.h index 2cbd289..81660cc 100644 --- a/dali/devel-api/threading/conditional-wait.h +++ b/dali/devel-api/threading/conditional-wait.h @@ -2,7 +2,7 @@ #define __DALI_CONDITIONAL_WAIT_H__ /* - * Copyright (c) 2015 Samsung Electronics Co., Ltd. + * Copyright (c) 2017 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. @@ -55,15 +55,18 @@ public: * Getter for the ConditionalWait locked for this instance's lifetime. * @return the ConditionalWait object currently locked. */ - ConditionalWait& GetLockedWait() const { return mWait; } + ConditionalWait& GetLockedWait() const; + + public: // Data, public to allow nesting class to access + + struct ScopedLockImpl; + ScopedLockImpl* mImpl; private: // Not implemented as ScopedLock cannot be copied: ScopedLock( const ScopedLock& ); const ScopedLock& operator=( const ScopedLock& ); - - ConditionalWait& mWait; }; /** diff --git a/dali/devel-api/threading/mutex.cpp b/dali/devel-api/threading/mutex.cpp index 4e0cb30..0f1770a 100644 --- a/dali/devel-api/threading/mutex.cpp +++ b/dali/devel-api/threading/mutex.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 Samsung Electronics Co., Ltd. + * Copyright (c) 2017 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. @@ -19,37 +19,29 @@ #include // EXTERNAL INCLUDES -#include +#include // INTERNAL INCLUDES #include -#include +#include namespace Dali { struct Mutex::MutexImpl { - pthread_mutex_t mutex; + std::mutex mutex; bool locked; }; Mutex::Mutex() : mImpl( new MutexImpl ) { - if( pthread_mutex_init( &mImpl->mutex, NULL ) ) - { - DALI_LOG_ERROR( "Unable to initialise Mutex\n" ); - } mImpl->locked = false; } Mutex::~Mutex() { - if( pthread_mutex_destroy( &mImpl->mutex ) ) - { - DALI_LOG_ERROR( "Unable to destroy Mutex\n" ); - } // nothing else to do as there is no Lock/Unlock API // ScopedLock destructor will always unlock the mutex delete mImpl; @@ -63,14 +55,16 @@ bool Mutex::IsLocked() Mutex::ScopedLock::ScopedLock( Mutex& mutex ) : mMutex( mutex ) { - Internal::Mutex::Lock( &mMutex.mImpl->mutex ); + mMutex.mImpl->mutex.lock(); + Internal::MutexTrace::Lock(); // matching sequence in conditional-wait.cpp mMutex.mImpl->locked = true; } Mutex::ScopedLock::~ScopedLock() { mMutex.mImpl->locked = false; - Internal::Mutex::Unlock( &mMutex.mImpl->mutex ); + Internal::MutexTrace::Unlock(); // reverse sequence from lock + mMutex.mImpl->mutex.unlock(); } } // namespace Dali diff --git a/dali/devel-api/threading/mutex.h b/dali/devel-api/threading/mutex.h index cd41f9a..9490212 100644 --- a/dali/devel-api/threading/mutex.h +++ b/dali/devel-api/threading/mutex.h @@ -2,7 +2,7 @@ #define __DALI_MUTEX_H__ /* - * Copyright (c) 2015 Samsung Electronics Co., Ltd. + * Copyright (c) 2017 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. diff --git a/dali/devel-api/threading/thread.cpp b/dali/devel-api/threading/thread.cpp index d7bfa0b..be4077d 100644 --- a/dali/devel-api/threading/thread.cpp +++ b/dali/devel-api/threading/thread.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 Samsung Electronics Co., Ltd. + * Copyright (c) 2017 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. @@ -20,7 +20,7 @@ // EXTERNAL INCLUDES #include -#include +#include #include namespace Dali @@ -28,43 +28,45 @@ namespace Dali struct Thread::ThreadImpl { - pthread_t thread; - bool isCreated; + ThreadImpl( Thread& aThis ) + : thread( &Thread::InternalThreadEntryFunc, std::ref( aThis ) ) + { + // std::thread starts execution immediately + } + ~ThreadImpl( ) + { + thread.join(); + } + std::thread thread; }; Thread::Thread() -: mImpl( new ThreadImpl ) +: mImpl( nullptr ) { - mImpl->isCreated = false; } Thread::~Thread() { - delete mImpl; + Join(); } void Thread::Start() { - DALI_ASSERT_DEBUG( !mImpl->isCreated ); - - int error = pthread_create( &(mImpl->thread), NULL, InternalThreadEntryFunc, this ); - DALI_ASSERT_ALWAYS( !error && "Failed to create a new thread" ); - mImpl->isCreated = true; + if( !mImpl ) + { + mImpl = new Thread::ThreadImpl( *this ); + } } void Thread::Join() { - if( mImpl->isCreated ) - { - mImpl->isCreated = false; - pthread_join( mImpl->thread, NULL ); - } + delete mImpl; + mImpl = nullptr; } -void* Thread::InternalThreadEntryFunc( void* This ) +void Thread::InternalThreadEntryFunc( Thread& aThis ) { - ( static_cast( This ) )->Run(); - return NULL; + aThis.Run(); } } // namespace Dali diff --git a/dali/devel-api/threading/thread.h b/dali/devel-api/threading/thread.h index 535e700..0e21639 100644 --- a/dali/devel-api/threading/thread.h +++ b/dali/devel-api/threading/thread.h @@ -2,7 +2,7 @@ #define __DALI_THREAD_H__ /* - * Copyright (c) 2015 Samsung Electronics Co., Ltd. + * Copyright (c) 2017 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,7 +21,6 @@ // INTERNAL INCLUDES #include - /** * The top level DALi namespace */ @@ -66,9 +65,9 @@ private: /** * Helper for the thread calling the entry function. - * @param[in] This A pointer to the current RenderThread object + * @param[in] This A pointer to the current thread object */ - static void* InternalThreadEntryFunc( void* This ); + static void InternalThreadEntryFunc( Thread& This ); // Undefined Thread( const Thread& ); @@ -79,6 +78,7 @@ private: struct ThreadImpl; ThreadImpl* mImpl; + }; } diff --git a/dali/internal/common/mutex-impl.cpp b/dali/internal/common/mutex-trace.cpp similarity index 89% rename from dali/internal/common/mutex-impl.cpp rename to dali/internal/common/mutex-trace.cpp index 9c85e83..c96b72c 100644 --- a/dali/internal/common/mutex-impl.cpp +++ b/dali/internal/common/mutex-trace.cpp @@ -16,8 +16,7 @@ */ // HEADER -#include - +#include #ifdef LOCK_BACKTRACE_ENABLED // EXTERNAL INCLUDES @@ -42,7 +41,7 @@ extern std::string Demangle( const char* symbol ); namespace Internal { -namespace Mutex +namespace MutexTrace { namespace @@ -66,7 +65,7 @@ thread_local BackTraceInfo gBackTraceInfo[ MAX_LOCK_SUPPORT ]; ///< Thread local thread_local unsigned int gThreadLockCount = 0; ///< Thread local storage for the backtrace of the locks } // unnamed namespace -void Lock( pthread_mutex_t* mutex ) +void Lock() { ++gThreadLockCount; @@ -106,23 +105,14 @@ void Lock( pthread_mutex_t* mutex ) //DALI_ASSERT_DEBUG( gThreadLockCount == 1 ); #endif // LOCK_BACKTRACE_ENABLED - - if( pthread_mutex_lock( mutex ) ) - { - DALI_LOG_ERROR( "Error calling pthread_mutex_lock\n" ); - } } -void Unlock( pthread_mutex_t* mutex ) +void Unlock() { - if( pthread_mutex_unlock( mutex ) ) - { - DALI_LOG_ERROR( "Error calling pthread_mutex_unlock\n" ); - } --gThreadLockCount; } -} // namespace Mutex +} // namespace MutexTrace } // namespace Internal diff --git a/dali/internal/common/mutex-impl.h b/dali/internal/common/mutex-trace.h similarity index 65% rename from dali/internal/common/mutex-impl.h rename to dali/internal/common/mutex-trace.h index bcf07e9..12c856f 100644 --- a/dali/internal/common/mutex-impl.h +++ b/dali/internal/common/mutex-trace.h @@ -1,8 +1,8 @@ -#ifndef __DALI_INTERNAL_MUTEX_H__ -#define __DALI_INTERNAL_MUTEX_H__ +#ifndef DALI_INTERNAL_MUTEX_TRACE_H +#define DALI_INTERNAL_MUTEX_TRACE_H /* - * Copyright (c) 2015 Samsung Electronics Co., Ltd. + * Copyright (c) 2017 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. @@ -18,9 +18,6 @@ * */ -// EXTERNAL INCLUDES -#include - namespace Dali { @@ -35,33 +32,25 @@ namespace Internal * * @note lock backtrace needs to be enabled to see the warnings. */ -namespace Mutex +namespace MutexTrace { /** - * @brief Locks the given mutex. - * - * Increments a thread-local storage counter. - * - * @param A pointer to the mutex that should be locked. + * @brief Increments a thread-local storage counter. * * @note If the counter is > 1 and lock backtrace is enabled, then the backtrace for all locks will be shown as a warning. */ -void Lock( pthread_mutex_t* mutex ); +void Lock(); /** - * @brief Unlocks the given mutex. - * - * @param A pointer to the mutex that should be unlocked. - * - * Decrements a thread-local storage counter. + * @brief Decrements a thread-local storage counter */ -void Unlock( pthread_mutex_t* mutex ); +void Unlock(); -} // namespace Mutex +} // namespace MutexTrace } // namespace Internal } // namespace Dali -#endif // __DALI_INTERNAL_MUTEX_H__ +#endif // DALI_INTERNAL_MUTEX_TRACE_H diff --git a/dali/internal/file.list b/dali/internal/file.list index 4b98598..bae7b47 100644 --- a/dali/internal/file.list +++ b/dali/internal/file.list @@ -6,7 +6,7 @@ internal_src_files = \ $(internal_src_dir)/common/internal-constants.cpp \ $(internal_src_dir)/common/math.cpp \ $(internal_src_dir)/common/message-buffer.cpp \ - $(internal_src_dir)/common/mutex-impl.cpp \ + $(internal_src_dir)/common/mutex-trace.cpp \ $(internal_src_dir)/common/image-sampler.cpp \ $(internal_src_dir)/common/image-attributes.cpp \ $(internal_src_dir)/common/fixed-size-memory-pool.cpp \ diff --git a/packaging/dali.spec b/packaging/dali.spec index a21daa5..df2756f 100644 --- a/packaging/dali.spec +++ b/packaging/dali.spec @@ -67,7 +67,7 @@ Integration development package for the OpenGLES Canvas - headers for integratin %build PREFIX="/usr" CXXFLAGS+=" -Wall -g -Os -DNDEBUG -fPIC -fvisibility-inlines-hidden -fdata-sections -ffunction-sections " -LDFLAGS+=" -Wl,--rpath=$PREFIX/lib -Wl,--as-needed -Wl,--gc-sections -lgcc_s -lgcc -lpthread -Wl,-Bsymbolic-functions " +LDFLAGS+=" -Wl,--rpath=$PREFIX/lib -Wl,--as-needed -Wl,--gc-sections -lgcc_s -lgcc -Wl,-Bsymbolic-functions " %ifarch %{arm} CXXFLAGS+=" -D_ARCH_ARM_ -mfpu=neon" -- 2.7.4