You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2015/07/31 00:30:17 UTC

[1/2] mesos git commit: Stout: Introduced THREAD_LOCAL wrapper for thread local storage.

Repository: mesos
Updated Branches:
  refs/heads/master 1b9ce37f3 -> b80ef400d


Stout: Introduced THREAD_LOCAL wrapper for thread local storage.

This replaced the `ThreadLocal` primitive with the new C++11 standard.
The exception is on OSX where we use `__thread` as `thread_local` is
not supported.

Review: https://reviews.apache.org/r/36845


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9c86375c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9c86375c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9c86375c

Branch: refs/heads/master
Commit: 9c86375cd9297cc02ed17d5043597d4b240bfbba
Parents: 1b9ce37
Author: Joris Van Remoortere <jo...@gmail.com>
Authored: Thu Jul 30 15:27:37 2015 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Thu Jul 30 15:27:37 2015 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/3rdparty/stout/Makefile.am  |  1 -
 .../3rdparty/stout/include/Makefile.am          |  2 +-
 .../3rdparty/stout/include/stout/thread.hpp     | 80 --------------------
 .../stout/include/stout/thread_local.hpp        | 30 ++++++++
 .../3rdparty/stout/tests/thread_tests.cpp       | 40 ----------
 5 files changed, 31 insertions(+), 122 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9c86375c/3rdparty/libprocess/3rdparty/stout/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/Makefile.am b/3rdparty/libprocess/3rdparty/stout/Makefile.am
index 89e7b18..f95ed03 100644
--- a/3rdparty/libprocess/3rdparty/stout/Makefile.am
+++ b/3rdparty/libprocess/3rdparty/stout/Makefile.am
@@ -56,6 +56,5 @@ EXTRA_DIST =					\
   tests/strings_tests.cpp			\
   tests/subcommand_tests.cpp			\
   tests/svn_tests.cpp				\
-  tests/thread_tests.cpp			\
   tests/uuid_tests.cpp				\
   tests/version_tests.cpp

http://git-wip-us.apache.org/repos/asf/mesos/blob/9c86375c/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/Makefile.am b/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
index 5c19e3e..b299ce8 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
+++ b/3rdparty/libprocess/3rdparty/stout/include/Makefile.am
@@ -84,7 +84,7 @@ nobase_include_HEADERS =		\
   stout/svn.hpp				\
   stout/synchronized.hpp		\
   stout/tests/utils.hpp			\
-  stout/thread.hpp			\
+  stout/thread_local.hpp		\
   stout/try.hpp				\
   stout/unreachable.hpp			\
   stout/utils.hpp			\

