[PATCH] D19412: [libcxx] Refactor pthread usage - II

[Asiri Rathnayake via cfe-commits]

rmaprath added a comment.

> On a bikeshed note: is `<__os_support>` the right name? Or should it be something like `<__thread>`  or `<__threading_support>`?


I went with `__os_support`  in case if we'd want to group further external dependencies (like, for example, if some non-c POSIX calls are used in the upcoming `filesystem` library). I can revert back to `__threading_support` if that's preferred.

Thanks.

/ Asiri


================
Comment at: include/__os_support:32
@@ +31,3 @@
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __os_mutex_init(__libcpp_mutex* __m, bool is_recursive = false)
----------------
mclow.lists wrote:
> These should all be marked with `_LIBCPP_ALWAYS_INLINE`.
Got it. Will update.

================
Comment at: src/algorithm.cpp:51
@@ -50,3 +50,3 @@
 #ifndef _LIBCPP_HAS_NO_THREADS
-static pthread_mutex_t __rs_mut = PTHREAD_MUTEX_INITIALIZER;
+static mutex __rs_mut;
 #endif
----------------
mclow.lists wrote:
> What happened to the initializer here?
I'm expecting the constructor of `mutex` to be run here at load time (before `main`). Note that this a libc++ mutex rather than a pthreads mutex (which does not required a constructor call like this). Would that be too much of an overhead?

================
Comment at: src/memory.cpp:130
@@ -129,11 +129,3 @@
 static const std::size_t __sp_mut_count = 16;
-static pthread_mutex_t mut_back_imp[__sp_mut_count] =
-{
-    PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER,
-    PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER,
-    PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER,
-    PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER
-};
-
-static mutex* mut_back = reinterpret_cast<std::mutex*>(mut_back_imp);
+static mutex mut_back[__sp_mut_count];
 
----------------
mclow.lists wrote:
> How does this array get initialized now?
Same as above, but bigger (this now requires 16 constructor calls during load time). I can check if clang would figure out that this is a trivial construction (at least for the default pthreads implementation) and be able to avoid the constructor calls altogether, but may be relying on such behaviour is not good.

================
Comment at: src/mutex.cpp:201
@@ -222,1 +200,3 @@
+static mutex mut;
+static condition_variable cv;
 #endif
----------------
mclow.lists wrote:
> These two variables used to be initialized. Now they're not.
Same, relying on initialization at load time.


http://reviews.llvm.org/D19412