Re: Review Request 35395: Improvements to Synchronized.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35395/
---
(Updated June 13, 2015, 12:51 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 Synchronizedbufferevent
{
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
Synchronizedbufferevent 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 (updated)
-
3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp
60eaf263f220b4990aefe4e5d6d2aa1296891e57
Diff: https://reviews.apache.org/r/35395/diff/
Testing
---
`make check`
Thanks,
Michael Park
Re: Review Request 35395: Improvements to Synchronized.
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 100; ++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`
```
100
==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`
```
100
==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 Synchronizedbufferevent
{
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
Synchronizedbufferevent 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
Re: Review Request 35395: Improvements to Synchronized.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35395/#review87737
---
Are (1) and (2) independent changes?
Any performance implications from (1)? A benchmark for 'synchronized' would be
great!
- Ben Mahler
On June 12, 2015, 5:06 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, 5:06 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 Synchronizedbufferevent
{
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
Synchronizedbufferevent 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
Re: Review Request 35395: Improvements to Synchronized.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35395/
---
(Updated June 12, 2015, 6:24 p.m.)
Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
Changes
---
Add documentation and address formatting issues.
Repository: mesos
Description
---
(1) Simplify introducing new synchronization primitives.
Before:
```cpp
template
class Synchronizedbufferevent
{
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
Synchronizedbufferevent 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 (updated)
-
3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp
60eaf263f220b4990aefe4e5d6d2aa1296891e57
Diff: https://reviews.apache.org/r/35395/diff/
Testing
---
`make check`
Thanks,
Michael Park
