24

Given the following example code:

int var;
int mvar;
std::mutex mvar_mutex;

void f(){
    mvar_mutex.lock();
    mvar = var * var;
    mvar_mutex.unlock();
}

I want to express that mvar_mutex is bound to the variable mvar and protects only that variable. mvar_mutex should not protect var because it is not bound to it. Hence the compiler would be allowed to transform the above code into the below code:

int var;
int mvar;
std::mutex mvar_mutex;

void f(){
    int r = var * var; //possible data race created if binding is not known
    mvar_mutex.lock();
    mvar = r;
    mvar_mutex.unlock();
}

This might reduce contention on the lock as less work is being done while holding it.

For int this can be done using std::atomic<int> mvar; and removing mvar_mutex, but for other types such as std::vector<int> this is not possible.

How do I express the mutex-variable binding in a way that C++ compilers understand it and do the optimization? It should be allowed to reorder any variable up or down across mutex boundaries for any variable that is not bound to that mutex

Since the code is being generated using clang::ASTConsumer and clang::RecursiveASTVisitor I am willing to use non-standard extensions and AST manipulations as long as clang (ideally clang 4.0) supports them and the resulting code does not need to be elegant or human-readable.

Edit since this seems to be causing confusion: The above transformation is not legal in C++. The described binding of mutex to variable doesn't exist. The question is about how to implement that or achieve the same effect.

