mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Park" <mcyp...@gmail.com>
Subject Re: Review Request 35395: Improvements to Synchronized.
Date Fri, 12 Jun 2015 23:00:14 GMT


> On June 12, 2015, 6:21 p.m., Ben Mahler wrote:
> > Are (1) and (2) independent changes?
> > 
> > Any performance implications from (1)? A benchmark for 'synchronized' would be great!

> Are (1) and (2) independent changes?

Yeah, they're more or less independent.
> Any performance implications from (1)? A benchmark for 'synchronized' would be great!

Here's a benchmarking test:

```cpp
#include <cstdio>
#include <mutex>

#if SWITCH
struct Synchronized
{
  Synchronized(std::mutex* m) : m_(m) {
    m_->lock();
  }
  ~Synchronized() {
    m_->unlock();
  }
  explicit operator bool() const { return true; }
  std::mutex* m_;
};
#else
struct Synchronized
{
  Synchronized(std::mutex* m,
               void (*acquire)(std::mutex*),
               void (*release)(std::mutex*))
      : m_(m), release_(release) {
    acquire(m_);
  }
  ~Synchronized() { release_(m_); }
  explicit operator bool() const { return true; }
  std::mutex* m_;
  void (*release_)(std::mutex*);
};

Synchronized synchronize(std::mutex* m) {
  return {
    m,
    [](std::mutex* m) { m->lock(); },
    [](std::mutex* m) { m->unlock(); }
  };
}
#endif

int main() {
  std::mutex m;
  int x = 0;
  for (int i = 0; i < 1000000; ++i) {
#if SWITCH
    if (Synchronized lock{&m})
#else
    if (Synchronized lock = synchronize(&m))
#endif
    {
      ++x;
    }
  }
  std::printf("%d\n", x);
}
```

__Before__:

Compile: `clang++ -std=c++11 -O2 -DSWITCH=1 synchronized_benchmark.cpp`

Callgrind: `valgrind --tool=callgrind ./a.out`
```
1000000
==25389==
==25389== Events    : Ir
==25389== Collected : 302332786
==25389==
==25389== I   refs:      302,332,786
```

__After__:

Compile: `clang++ -std=c++11 -O2 -DSWITCH=0 synchronized_benchmark.cpp`

Callgrind: `valgrind --tool=callgrind ./a.out`
```
1000000
==25514==
==25514== Events    : Ir
==25514== Collected : 302333405
==25514==
==25514== I   refs:      302,333,405
```


- Michael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35395/#review87737
-----------------------------------------------------------


On June 12, 2015, 10:01 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35395/
> -----------------------------------------------------------
> 
> (Updated June 12, 2015, 10:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> (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;
>   }
> }
> ```
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp 60eaf263f220b4990aefe4e5d6d2aa1296891e57

> 
> Diff: https://reviews.apache.org/r/35395/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message