C++ class with thread - destructor, constructor, move constructor?

Question

I am going through the book Concurrency in action for C++. There is an example there of a class called scoped_thread (pg 27), which ensures the RAII idiom and the thread has joined before its respective end of scope.

This current example does not allow it to be used with functions that best requires the move operator ,such as the emplace_back member function for vector (?). This way you can call the class constructor and possibly allow better optimization than push_back. As such, I wanted to add move constructor in class scoped_thread.

Before I present the material, the two questions are as follows

1. What's the reason for the non-parallel constructor and move constructor calls? how to reduce them?
2. Is my move constructor correct?
3. I am now calling joinable() two times. Originally it was in the constructor. But since I created the move constructor, I've had to check for joinable() in the destructor as well. This may tie in with question 2 if my move constructor is not good

See EDIT at bottom of page also before continuing

Without further ado, here is my class code (link provided below for full code in online compiler)

CLASS SCOPED_THREAD

#define PRINT(X) std::cout << X << std::endl;

static unsigned dtor_count = 0;
static unsigned ctor_count = 0;
static unsigned mtor_count = 0;

class scoped_thread {
    thread t;
public:
    explicit scoped_thread(thread t_) :
        t(std::move(t_))
    {
        if (!t.joinable())
            throw std::logic_error("No thread!");
        PRINT("Called scoped_thread CTor");
        ctor_count++;
    }
    ~scoped_thread()
    {
        if (t.joinable())
            t.join();
        PRINT("Called scope_thread DTor");
        dtor_count++;
    }
    scoped_thread(const scoped_thread&) = delete; // copy CTor
    scoped_thread& operator=(const scoped_thread&) = delete; // copy init
    scoped_thread(scoped_thread&& s) {
        t = std::move(s.t);
        mtor_count++;
    };
};

The static variables are used as a counter for the number of calls to the constructor, destructor and move constructor.

Here is an example use. (Note: the commented section is what could have been achieved without this class)

EXAMPLE USE

void do_work(int i) { PRINT("Made Thread : " << i); };
void thread_vector_example()
{
    // normal version: have to call thread join
    /*
    std::vector<std::thread> threads;
    for (unsigned i = 0; i < 10; ++i)
        threads.push_back(std::thread(do_work, i));
    std::for_each(begin(threads), end(threads),
        std::mem_fn(&std::thread::join));   
    */

    std::vector<scoped_thread> sthreads;
    for (unsigned i = 0; i < 10; ++i) {
        sthreads.emplace_back(std::thread(do_work, i));
    }   
}   

1. From the g++ compiler (link given below) with options g++ -std=c++14 -O2 -Wall -pthread the results are as follows

   • constructors called: 10
   • destructors called: 25
   • move constructors called: 15

2. From the Visual Studio C++ compiler (standard options)

   • constructors called: 10
   • destructors called: 35
   • move constructors called: 25

link to file--> http://coliru.stacked-crooked.com/a/33ce9f3daab4dfda

EDIT

After calling the reserve function i.e. sthreads.reserve(10), I can have a more better version where it calls constructor and destructor 10 times only. However, there still is an issue when I remove the move constructor from the class, the code will not compile

Answer 1

1. What's the reason for the non-parallel constructor and move constructor calls? how to reduce them?

The reason for 10 constructor calls is that you construct 10 objects...

As you've already figured out, the reason for move constructor calls is the reallocation of vector's internal storage due to exceeding the capacity and you can get rid of them by reserving enough capacity before hand.

2. Is my move constructor correct?

Yes. Once you stop debugging the constructor counts, scoped_thread(scoped_thread&& s) = default; should suffice.

I am now calling joinable() two times. Originally it was in the constructor. But since I created the move constructor, I've had to check for joinable() in the destructor as well. This may tie in with question 2 if my move constructor is not good

The possibility of being moved from does require the test in the destructor. But since it's now tested in the destructor, there isn't any point in testing it in the constructor in my opinion.

However, there still is an issue when I remove the move constructor from the class, the code will not compile

Even though you use the vector in a way that causes no moves (reserve space and emplace new objects), the objects must nevertheless be movable. The compiler cannot simply check that you never exceed the capacity during run time and accordingly not generate the code for reallocation which depends on the type being movable or copyable.

Here's a bonus idea since you want to avoid moves. Don't move the thread into the constructor but construct it in-place instead (You may still want to provide the construct that moves a thread):

template<class Fun, class... Args> 
explicit scoped_thread(Fun&& f, Args&&... args):
t(std::forward<Fun>(f), std::forward<Args>(args)...)
{}

//...
sthreads.emplace_back(do_work, i);