http://git-wip-us.apache.org/repos/asf/mesos/blob/9c86375c/3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp
deleted file mode 100644
index 552d6e9..0000000
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp
+++ /dev/null
@@ -1,80 +0,0 @@
-/**
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *  http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#ifndef __STOUT_THREAD_HPP__
-#define __STOUT_THREAD_HPP__
-
-#include <errno.h>
-#include <pthread.h>
-#include <stdio.h> // For perror.
-
-#include <string>
-
-#include <stout/abort.hpp>
-
-template <typename T>
-struct ThreadLocal
-{
-  ThreadLocal()
-  {
-    errno = pthread_key_create(&key, NULL);
-
-    if (errno != 0) {
-      ABORT(std::string("Failed to create thread local, pthread_key_create: ") +
-            strerror(errno));
-    }
-  }
-
-  ~ThreadLocal()
-  {
-    errno = pthread_key_delete(key);
-
-    if (errno != 0) {
-      ABORT("Failed to destruct thread local, pthread_key_delete: " +
-            std::string(strerror(errno)));
-    }
-  }
-
-  ThreadLocal<T>& operator = (T* t)
-  {
-    errno = pthread_setspecific(key, t);
-
-    if (errno != 0) {
-      ABORT(std::string("Failed to set thread local, pthread_setspecific: ") +
-            strerror(errno));
-    }
-    return *this;
-  }
-
-  operator T* () const
-  {
-    return reinterpret_cast<T*>(pthread_getspecific(key));
-  }
-
-  T* operator -> () const
-  {
-    return reinterpret_cast<T*>(pthread_getspecific(key));
-  }
-
-private:
-  // Not expecting any other operators to be used (and the rest?).
-  bool operator * (const ThreadLocal<T>&) const;
-  bool operator == (const ThreadLocal<T>&) const;
-  bool operator != (const ThreadLocal<T>&) const;
-  bool operator < (const ThreadLocal<T>&) const;
-  bool operator > (const ThreadLocal<T>&) const;
-
-  pthread_key_t key;
-};
-
-#endif // __STOUT_THREAD_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/9c86375c/3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp
new file mode 100644
index 0000000..454abdf
--- /dev/null
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp
@@ -0,0 +1,30 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __STOUT_THREAD_LOCAL_HPP__
+#define __STOUT_THREAD_LOCAL_HPP__
+
+// A wrapper around the thread local storage attribute. The default
+// clang on OSX does not support the c++11 standard `thread_local`
+// intentionally until a higher performance implementation is
+// released. See https://devforums.apple.com/message/1079348#1079348
+// Until then, we use `__thread` on OSX instead.
+// We required that THREAD_LOCAL is only used with POD types as this
+// is a requirement of `__thread`.
+#ifdef __APPLE__
+#define THREAD_LOCAL __thread
+#else
+#define THREAD_LOCAL thread_local
+#endif
+
+#endif // __STOUT_THREAD_LOCAL_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/9c86375c/3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp
deleted file mode 100644
index 319fcdf..0000000
--- a/3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp
+++ /dev/null
@@ -1,40 +0,0 @@
-/**
-* Licensed under the Apache License, Version 2.0 (the "License");
-* you may not use this file except in compliance with the License.
-* You may obtain a copy of the License at
-*
-*     http://www.apache.org/licenses/LICENSE-2.0
-*
-* Unless required by applicable law or agreed to in writing, software
-* distributed under the License is distributed on an "AS IS" BASIS,
-* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-* See the License for the specific language governing permissions and
-* limitations under the License
-*/
-
-#include <string>
-
-#include <gtest/gtest.h>
-
-#include <stout/thread.hpp>
-
-TEST(ThreadTest, Local)
-{
-  ThreadLocal<std::string>* _s_ = new ThreadLocal<std::string>();
-
-  std::string* s = new std::string();
-
-  ASSERT_TRUE(*(_s_) == NULL);
-
-  (*_s_) = s;
-
-  ASSERT_TRUE(*(_s_) == s);
-  ASSERT_FALSE(*(_s_) == NULL);
-
-  (*_s_) = NULL;
-
-  ASSERT_TRUE(*(_s_) == NULL);
-
-  delete s;
-  delete _s_;
-}


[2/2] mesos git commit: Used THREAD_LOCAL to replace ThreadLocal.

Posted by be...@apache.org.
Used THREAD_LOCAL to replace ThreadLocal.

Review: https://reviews.apache.org/r/36844


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b80ef400
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b80ef400
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b80ef400

Branch: refs/heads/master
Commit: b80ef400d139fae5c843173163e6a392e75bff4a
Parents: 9c86375
Author: Joris Van Remoortere <jo...@gmail.com>
Authored: Thu Jul 30 15:29:01 2015 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Thu Jul 30 15:29:02 2015 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/3rdparty/Makefile.am         |  1 -
 3rdparty/libprocess/include/process/executor.hpp | 12 +++++-------
 3rdparty/libprocess/include/process/process.hpp  | 10 +++-------
 3rdparty/libprocess/src/libev.cpp                |  3 ++-
 3rdparty/libprocess/src/libev.hpp                | 14 ++++++--------
 3rdparty/libprocess/src/libevent.cpp             |  3 ++-
 3rdparty/libprocess/src/libevent.hpp             | 14 ++++++--------
 3rdparty/libprocess/src/process.cpp              |  6 +++---
 8 files changed, 27 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b80ef400/3rdparty/libprocess/3rdparty/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/Makefile.am b/3rdparty/libprocess/3rdparty/Makefile.am
index bd95fe1..790bb46 100644
--- a/3rdparty/libprocess/3rdparty/Makefile.am
+++ b/3rdparty/libprocess/3rdparty/Makefile.am
@@ -190,7 +190,6 @@ stout_tests_SOURCES =				\
   $(STOUT)/tests/some_tests.cpp			\
   $(STOUT)/tests/strings_tests.cpp		\
   $(STOUT)/tests/subcommand_tests.cpp		\
