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/06/14 11:46:29 UTC

[11/21] mesos git commit: Improvements to Synchronized.

Improvements to Synchronized.

(1) Simplify introducing new synchronization primitives.

Before:

```cpp
template <>
class Synchronized<bufferevent>
{
public:
  Synchronized(bufferevent* _bev) : bev(CHECK_NOTNULL(_bev))
  {
    bufferevent_lock(bev);
  }

  Synchronized(bufferevent** _bev) : Synchronized(*CHECK_NOTNULL(_bev)) {}

  ~Synchronized()
  {
    bufferevent_unlock(bev);
  }

  operator bool() const { return true; }

private:
  bufferevent* bev;
};
```

After:

```cpp
Synchronized<bufferevent> synchronize(bufferevent* bev) {
  return {
    bev,
    [](bufferevent* bev) { bufferevent_lock(bev); }
    [](bufferevent* bev) { bufferevent_unlock(bev); }
  };
}
```

(2) Enable `return` within `synchronized` and avoid the `control reaches end of non-void function` warning.

```cpp
int foo()
{
  int x = 42;
  std::mutex m;
  synchronized (m) {
    return x;
  }
}
```

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


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

Branch: refs/heads/master
Commit: 19b1960ffd01a61e4e0d2b7b55f5aa60a9cf8738
Parents: c683b00
Author: Michael Park <mc...@gmail.com>
Authored: Sat Jun 13 05:51:50 2015 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jun 14 02:43:00 2015 -0700

----------------------------------------------------------------------
 .../stout/include/stout/synchronized.hpp        | 156 ++++++++++++++-----
 1 file changed, 120 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/19b1960f/3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp
index 60eaf26..9b11cbc 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp
@@ -19,57 +19,141 @@
 #include <mutex>
 #include <type_traits>
 
-// A helper class for the synchronized(m) macro. It is an RAII 'guard'
-// for a synchronization primitive 'T'. The general template handles
-// cases such as 'std::mutex' and 'std::recursive_mutex'.
+#include <stout/preprocessor.hpp>
+
+// An RAII class for the 'synchronized(m)' macro.
 template <typename T>
 class Synchronized
 {
 public:
-  Synchronized(T* _lock) : lock(CHECK_NOTNULL(_lock)) { lock->lock(); }
-  Synchronized(T** _lock) : Synchronized(*CHECK_NOTNULL(_lock)) {}
+  explicit Synchronized(T* t, void (*acquire)(T*), void (*release)(T*))
+    : t_(CHECK_NOTNULL(t)), release_(release)
+  {
+    acquire(t_);
+  }
+
+  ~Synchronized() { release_(t_); }
 
-  ~Synchronized() { lock->unlock(); }
+  // NOTE: 'false' being returned here has no significance.
+  //       Refer to the NOTE for 'synchronized' at the bottom for why.
+  explicit operator bool() const { return false; }
 
-  operator bool() const { return true; }
 private:
-  T* lock;
+  T* t_;
+
+  void (*release_)(T*);
 };
 
 
-// A specialization of the Synchronized class for 'std::atomic_flag'.
-// This is necessary as the locking functions are different.
-template <>
-class Synchronized<std::atomic_flag>
+// The generic version handles mutexes which have 'lock' and 'unlock'
+// member functions such as 'std::mutex' and 'std::recursive_mutex'.
+template <typename T>
+Synchronized<T> synchronize(T* t)
 {
-public:
-  Synchronized(std::atomic_flag* _flag) : flag(CHECK_NOTNULL(_flag))
-  {
-    while (flag->test_and_set(std::memory_order_acquire)) {}
-  }
-  Synchronized(std::atomic_flag** _flag)
-    : Synchronized(*CHECK_NOTNULL(_flag)) {}
+  return Synchronized<T>(
+    t,
+    [](T* t) { t->lock(); },
+    [](T* t) { t->unlock(); }
+  );
+}
 
-  ~Synchronized()
-  {
-    flag->clear(std::memory_order_release);
-  }
 
