0

I'm having a bit of trouble with this bit:

volatile uint32_t chunk_size;
volatile T* chunk;
std::vector<volatile T*>* chunks;
CRITICAL_SECTION cs;

...

void push(T item) {
    EnterCriticalSection(&cs);

    uint32_t slot = chunk_size++;

    if (slot == chunk_capacity) {
        chunk = new T[chunk_capacity];
        chunks->push_back(chunk);
        chunk_size = 1;
        slot = 0;
    }

    chunk[slot] = item;

    LeaveCriticalSection(&cs);
}

So that should be a simple thread safe push.

I think T* chunk needs to be volatile because I don't want any thread caching the value since one of them might reallocate and change it.

The vector itself shouldn't need to be volatile since even though its contents change, the object itself doesn't move.

The vectors template type I'm not sure about, on one hand c++ won't let me push volatile types so I'd have to cast it to non-volatile. At the same time though, it is also a multiple consumer container and when one of them pops the container, it clears the vector. So if the address of chunks[2] were to be cached for instance, things will definitely go wrong.

So, should the vectors type template be volatile or not?

user81993
  • 6,167
  • 6
  • 32
  • 64
  • 3
    Rule of thumb. I you do not know where volatile is applicable (e.g.: hardware registers) do not use it (it will be wrong) –  Jun 29 '16 at 20:16
  • 1
    You probably want `std::atomic` instead of `volatile`. – Jarod42 Jun 29 '16 at 20:18
  • See: http://stackoverflow.com/questions/2484980/why-is-volatile-not-considered-useful-in-multithreaded-c-or-c-programming –  Jun 29 '16 at 20:21
  • @Jarod42 I definitely don't, I already tried a lock-free implementation and due to having at least 3 atomic operations per push its twice as slow as with cs. Looking at the code over at msdn, they write data to volatiles in critical sections so I'm assuming thats the way to do it. – user81993 Jun 29 '16 at 20:21
  • Why use critical sections instead of `std::mutex` with a lock (e.g., `std::unique_lock`? What is going to happen if `new` in the `push` function throws (or `push_back`)? – James Adkison Jun 29 '16 at 20:24
  • 1
    `volatile` is not necessary in this code. The compiler can see where `chunk` and `chunk_size` are changeable globals and will not produce incorrect code that assumes values. – Donnie Jun 29 '16 at 20:25
  • 1
    If you see someone using `volatile` for thread safety ignore them and move on. You cannot count on it for thread safety in today's world. – NathanOliver Jun 29 '16 at 20:26
  • @JamesAdkison CS is slightly quicker, thats about it. If either of them throw then I crash, there would be no elegant way to handle that anyways considering what the rest of the application is doing. – user81993 Jun 29 '16 at 20:27
  • `volatile` should not be used for thread safety as one thing it does NOT do is ensure CPU cache coherency across multiple CPU's. Extra instructions need to be inserted and that is one effect of using `std::atomic<*>` . See also: http://en.cppreference.com/w/cpp/language/cv - "volatile objects suitable for communication with a signal handler, but not with another thread of execution" – Richard Critten Jun 29 '16 at 21:02

0 Answers0