Change Dali::Thread, Dali::Mutex and Dali::ConditionalWait to use std::thread from... 19/144719/9
authorKimmo Hoikka <kimmo.hoikka@samsung.com>
Thu, 17 Aug 2017 13:51:33 +0000 (14:51 +0100)
committerKimmo Hoikka <kimmo.hoikka@samsung.com>
Thu, 24 Aug 2017 10:07:00 +0000 (11:07 +0100)
Change-Id: I249a75bcf5bda7c4a747182cfb95710db244477f

build/tizen/dali-core/Makefile.am
dali/devel-api/threading/conditional-wait.cpp
dali/devel-api/threading/conditional-wait.h
dali/devel-api/threading/mutex.cpp
dali/devel-api/threading/mutex.h
dali/devel-api/threading/thread.cpp
dali/devel-api/threading/thread.h
dali/internal/common/mutex-trace.cpp [moved from dali/internal/common/mutex-impl.cpp with 89% similarity]
dali/internal/common/mutex-trace.h [moved from dali/internal/common/mutex-impl.h with 65% similarity]
dali/internal/file.list
packaging/dali.spec

index ad081d5..137b6f6 100644 (file)
@@ -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
index adc5760..fa0a86c 100644 (file)
@@ -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.
 #include <dali/devel-api/threading/conditional-wait.h>
 
 // EXTERNAL INCLUDES
-#include <pthread.h>
+#include <mutex>
+#include <condition_variable>
 
 // INTERNAL INCLUDES
 #include <dali/integration-api/debug.h>
-#include <dali/internal/common/mutex-impl.h>
+#include <dali/internal/common/mutex-trace.h>
 
 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<std::mutex> 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 );
 
index 2cbd289..81660cc 100644 (file)
@@ -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;
   };
 
   /**
index 4e0cb30..0f1770a 100644 (file)
@@ -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.
 #include <dali/devel-api/threading/mutex.h>
 
 // EXTERNAL INCLUDES
-#include <pthread.h>
+#include <mutex>
 
 // INTERNAL INCLUDES
 #include <dali/integration-api/debug.h>
-#include <dali/internal/common/mutex-impl.h>
+#include <dali/internal/common/mutex-trace.h>
 
 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
index cd41f9a..9490212 100644 (file)
@@ -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.
index d7bfa0b..be4077d 100644 (file)
@@ -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 <cstddef>
-#include <pthread.h>
+#include <thread>
 #include <dali/integration-api/debug.h>
 
 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<Thread*>( This ) )->Run();
-  return NULL;
+  aThis.Run();
 }
 
 } // namespace Dali
index 535e700..0e21639 100644 (file)
@@ -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 <dali/public-api/common/dali-common.h>
 
-
 /**
  * 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;
+
 };
 
 }
similarity index 89%
rename from dali/internal/common/mutex-impl.cpp
rename to dali/internal/common/mutex-trace.cpp
index 9c85e83..c96b72c 100644 (file)
@@ -16,8 +16,7 @@
  */
 
 // HEADER
-#include <dali/internal/common/mutex-impl.h>
-
+#include <dali/internal/common/mutex-trace.h>
 
 #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
 
similarity index 65%
rename from dali/internal/common/mutex-impl.h
rename to dali/internal/common/mutex-trace.h
index bcf07e9..12c856f 100644 (file)
@@ -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 <pthread.h>
-
 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
index 4b98598..bae7b47 100644 (file)
@@ -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 \
index a21daa5..df2756f 100644 (file)
@@ -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"