-  $(STOUT)/tests/thread_tests.cpp		\
   $(STOUT)/tests/uuid_tests.cpp			\
   $(STOUT)/tests/version_tests.cpp
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/b80ef400/3rdparty/libprocess/include/process/executor.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/executor.hpp b/3rdparty/libprocess/include/process/executor.hpp
index 434d23a..23384c2 100644
--- a/3rdparty/libprocess/include/process/executor.hpp
+++ b/3rdparty/libprocess/include/process/executor.hpp
@@ -20,7 +20,7 @@
 #include <process/id.hpp>
 #include <process/process.hpp>
 
-#include <stout/thread.hpp>
+#include <stout/thread_local.hpp>
 
 namespace process {
 
@@ -68,14 +68,12 @@ private:
 };
 
 
-// Per thread executor pointer. The extra level of indirection from
-// _executor_ to __executor__ is used in order to take advantage of
-// the ThreadLocal operators without needing the extra dereference as
-// well as lazily construct the actual executor.
-extern ThreadLocal<Executor>* _executor_;
+// Per thread executor pointer. We use a pointer to lazily construct the
+// actual executor.
+extern THREAD_LOCAL Executor* _executor_;
 
 #define __executor__                                                    \
-  (*_executor_ == NULL ? *_executor_ = new Executor() : *_executor_)
+  (_executor_ == NULL ? _executor_ = new Executor() : _executor_)
 
 } // namespace process {
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/b80ef400/3rdparty/libprocess/include/process/process.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/process.hpp b/3rdparty/libprocess/include/process/process.hpp
index 8620547..2558198 100644
--- a/3rdparty/libprocess/include/process/process.hpp
+++ b/3rdparty/libprocess/include/process/process.hpp
@@ -36,7 +36,7 @@
 #include <stout/lambda.hpp>
 #include <stout/option.hpp>
 #include <stout/synchronized.hpp>
-#include <stout/thread.hpp>
+#include <stout/thread_local.hpp>
 
 namespace process {
 
@@ -505,12 +505,8 @@ inline bool wait(const ProcessBase* process, const Duration& duration)
 }
 
 
-// Per thread process pointer. The extra level of indirection from
-// _process_ to __process__ is used in order to take advantage of the
-// ThreadLocal operators without needing the extra dereference.
-extern ThreadLocal<ProcessBase>* _process_;
-
-#define __process__ (*_process_)
+// Per thread process pointer.
+extern THREAD_LOCAL ProcessBase* __process__;
 
 // NOTE: Methods in this namespace should only be used in tests to
 // inject arbitrary events.

http://git-wip-us.apache.org/repos/asf/mesos/blob/b80ef400/3rdparty/libprocess/src/libev.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libev.cpp b/3rdparty/libprocess/src/libev.cpp
index 55ed6ef..aca10a1 100644
--- a/3rdparty/libprocess/src/libev.cpp
+++ b/3rdparty/libprocess/src/libev.cpp
@@ -20,6 +20,7 @@
 #include <stout/duration.hpp>
 #include <stout/lambda.hpp>
 #include <stout/nothing.hpp>
+#include <stout/thread_local.hpp>
 
 #include "event_loop.hpp"
 #include "libev.hpp"
@@ -39,7 +40,7 @@ std::mutex* watchers_mutex = new std::mutex();
 std::queue<lambda::function<void(void)>>* functions =
   new std::queue<lambda::function<void(void)>>();
 
-ThreadLocal<bool>* _in_event_loop_ = new ThreadLocal<bool>();
+THREAD_LOCAL bool* _in_event_loop_ = NULL;
 
 
 void handle_async(struct ev_loop* loop, ev_async* _, int revents)

http://git-wip-us.apache.org/repos/asf/mesos/blob/b80ef400/3rdparty/libprocess/src/libev.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libev.hpp b/3rdparty/libprocess/src/libev.hpp
index fd26728..09210ab 100644
--- a/3rdparty/libprocess/src/libev.hpp
+++ b/3rdparty/libprocess/src/libev.hpp
@@ -25,7 +25,7 @@
 
 #include <stout/lambda.hpp>
 #include <stout/synchronized.hpp>
