Reputation: 1642
I have a thread which basically does:
int changed; //global variable
..
for (;;) {
pthread_mutex_lock(&mtx);
if (changed) {
do_changes();
changed = 0;
}
pthread_mutex_unlock(&mtx);
do_stuff();
}
The loop is run a few hundred thousand times a second, while the global changed
variable will be set rarely (a few times a day) by another thread.
With a change to
volatile int changed; //global variable
..
for (;;) {
if (changed) {
pthread_mutex_lock(&mtx);
do_changes();
changed = 0;
pthread_mutex_unlock(&mtx);
}
do_stuff();
}
I can measure 3-4% performance increase of the loop with this approach, which is worth while pursuing.
However volatile variables seems to be heavily discouraged. Are there any drawbacks with the approach here ? Any corner cases that might cause the 2. version to not work as intended ?
Upvotes: 0
Views: 253
Reputation: 3338
Author: However volatile variables seems to be heavily discouraged. Are there any drawbacks with the approach here ? Any corner cases that might cause the 2. version to not work as intended ?
First of all: Whenever you are tempted to use volatile
think again if you don't need atomics. Now see what may happen:
1) It is not safe if you have the loop in multiple threads. You can think about duplicating the check:
if (changed) { // quick check
pthread_mutex_lock(&mtx);
if (changed) { // another thread could do the work
...
2) If your code is critical to see it was changed, you need to use atomics, because that if(changed)
before pthread_mutex_lock
may not see it because of cache.
3) It may work on x86(_64) with strong memory ordering and atomic int accesses, but fail on other architecture. That is the reason why volatile
is discouraged, use atomics (and make a habbit of it). volatile
does not force atomic usage or any other synchronization. Atomics do (read-modify-write instructions).
std::atomic_flag validated;
std::mutex mx; struct MyData { ... } data;
void change() {
lock_guard<mutex> lock(mx);
data.something();
validated.clear();
}
void validate() {
if(!validated.test_and_set()) {
lock_guard<mutex> lock(mx);
data.update();
}
}
NOTE: You will never know for sure if data is valid or not unless you hold the lock and use another variable for it.
4) Just try your original code with pthread_spinlock_t
5) Little advice: don't play God with synchronization unless you really know, what you are doing. You can switch from mutex to spinlock (written by somebody else) and do some benchmarking.
About edits and comments: The original answer just started with 1) nothing before. As it turned out, some people neither read the full question nor the full answer. Pitty those quick downvoters. This site is not facebook and those votes should be eihter is helpful or is not helpful, these up/downvotes are no likes and dislikes like on facebook! I may disagree with some parts of the other answers, but still think they are helpful, although incomplete (not aswering the full question, but only a part of it) or partially incorrent (there is nothing bad in writing same value to variable from many threads if we know that we can do that without problems).
Upvotes: 1
Reputation: 136256
volatile
does not make your variable thread safe or atomic. You may like to use C11 atomics for that.
You basically have two threads changing changed
variable thus overwriting previous values causing data race.
I cannot recommend enough watching atomic Weapons: The C++ Memory Model and Modern Hardware. (It applies to C as well).
Upvotes: 2
Reputation: 1493
If you are using a C11 environment that support them you can use atomic variables. If your system supports it, they use special instructions to achieve atomicity, instead of locks. If your system don't support that, they use locks (the flag type is always lockless).
If you don't have C11, but you have a GCC-compatible compiler see the sync family of functions. It's similar (but older) to the C11 atomic variables but if your system don't support them they generate a function call.
Upvotes: 1
Reputation: 129
It is a good idea to make it volatile to ensure that the compiler is not doing unwanted optimizations, but it doesn't make it atomic.
If there are more threads reading "changed" it could happen that one thread updates "changed" to 0 when there's another thread waiting to execute do_changes(), which will happen once the mutex is released because the condition was already evaluated.
If you want to avoid this then move the if statement inside the mutex protected space.
Hope this helps.
Carles.
Upvotes: -1