nwp
  • 9,623
  • 5
  • 38
  • 68
  • 1
    Are you sure that compiler is allowed to move the multiplication before the lock? It shouldn't do it since the multiplication is also protected by the lock. If compiler is allowed to move in such way then lots of codes should break down. – taskinoor Jul 04 '17 at 11:26
  • 1
    @taskinoor No, it isn't allowed to. But that is what OP wants to achieve. The compiler should be signalled that it should only protect `mvar`. – Hatted Rooster Jul 04 '17 at 11:28
  • 5
    use a `lock_guard` or `unique_lock` instead of manually locking and unlocking the lock. – Paul Rooney Jul 04 '17 at 11:28
  • 1
    @taskinoor It isn't allowed. I want to allow it. I don't know how to do that. – nwp Jul 04 '17 at 11:30
  • Oh okay. I misunderstood the question. Thanks for clarifying. – taskinoor Jul 04 '17 at 11:31
  • What about wrapping the complex type in a pointer and making that atomic? – Hatted Rooster Jul 04 '17 at 11:46
  • @RickAstley That doesn't prevent race conditions on the object. Unless one swaps the pointer for a `nullptr` first and adds code that retries swapping when a `nullptr` was read. That might actually be viable. – nwp Jul 04 '17 at 11:51
  • I was about to improve on that comment until I read your solution. I think that'd be viable indeed. – Hatted Rooster Jul 04 '17 at 12:09
  • 1
    I am not sure you thought this through. Every sensible mechanism that I can think of will change semantics in some way that is potentially wrong as soon as I slightly extend the example you have given to more than a single isolated write. You would at least need to provide some more details about how exactly you envision this optimization to behave to allow for a reasonable answer. – ComicSansMS Jul 11 '17 at 13:00
  • 2
    @ComicSansMS An alternative way of expressing it is this: `mutex.lock` creates a barrier where code can move down, but not up. `mutex.unlock` creates a barrier where code can move up, but not down. (barriers cannot cross barriers). I want to modify it so the barriers only apply to the one specific object and other code can be moved around freely. For example replacing the `mutex.lock` with `acquire_barrier();` and `mutex.unlock` with `release_barrier();` would be an acceptable solution if those functions were implementable with the desired reordering behavior. – nwp Jul 11 '17 at 13:11
  • 4
    @nwp Please, explain why do need this. In real scenarios, `std::atomic` and lock-free algorithms should fulfill your needs. – Andrey Nasonov Jul 12 '17 at 23:14
  • @AndreyNasonov The goal is to implement something like synchronized variables. They use some synchronization mechanism such as a mutex, unlike unsynchronized variables that don't. Ideally that would produce a system that is easier to understand and possibly more efficient. However, any implementation for the synchronization I come up with also synchronize unsynchronized variables because from a compiler's point of view there is no link between mutex and variable. – nwp Jul 12 '17 at 23:57
  • Why don't you just use the structure in your second example? That conveys the intent of what you want, and it works, doesn't it? – edA-qa mort-ora-y Jul 16 '17 at 17:09
  • @edA-qamort-ora-y The rewrite works for this example. As soon as `f` calls `bar();` it cannot be done anymore, because I have to call `bar` before, during or after the lock. The compiler is then not allowed to move the function call (except into the lock which is generally bad) because of the mutex. The mutex puts restraints on the code that I don't want it to have, so I'm looking for a more fine-grained mutex. – nwp Jul 16 '17 at 19:40
  • @nwp I'm still not quite sure I follow. Can you show code where the second structure doesn't work for your? I've done quite a lot of work with mutexes and fences, and generally the only way to get stuff not to be done inside th sensitive section is to not put it there. – edA-qa mort-ora-y Jul 16 '17 at 21:04
  • 1
    I suspect your theoretical performance gains will always be eaten by the cost of crazy machinery around it until there is a native compiler support for such an idiom. – MK. Jul 17 '17 at 18:37
  • 1
    I think there's a slight misunderstanding of what a mutex is. Mutexes don't protect memory, they protect threads. You don't really have a choice here other than to use atomics. Also, you should really use a `lock_guard`. – Qix - MONICA WAS MISTREATED Jul 17 '17 at 20:37
  • Does this even make sense? Assuming you had a way to protect ONLY mvar and you had a statement like a = protected(mvar) + 1; you have no guarantee of the relationship between mvar and a! `mvar` could change between the time you fetch it before you even add 1 and change again before the results of the addition were stored in `a`. I think what you are looking for is `atomic`, so you can say memvar++ and be guaranteed that memvar will be incremented safely. – Dale Wilson Jul 17 '17 at 20:50
  • @Qix My understanding is that mutexes protect objects from race conditions. Race conditions are defined as 2 threads accessing the same memory location at the same time where at least one is writing. I'm not stopping all but 1 thread, I'm restricting access to certain memory locations, so I'd say mutexes protect memory. Using atomics is probably the only way to get this to work, but so far not one answer uses atomics at all. – nwp Jul 17 '17 at 21:20
  • @DaleWilson You are right in that there are none of the guarantees you mentioned. That is how I want it to be. Yes, I'm probably looking for atomics in some way, but they must work for any type, such as `std::vector`, while `std::atomic` is not allowed. I suspect that one can make a solution that uses `std::atomic` somehow, but my attempts didn't get me very far. – nwp Jul 17 '17 at 21:22
  • @nwp No. Mutexes do not protect memory. At all. Think of mutexes like flags. If a thread tries to set the "flag" of a mutex, but the mutex is already "flagged", then the second thread will wait until the original thread has removed the "flag". That's all mutexes do. From that, you get the benefit of being able to lock parts of memory by always locking a mutex when you access a variable. – Qix - MONICA WAS MISTREATED Jul 17 '17 at 21:35
  • Ahh.. I think your use of an `int` as the protected 'object' is confusing things. Make it a more complex object and your purpose becomes more clear. Although I agree with the comment that this will have little practical value (you'll be hard pressed to measure a performance difference) it is an interesting intellectual challenge. – Dale Wilson Jul 17 '17 at 21:36
  • @nwp: You cannot do this in current C++. I think that you should first decide, which do you need exactly: an atomic access to an object, or a special mutex which can be bound to a variable (these are similar, but differ a little bit). Then you should ask a specific question, that how you can achieve this design with modifying/extending clang. Maybe you can try putting this question in a clang developer list too. – geza Jul 18 '17 at 10:40
  • "_For `int` this can be done using `std::atomic mvar;`_" Not even for a single `int` is `atomic` a complete replacement as mutexes work together with condition variables, as they are needed to safely test then wait then test a signaled condition. – curiousguy Dec 09 '19 at 01:15

6 Answers6

11

If you wish to achieve that the std::mutex will only be held until an operation is performed on the protected object, you can write a wrapper class as follows:

#include <cstdio>
#include <mutex>

template<typename T>
class LockAssignable {
public:
    LockAssignable& operator=(const T& t) {
        std::lock_guard<std::mutex> lk(m_mutex);
        m_protected = t;
        return *this;
    }
    operator T() const {
        std::lock_guard<std::mutex> lk(m_mutex);
        return m_protected;
    }    
    /* other stuff */
private:
    mutable std::mutex m_mutex;
    T m_protected {};
};

inline int factorial(int n) {
    return (n > 1 ? n * factorial(n - 1) : 1);
}

int main() {
    int var = 5;
    LockAssignable<int> mvar;

    mvar = factorial(var);
    printf("Result: %d\n", static_cast<int>(mvar));
    return 0;
}

In the example above the factorial will be calculated in advance and the m_mutex will be acquired only when the assignment or the implicit conversion operator being called on mvar.

Assembly Output

Akira
  • 4,385
  • 3
  • 24
  • 46
  • The reason why the example doesn't satisfy the requirement is that `printf` and whatever it does is being protected by `mvar_mutex`. It should only protect `mvar`. The compiler should be allowed to move the `printf` outside of the `lock` block, but it isn't with this implementation. Same with the wrapper class. The `mutex` it uses not only protects `m_protected` but also synchronizes code around it, which it should not do. – nwp Jul 13 '17 at 08:56
  • @nwp, I edited my first example due to the `printf` was only there for feedback purposes. The `int r = factorial(var)` part was which I mean that is outside of the critical section protected by `std::mutex`. – Akira Jul 13 '17 at 09:00
  • Now there is an unprotected read on `mvar`. You can change it to `printf("Result: %d\n", r);` to fix that, but there is still the issue that the `printf` should be allowed to move up above the `lock` block, but the `mutex` prevents that even though there is no `mvar` in the `printf` anymore. Also the point is to not do all reordering that the compiler is supposed to do manually. The goal is to allow the compiler to rearrange code as it sees fit. – nwp Jul 13 '17 at 09:03
8

For the primitive data types you can use std::atomic with std::memory_order_relaxed. The documentation states that:

there are no synchronization or ordering constraints imposed on other reads or writes, only this operation's atomicity is guaranteed

In the following example, the atomicity of the assignation is guaranteed, but the compiler should be able to move the operations.

std::atomic<int> z = {0};
int a = 3;
z.store(a*a, std::memory_order_relaxed);

For objects, I thought of several solutions, but:

  • There is no standard way to remove ordering requirements from std::mutex.
  • It is not possible to create a std::atomic<std::vector>.
  • It is not possible to create a spinlock using std::memory_order_relaxed (see the example).

I have found some answers that state that:

  • If the function is not visible in the compilation unit, the compiler generates a barrier because it does not know which variables it uses.
  • If the function is visible and there is a mutex, the compiler generates a barrier. For example, see this and this

So, in order to express that mvar_mutex is bound to the variable, you can use some classes as stated by the other answers but I do not think it is possible to fully allow the reordering of the code.

J. Calleja
  • 4,855
  • 2
  • 33
  • 54
3

How about a locked var template ?

template<typename Type, typename Mutex = std::mutex>
class Lockable
{
public:
   Lockable(_Type t) : var_(std::move(t));
   Lockable(_Type&&) = default;
   // ...  could need a bit more

   T operator = (const T& x) 
   { 
      std::lock_guard<Lockable> lock(*this);
      var_ = x;
      return x;
   }

   T operator *() const
   { 
      std::lock_guard<Lockable> lock(*this);
      return var_;
   }

   void lock() const   { const_cast<Lockable*>(this)->mutex_.lock(); }
   void unlock() const { const_cast<Lockable*>(this)->mutex_.unlock().; }
private:
  Mutex mutex_;
  Type var_;
};

locked by assignment operator

Lockable<int>var;
var = mylongComputation();

Works great with lock_guard

Lockable<int>var;
std::lock_guard<Lockable<int>> lock(var);
var = 3;

Practical on containers

Lockable<std::vector<int>> vec;

etc...

Michaël Roy
  • 6,338
  • 1
  • 15
  • 19
3

I want to express that mvar_mutex is bound to the variable mvar and protects only that variable.

You can't do this. A mutex actually guards the critical region of machine instructons between the acquisition and release. Only by convention is that associated with a particular instance of shared data.

To avoid doing unnecessary steps inside the critical region, keep the critical regions as simple as possible. In a critical region, only with local variables which the compiler can "see" are obviously not shared with other threads, and with one set of shared data belonging to that mutex. Try not to access other data in the critical region that might be suspected of being shared.

If you could have your proposed language feature, it would only introduce the possibility of error into a program. All it does is take code which is now correct, and make some of it incorrect (in exchange for the promise of some speed: that some code stays correct and is faster, because extraneous computations are moved out of the critical region).

It's like taking a language which already has a nice order of evaluation, in which a[i] = i++ is well defined, and screwing it up with unspecified evaluation order.

Kaz
  • 55,781
  • 9
  • 100
  • 149
  • Obviously weakening guarantees for all C++ programs would generally break C++ programs. I don't want to do that for all C++ programs, just for some of mine. I know that mutex barriers tend to be hardware instructions that are not able to express that some instructions can pass the barrier while others cannot. However, it should be possible to tell that to the compiler which is not bound by the instruction set during optimization phases. I do like this answer though as it is not just "wrap the mutex in some class" but actually tries to answer the question. – nwp Jul 17 '17 at 21:31
  • If you had this, you would break your own C++ programs some of the time thinking you're immune because you invented it. :) :) – Kaz Jul 17 '17 at 21:34
2

You can use folly::Synchronized to make sure that the variable is only accessed under a lock:

int var;
folly::Synchronized<int> vmar;

void f() {
  *mvar.wlock() = var * var;
}
vitaut
  • 49,672
  • 25
  • 199
  • 336
  • 1
    Digging through the [code](https://github.com/facebook/folly/blob/master/folly/Synchronized.h#L874) I'm fairly sure that all that `folly::Synchronized` does is automatically lock and unlock an `std::mutex`. It is thus only a different implementation of `LockAssignable` from Akira's answer and does not allow reordering of code due to the barriers of `std::mutex`. – nwp Jul 14 '17 at 09:04
  • `folly::Synchronized` is more than just `LockAssignable`, but assignment operator is indeed the part most relevant to your example. – vitaut Jul 15 '17 at 12:45
  • 1
    @nwp, you can avoid `std::mutex` with some kind of spinlocks, as the **Boost.Lockfree** library does but with specific types with really fast operations. I wouldn't use a spinlock on `std::vector::push_back` with possible reallocation in the background. – Akira Jul 17 '17 at 20:44
1

I want to express that mvar_mutex is bound to the variable mvar and protects only that variable.

This is not how a mutex works. It doesn't "bind" to anything in order to protect it. You are still free to access this object directly, in complete disregard with any sort of thread safety whatsoever.

What you should do is hide away the "protected variable" so that it is not directly accessible at all, and write an interface that manipulates it that goes through the mutex. This way you ensure that access to the underlying data is protected by that mutex. It can be a single object, it can be a functional group of objects, it can be a collection of many objects, mutexes and atomics, designed to minimize blocking.

dtech
  • 47,916
  • 17
  • 112
  • 190
  • The problem I have is not to protect a variable. `std::mutex` and `std::unique_lock` cover that nicely. The problem is that `std::mutex`, no matter if hidden behind some class or not, will impose restrictions on variables that have nothing to do with the `std::mutex`. – nwp Jul 17 '17 at 22:31
  • Not necessarily - you build the access interface to go through the mutex only when necessary. Maybe you should elaborate on your "restrictions" assumption. – dtech Jul 18 '17 at 07:31
  • An example would be `foo++; std::unique_lock(mvar_mutex); foo--;`. I would expect the compiler to optimize away `foo++;` and `foo--;`, but it cannot, because the semantics of the mutex barriers don't allow that. I want to find a way to allow that. – nwp Jul 18 '17 at 08:04
  • Yeah... how about something that makes practical sense :) For example in the OP `mvar = var * var;` - you can do that, but `mvar` will be a wrapper object that implements the `=` operator, so the CPU will first compute the multiplication and only lock the underlying data for the duration of the assignment. The idea of tucking away the mutex is to not lock and unlock it manually, but have the wrapper object do that for you, only when necessary. – dtech Jul 18 '17 at 09:59