-  operator bool() const { return true; }
-private:
-  std::atomic_flag* flag;
-};
+// An overload of the 'synchronize' function for 'std::atomic_flag'.
+inline Synchronized<std::atomic_flag> synchronize(std::atomic_flag* lock)
+{
+  return Synchronized<std::atomic_flag>(
+    lock,
+    [](std::atomic_flag* lock) {
+      while (lock->test_and_set(std::memory_order_acquire)) {}
+    },
+    [](std::atomic_flag* lock) {
+      lock->clear(std::memory_order_release);
+    }
+  );
+}
+
+
+// An overload of the 'synchronize' function for 'pthread_mutex_t'.
+inline Synchronized<pthread_mutex_t> synchronize(pthread_mutex_t* mutex)
+{
+  return Synchronized<pthread_mutex_t>(
+    mutex,
+    [](pthread_mutex_t* mutex) {
+      pthread_mutex_lock(mutex);
+    },
+    [](pthread_mutex_t* mutex) {
+      pthread_mutex_unlock(mutex);
+    }
+  );
+}
+
+
+template <typename T>
+T* synchronized_get_pointer(T** t)
+{
+  return *CHECK_NOTNULL(t);
+}
+
+
+template <typename T>
+T* synchronized_get_pointer(T* t)
+{
+  return t;
+}
+
+
+// Macros to help generate "unique" identifiers for the
+// synchronization variable name and label. The line number gets
+// embedded which makes it unique enough, but not absolutely unique.
+// It shouldn't be a problem however, since it's very unlikely that
+// anyone would place multiple 'synchronized' blocks on one line.
+#define SYNCHRONIZED_PREFIX CAT(__synchronizer_, __LINE__)
+#define SYNCHRONIZED_VAR CAT(SYNCHRONIZED_PREFIX, _var__)
+#define SYNCHRONIZED_LABEL CAT(SYNCHRONIZED_PREFIX, _label__)
 
 
 // A macro for acquiring a scoped 'guard' on any type that can satisfy
-// the 'Synchronized' interface. Currently this includes 'std::mutex',
-// 'std::recursive_mutex' and 'std::atomic_flag'.
-// Example:
-//   std::mutex m;
-//   synchronized (m) {
-//     // Do something under the lock.
-//   }
-#define synchronized(m)                                                 \
-  if (auto __ ## __file__ ## _ ## __line__ ## __lock = Synchronized<typename std::remove_pointer<decltype(m)>::type>(&m)) // NOLINT(whitespace/line_length)
+// the 'Synchronized' interface. We support 'std::mutex',
+// 'std::recursive_mutex' and 'std::atomic_flag' by default.
+//
+//   Example usage:
+//     std::mutex m;
+//     synchronized (m) {
+//       // Do something under the lock.
+//     }
+//
+// You can easily extend support for your synchronization primitive
+// here, by overloading the 'synchronize' function.
+//
+//   Example overload:
+//     inline Synchronized<bufferevent> synchronize(bufferevent* bev)
+//     {
+//       return Synchronized<bufferevent>(
+//         bev,
+//         [](bufferevent* bev) { bufferevent_lock(bev); },
+//         [](bufferevent* bev) { bufferevent_unlock(bev); },
+//       );
+//     }
+//
+// How it works: An instance of the synchronization primitive is
+// constructed inside the 'condition' of if statement. This variable
+// stays alive for the lifetime of the block. The trick with 'goto',
+// 'else' and the label allows the compiler to figure out that the
+// synchronized block will always execute. This allows us to return
+// within the synchronized block and avoid the
+// 'control reaches the end of a non-void function' warning.
+// Note that the variable declared inside the 'condition' of an if
+// statement is guaranteed to live through the 'else' clause as well.
+//
+// From Section 6.4.3:
+//   A name introduced by a declaration in a condition (either
+//   introduced by the decl-specifier-seq or the declarator of the
+//   condition) is in scope from its point of declaration until the
+//   end of the substatements controlled by the condition.
+#define synchronized(m)                                                     \
+  if (Synchronized<typename std::remove_pointer<decltype(m)>::type>         \
+        SYNCHRONIZED_VAR = ::synchronize(::synchronized_get_pointer(&m))) { \
+    goto SYNCHRONIZED_LABEL;                                                \
+  } else SYNCHRONIZED_LABEL:
 
 #endif // __STOUT_SYNCHRONIZED_HPP__