10

I am using a few atomic variables, all unsigned int's, and I wanted to collect them into a structure - effectively a POD. However I also want a constructor because my compiler is not quite c++11 (so I have to define my own constructor to create it with initial values).

So originally I had:

// Names are not the real names - this is just for example std::atomic<int> counter1; std::atomic<int> counter2; std::atomic<int> counter3;

And then I was happy to just increment/decrement them as I needed. But then I decided I wanted a few more counters and therefore to put them into a structure:

struct my_counters { int counter1; int counter2; int counter3; // Constructor so that I can init the values I want. my_counters(c1, c2, c3) : counter1(c1), counter2(c2), counter3(c3){;} };

But since I have added a custom constructor this is no longer technically a POD. I was reading other questions regarding this and they where saying that to use std::atomic I need a POD, but other questions I read suggested that the struct needs to be copyable or some such... anyway, I got confused and I want to know if I can safely use my struct my_counters as an atomic type:

std::atomic<my_counters> counters;

And then within various threads:

// Are these operations now still atomic (and therefore safe to use across threads): counters.counter1++; counters.counter2--; counters.counter3 += 4;
Improve this question
9
  • 12
    Atomic struct is different that struct with atomic members. With atomic struct, you have to copy the whole struct for each modification. Commented May 30, 2018 at 10:00
  • 2
    atomic<my_counters> doesn't have a .counter1 member, so counters.counter1++; won't compile. You could implement all 3 modifications with a cmpxchg loop, but a 3-int struct would only be lock-free on a few platforms (like some compilers for x86-64 with lock cmpxchg16b) Commented May 30, 2018 at 10:03
  • 1
    If you have 3 separate atomic objects, don't put them all in the same struct if you want to use them from different threads. Having them all in one cache line will cause false sharing, so threads using counter1 will contend with threads using counter2. (If they're usually all used at the same time, then in the same cache line is good, though.) Commented May 30, 2018 at 10:05
  • 4
    The requirements for the primary template of std::atomic<T> is that T is TriviallyCopyable, not POD. my_counters is TriviallyCopyable Commented May 30, 2018 at 10:38
  • 1
    @code_fodder all PODs are TriviallyCopyable, and colloquially people may say POD when they mean "safe to memcpy" Commented May 30, 2018 at 10:47

2 Answers 2

Reset to default
21

Others have said it, but just for clarity, I think you need this:

struct my_counters { std::atomic<int> counter1; std::atomic<int> counter2; std::atomic<int> counter3; // Constructor so that I can init the values I want. my_counters(c1, c2, c3) : counter1(c1), counter2(c2), counter3(c3){;} };

And then simply:

my_counters counters;

To put it another way, it's the counters that are atomic, not the struct. The struct just serves to group them together and initialise them.

Edit by Peter

If you use these counters from different threads at the same time, you may want to avoid false-sharing contention between threads by putting each counter in a separate cache line. (Typically 64 bytes). You can use C++11 alignas on the members to get your compiler to pad the struct layout, or manually insert some dummy char padding[60] members between each atomic.

Edit by me

Good link about understanding the cache in general here. Worth reading. Intel cache lines seem to be 64 bytes these days, from just a quick bit of googling, but don't quote me.

Another edit by me

A lot has been said in the comments below about the ins and outs of using std::atomic to look after an (arbitrary) class or struct, e.g.

struct MyStruct { int a; int b; }; std::atomic<MyStruct> foo = { };

But the question I have is this: when is this ever useful? Specifically, as ivaigult points out, you can't use std::atomic to mutate individual members of MyStruct in a threadsafe way. You can only use it to load, store or exchange the entire thing and wanting to do that is not that common.

The only legitimate use case I can think of is when you want to be able to share something like (for example) a struct tm between threads in such a way that a thread doesn't ever see it in an inconsistent state. Then, if the struct is small, you might get away without a lock on your particular platform and that is useful. Just be aware of the implications (priority inversion being the most serious, for realtime code) if you can't.

If you do want to share a struct between threads and be able to update individual members in a threadsafe way, then std::atomic doesn't cut it (and nor was it designed to). Then, you have to fall back on a mutex, and in order to do this it is convenient to derive your struct from std::mutex like so:

struct AnotherStruct : public std::mutex { int a; int b; };

And now I can do (for example):

AnotherStruct bar = { }; bar.lock (). bar.a++; bar.b++; bar.unlock ();

This lets you update two (presumably in some way linked) variables in a threadsafe way.

I'm sorry if all this is obvious to the more seasoned campaigners out there but I wanted to clarify things in my own mind. It actually has nothing to do with the OP's question.

Improve this answer
Sign up to request clarification or add additional context in comments.

15 Comments

Actually I had not inferred that I could use each counter as an atomic within a normal struct, nice : ) ... Also I did not quite understand peter's comment exactly with reference to your answer. Do I still need to use padding if I am using the solution that you propose? +1 btw for making this clear to me
The answer to that question depends, actually, on how you are going to access data in shared structs and classes across threads in general. It doesn't apply only to your atomics, although it may be more relevant there. There's an article here about an issue called false sharing. The gist of it is that you can optimise flushing of cache lines by adapting the layout and / or padding of your structs to suit the pattern of access to member variables in your various threads. No more space, sorry
You don't necessarily need to leave the bytes between counters unused, if you want them in separate cache lines. Using it for things that are typically written by the same thread at the same time as the counter is good. (Anything else is potentially bad, though.) Interesting idea to use alignas(); that's great for static (i.e. global) vars, but not free for automatic storage (over-aligning the stack costs a few instructions in the function that creates the object.)
And BTW, no you can't have an atomic struct with atomic members. atomic<int> is a non-POD type (no copy constructor), so and atomic<T> only works for trivially-copyable T. As a hack, you could have a union between this an atomic<struct> and a struct with atomic<int> members. But it's a real hack and not portably safe. How can I implement ABA counter with c++11 CAS?
Sorry, stepped on your edit. Yes, 64-byte lines are widespread. It's not a coincidence that's the max burst-transfer size of DDR1/2/3/4 SDRAM.
|
5

