is it ok to use std::atomic with a struct that is POD except that it has a construtor?

Question

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;

Answer 1

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.

Answer 2

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.