-#include <stout/thread.hpp>
+#include <stout/thread_local.hpp>
 
 namespace process {
 
@@ -47,14 +47,12 @@ extern std::mutex* watchers_mutex;
 // loop (protected by 'watchers' above).
 extern std::queue<lambda::function<void(void)>>* functions;
 
-// Per thread bool pointer. The extra level of indirection from
-// _in_event_loop_ to __in_event_loop__ is used in order to take
-// advantage of the ThreadLocal operators without needing the extra
-// dereference as well as lazily construct the actual bool.
-extern ThreadLocal<bool>* _in_event_loop_;
+// Per thread bool pointer. We use a pointer to lazily construct the
+// actual bool.
+extern THREAD_LOCAL bool* _in_event_loop_;
 
-#define __in_event_loop__ *(*_in_event_loop_ == NULL ?               \
-  *_in_event_loop_ = new bool(false) : *_in_event_loop_)
+#define __in_event_loop__ *(_in_event_loop_ == NULL ?                \
+  _in_event_loop_ = new bool(false) : _in_event_loop_)
 
 
 // Wrapper around function we want to run in the event loop.

http://git-wip-us.apache.org/repos/asf/mesos/blob/b80ef400/3rdparty/libprocess/src/libevent.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent.cpp b/3rdparty/libprocess/src/libevent.cpp
index c604caa..9d2af55 100644
--- a/3rdparty/libprocess/src/libevent.cpp
+++ b/3rdparty/libprocess/src/libevent.cpp
@@ -24,6 +24,7 @@
 
 #include <stout/os/signals.hpp>
 #include <stout/synchronized.hpp>
+#include <stout/thread_local.hpp>
 
 #include "event_loop.hpp"
 #include "libevent.hpp"
@@ -38,7 +39,7 @@ std::queue<lambda::function<void(void)>>* functions =
   new std::queue<lambda::function<void(void)>>();
 
 
-ThreadLocal<bool>* _in_event_loop_ = new ThreadLocal<bool>();
+THREAD_LOCAL bool* _in_event_loop_ = NULL;
 
 
 void async_function(int socket, short which, void* arg)

http://git-wip-us.apache.org/repos/asf/mesos/blob/b80ef400/3rdparty/libprocess/src/libevent.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent.hpp b/3rdparty/libprocess/src/libevent.hpp
index 3a0a46b..6617ea6 100644
--- a/3rdparty/libprocess/src/libevent.hpp
+++ b/3rdparty/libprocess/src/libevent.hpp
@@ -18,7 +18,7 @@
 #include <event2/event.h>
 
 #include <stout/lambda.hpp>
-#include <stout/thread.hpp>
+#include <stout/thread_local.hpp>
 
 namespace process {
 
@@ -26,15 +26,13 @@ namespace process {
 extern event_base* base;
 
 
-// Per thread bool pointer. The extra level of indirection from
-// _in_event_loop_ to __in_event_loop__ is used in order to take
-// advantage of the ThreadLocal operators without needing the extra
-// dereference as well as lazily construct the actual bool.
-extern ThreadLocal<bool>* _in_event_loop_;
+// Per thread bool pointer. We use a pointer to lazily construct the
+// actual bool.
+extern THREAD_LOCAL bool* _in_event_loop_;
 
 
-#define __in_event_loop__ *(*_in_event_loop_ == NULL ?               \
-  *_in_event_loop_ = new bool(false) : *_in_event_loop_)
+#define __in_event_loop__ *(_in_event_loop_ == NULL ?                \
+  _in_event_loop_ = new bool(false) : _in_event_loop_)
 
 
 enum EventLoopLogicFlow {

http://git-wip-us.apache.org/repos/asf/mesos/blob/b80ef400/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 6d3609d..fd000f4 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -91,7 +91,7 @@
 #include <stout/path.hpp>
 #include <stout/strings.hpp>
 #include <stout/synchronized.hpp>
-#include <stout/thread.hpp>
+#include <stout/thread_local.hpp>
 #include <stout/unreachable.hpp>
 
 #include "config.hpp"
@@ -467,10 +467,10 @@ PID<GarbageCollector> gc;
 PID<Help> help;
 
 // Per thread process pointer.
-ThreadLocal<ProcessBase>* _process_ = new ThreadLocal<ProcessBase>();
+THREAD_LOCAL ProcessBase* __process__ = NULL;
 
 // Per thread executor pointer.
-ThreadLocal<Executor>* _executor_ = new ThreadLocal<Executor>();
+THREAD_LOCAL Executor* _executor_ = NULL;
 
 // TODO(dhamon): Reintroduce this when it is plumbed through to Statistics.
 // const Duration LIBPROCESS_STATISTICS_WINDOW = Days(1);