> 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
>
>