In most cases std::atomic is pointless for structures because you will end up with copying the entire structure for every change:

std::atomic<my_counters> var(1,2,3); my_counters another_var = var.load(); // atomic copying another_var.counter1++; var.store(another_var); // atomic copying

Moreover, load and store are separate operations, so we can't guarantee that var.counter1 is 3 for two threads executing the code above.

Also, if your target CPU doesn't support atomic operations for structures of this size, std::atomic will fallback to using mutex:

#include <atomic> #include <iostream> struct counters { int a; int b; int c; }; int main() { std::atomic<counters> c; std::atomic<int> a; std::cout << std::boolalpha << c.is_lock_free() << std::endl; std::cout << std::boolalpha << a.is_lock_free() << std::endl; return 0; }

Demo

You may see in the demo, that std::atomic<counters> uses a mutex internaly.

So, you would better to have std::atomic<int> as class members, as Paul suggests.

Improve this answer

26 Comments

not-so-fun fact: gcc7 and later on x86-64 report false for is_lock_free on 16-byte atomic objects, but don't use a mutex. libatomic uses lock cmpxchg16b if it's available. is_lock_free() returned false after upgrading to MacPorts gcc 7.3. But yes, there are huge downsides to atomic structs larger than 8 bytes on x86, or larger than the register width on most other architectures. (On x86 in 32-bit mode, 8-byte atomic objects are reasonably efficient with gcc, but not clang.)
@PaulSanders: What's broken? atomic structs? Not if you use them correctly for the rare cases where they're useful. If you mean gcc7/8, no, there are valid reasons for report 16-byte structs as not lock-free. e.g. pure-load and pure-store are slow, and multiple readers contend with each other. So it's lock free but not fast, which is what you usually really want to know. See also Genuinely test std::atomic is lock-free or not. (Using lock cmpxchg16b for a 16-byte load requires having the cache line in M state, not shared).
@PaulSanders: So you wish there was an API for using the hash-table of locks that non-lock-free atomic structs use internally? But how would your .lock() API work for lock-free atomic objects? It wouldn't only be possible to compile it into a lock cmpxchg loop (or Intel TSX transaction) if the compiler can see the .lock() and the .unlock() start and end of the transaction. So maybe the API could take a lambda to be done as an atomic transaction, or something. That might be a good way to let the compiler choose between a mutex vs. a compare_exchange loop depending on the target.
@PeterCordes Maybe 16-byte structures have "huge" downsides compared to 8-byte for some operations, but the gap between "16 byte lock-free structures that sometimes need cmpxchg16 to read them" and "mutex-using structures" can be absolutely huge for algorithms that care. For some algorithms with 16-byte structures that need to sometimes be modified atomically, fast paths may actually only read an 8-byte part of the structure which can be done quickly (current compilers too stupid, however - and wtf is icc doing there?).
People are used to using a mutex per protected object model, but this "global set of shared locks" has a lot of value too, especially for small, common objects.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.