HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Thread-safe game engine: multi-threading best practices?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
enginemultithreadinggamethreadpracticessafebest

Problem

I'm writing a multithreaded game engine, and I'm wondering about best practices around waiting for threads. It occurs to me that there could be much better options out there than what I've implemented, so I'm wondering what you think.

-
wait() method gets called at the top of every other method in the class. This is my current implementation, and I'm realizing it's not ideal.

class Texture {
public:
    Texture(const char *filename, bool async = true);
    ~Texture();

    void Render();

private:
    SDL_Thread *thread;
    const char *filename;

    void wait();
    static int load(void *data);
}

void Texture::wait() {
    if (thread != NULL) {
        SDL_WaitThread(thread, NULL);
        thread = NULL;
    }
}

int Texture::load(void *data) {
    Texture *self = static_cast(data);

    // Load the Image Data in the Thread Here...

    return 0;
}

Texture::Texture(const char *filename, bool async) {
    this->filename = filename;
    if (async) {
        thread = SDL_CreateThread(load, NULL, this);
    } else {
        thread = NULL;
        load(this);
    }
}

Texture::~Texture() {
    // Unload the Thread and Texture Here
}

void Texture::Render() {
    wait();

    // Render the Texture Here
}


-
Convert the wait() method in to a function pointer. This would save my program from a jmp at the top of every other method, and simply check for thread != NULL at the top of every method. Still not ideal, but I feel like the fewer jumps, the better. (I've also considered just using the inline keyword on the function... but would this include the entire contents of the wait function when all I really need is the if (thread != NULL) check to determine whether the rest of the code should be executed or not?)

-
Convert all of the class' methods in to function pointers, and ditch the whole concept of calling wait() except while actually loading the texture. I see advantages and disadvantages to this approach... namely, this feels the most diffi

Solution

I'm wondering about best practices around waiting for threads.

That's too big a question. Instead I'll just review your existing code.


This is my current implementation, and I'm realizing it's not ideal.

You construct this with a bool async parameter, so that it loads synchronously or asynchronously. There are (at least) two alternatives.

One is to make two derived subclasses:

class Texture
{
    ... etc ...
    virtual ~Texture() {}
protected:
    Texture(const char *filename)
    {
        this->filename = filename;
    }
    static int load(void *data);
private:
    virtual void wait() = 0;
    const char *filename;
}

class SyncTexture : public Texture {
public:
    SyncTexture(const char *filename)
    : Texture(filename)
    {
        load(this);
    }
private:
    virtual void wait() {} // do nothing
};

class AsyncTexture : public Texture {
public:
    AsyncTexture(const char *filename)
    : Texture(filename)
    {
        thread = SDL_CreateThread(load, NULL, this);
    }
private:
    SDL_Thread *thread;
    virtual void wait() { SDL_WaitThread(thread, NULL); }
};


The above is similar to what you might have been suggesting when you talked about a "function pointer": i.e. wait is 'virtual' and called through a so-called vtable.

The second possibility is to do it using templates:

template 
class Texture {
public:
    Texture(const char *filename);
};


This may be faster at run-time, because the value of async is known/fixed at compile-time.


Convert the "wait()" method in to a function pointer.

I don't understand what you're thinking of when you say this, and you haven't provided the source code for it.


I've also considered just using the "inline" keyword on the function... but would this include the entire contents of the wait function when all I really need is the "if (thread != NULL)" check to determine whether the rest of the code should be executed or not?

I guess that wait is small and simple enough to be inlined.

The "rest of the code" is only the following ...

{
    SDL_WaitThread(thread, NULL);
    thread = NULL;
}


... and it wouldn't be called if thread is null: so what are you worrying about: the increase in size of your software? I doubt you have enough methods (places where you call wait), such that inlining them would make significant difference to the size of your code.

Code Snippets

class Texture
{
    ... etc ...
    virtual ~Texture() {}
protected:
    Texture(const char *filename)
    {
        this->filename = filename;
    }
    static int load(void *data);
private:
    virtual void wait() = 0;
    const char *filename;
}

class SyncTexture : public Texture {
public:
    SyncTexture(const char *filename)
    : Texture(filename)
    {
        load(this);
    }
private:
    virtual void wait() {} // do nothing
};

class AsyncTexture : public Texture {
public:
    AsyncTexture(const char *filename)
    : Texture(filename)
    {
        thread = SDL_CreateThread(load, NULL, this);
    }
private:
    SDL_Thread *thread;
    virtual void wait() { SDL_WaitThread(thread, NULL); }
};
template <bool async = true>
class Texture {
public:
    Texture(const char *filename);
};
{
    SDL_WaitThread(thread, NULL);
    thread = NULL;
}

Context

StackExchange Code Review Q#25269, answer score: 3

Revisions (0)

No revisions yet.