3
\$\begingroup\$

I want to improve my multithreading competence and tried to build a compact doublebuffered way to separate two threads where one is only displaying the state and the other one updates the state. So I am here to hear whether I done it "right" and what to improve.

#include <thread>
#include <mutex>
#include <iostream>
#include <fstream>

template <class T> class StateBuffer
{
public:
    typedef T value_t;

    void Swap()
    {
        {
            std::lock_guard<std::mutex> _(swapMutex);
            index = 1 - index;
        }
        Write() = Read();
    }
    value_t& Write()
    {
        std::lock_guard<std::mutex> _(swapMutex);
        return states[index];
    }
    const value_t Read()
    {
        std::lock_guard<std::mutex> _(swapMutex);
        return states[1-index];
    }
    const value_t&& ReadRef()
    {
        std::lock_guard<std::mutex> _(swapMutex);
        return std::ref(states[1-index]);
    }
private:
    mutable std::mutex swapMutex;
    T states[2];
    uint8_t index = 0; // index = write, 1-index = read
};

struct state_t
{
    int count = 0;
    bool stop = false;
};
typedef StateBuffer<state_t> TestStateBuffer;
TestStateBuffer state;

auto constexpr NO_TIME = std::chrono::seconds(0);

void r()
{
    std::ofstream log("log.txt");
    bool run = true;
    while (run)
    {
        state_t display = state.Read();

        log << display.count << "\n";
        run = !display.stop;

        std::this_thread::sleep_for(NO_TIME);
    }
    log.flush();
}

void u()
{
    bool run = true;

    while (run)
    {
        state_t& write = state.Write();

        run = !write.stop;
        write.stop = ++write.count > 1000000;

        state.Swap();
        std::this_thread::sleep_for(NO_TIME);
    }
}

int main(int, char**)
{
    std::thread render(r);
    std::thread update(u);

    update.join();
    render.join();

    std::cout << "ende" << std::endl;
    std::cin.get();

    return 0;
}

Yes, this is intended to find use in gamedev.

Questions that have gone through my mind:

  1. I use sleep(0) to force a thread swap. Do I need this?
  2. Is there a way how to solve this with a conditional_variable as I read that locking a mutex is expensive in comparison to check with a conditional_variable (although one still need a mutex)?
  3. As I am using references is this really safe?
  4. Have I left anything out?

Environment: Windows 7 x64 g++ on MinGW / Mint 17 x64 clang++ (both C++11 enabled)

\$\endgroup\$

2 Answers 2

1
\$\begingroup\$
  1. No. Using sleep is the wrong way in any most cases that you can think of. Use the correct functions for this. The C++ standard does not contain a standardized way of doing so. Please use SwitchToThread for Windows and pthread_yield for Unix. To gain more information, you may ask a search engine and for context switching, this is a more common identifier than swapping.
  2. Yes, you are right. Mutual exclusion is indeed slower than a condition variable, whereas it is not always replaceable. Since I don't know how your compiler implements mutexes, you may want to search for differences between mutex and critical sections on Windows. I don't know if there are two different ways also on Unix and other systems. But to answer your question: Yes, one can solve this with condition variables. The difference is very-well stated here.
  3. References are as safe as pointers, since all references are pointers internally after the compiler has done his work. Just be sure not to pass references, that you are expecting to own a resource exclusively, to functions which don't own the resource at all. This leads to very unpredictable code, as wrong coding in multi threaded environments always does.
  4. Seems to be okay. But don't count on my word in the case of the move-operator. I simply don't know if this is going to be valid.
\$\endgroup\$
2
  • \$\begingroup\$ It will be a bit difficult to use the thread/mutex-implementation and guess what implementation it is, but I'll try. The linked article is interesting and I will give it a try, whether I can make it an (optional) monitor. Ok, I remove the reference and refref(&&) parts so one will always have to copy to ensure the validity of the buffered data. \$\endgroup\$ Sep 9, 2014 at 19:42
  • 2
    \$\begingroup\$ there is a yield-function defined for c++11. I must admit I did not find it by searching for context switching. \$\endgroup\$ Sep 10, 2014 at 8:47
1
\$\begingroup\$

Why not use an atomic counter instead of a mutex for StateBuffer? Your double-buffer seems like a perfect case for the atomic integers.


struct state_t { ... }

The is one minor issue here. The _t suffix is reserved for future use by the POSIX standard. "state" is a fairly common name, so there is a chance of conflict here. I suggest picking another name, preferably without the _t at the end.


Why not make this a using alias?

typedef StateBuffer<state_t> TestStateBuffer;

vs

using TestStateBuffer = StateBuffer<state_t>;

The behavior is the same. The later is just a more modern way of expressing it.


You should replace the sleep_for(0) with std::this_thread::yield().


A variable named _

std::lock_guard<std::mutex> _(swapMutex);

That just doesn't look right... It is too shady for my taste. I would give that variable any simple name like lock:

std::lock_guard<std::mutex> lock(swapMutex);
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for your answer. In the comments of TheWavelength' answer you see I already found the c++11 yield. I use _ as variable name as I don't need to access it but has to keep stored in variable to get not instantly destroyed, but that (as you mentioned) is dependend on taste. The struct name is just for testing purpose but thanks for the hint. Any bigger differences in the comparison typedef vs using or just again taste-dependend? You are right. As I am only locking against the index I could use an atomic for that. \$\endgroup\$ Sep 11, 2014 at 9:43
  • \$\begingroup\$ @JoshuaBehrens, the using alias is just a more modern way of doing the same thing. The behaviour is the same. \$\endgroup\$
    – glampert
    Sep 11, 2014 at 17:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.