Small C++ thread pool

Question

I've made a small thread pool that i'll be using to communicate with a Redis server:

#include "redox.hpp"

redox::Redox rdx;

void my_cool_function()
{
    // do stuff with rdx
}

int main()
{
    rdx.connect("localhost");
    int max_threads = std::thread::hardware_concurrency();
    std::vector<std::thread> pool;

    for (int i = 0; i < max_threads; i++) {
        pool.push_back(std::thread(my_cool_function));
    }

    for (std::thread &thread : pool) {
        thread.join();
    }

    pool.empty();
    rdx.disconnect();

    return 0;
}

Do I need to call delete rdx on the end of my main function? What about the pool var?

Answer 1

I see a number of things that may help you improve your code.

Make sure to #include all required headers

This program refers to std::vector and std::thread but does not include the corresponding headers. Fix that by adding these lines:

#include <thread>
#include <vector>

Check return values

The function rdx.connect() can fail. Your program should check the return value and handle the case in which it does.

Understand your data structures

The std::vector is used appropriately except for the peculiar call to pool.empty() at the end of the code. The purpose for empty() is to check to see if the vector is empty, not to empty the vector. In any case, it's not necessary to empty it since it will automatically be destroyed when it goes out of scope at the end of main().

Understand the thread pool pattern

The question describes this as a "thread pool" but that's not what I see here. It's not a finite set of threads doing some other number of tasks; rather it's one function being called once for each thread. While this is multithreading, it's not a thread pool. To keep from confusing yourself or others, it's useful to use common terms in the usual way.

Avoid concurrency problems

Any time multiple tasks run in parallel, we have to take extra care to assure that things work correctly. In particular, my_cool_function() claims to "do stuff with rdx", but rdx is a single global variable. The code for that function isn't shown, but it's highly likely that some form of locking will be required to avoid problems with concurrent access to the single global object.

Omit return 0 at the end of main

The compiler will automatically generate a return 0; at the end of main so it is not necessary to supply your own.

Answer 2

A thread pool keeps a bunch of threads ready for executing work. The emphasis is that after a thread-pool thread finishes a job, it is reused by the pool to execute new jobs. This avoids the overhead of recreating a new thread every time.

Your code simply spawns a few threads that execute one function and then return. Although it might serve your purpose, I wouldn't go as far as calling it a thread pool.

Speaking of purpose, your code doesn't show why you would like to spawn multiple threads to access the Redis server. What exact issue are you trying to solve? I'm not familiar with the Redis API but perhaps it offers async methods that return futures/promises.