Reusing a unique_lock in a consumer loop

Question

I stumbled over the following code in Bjarne Stroustrup's "The C++ Programming Language, 4th Edition" on page 119:

queue<Message> mqueue;
condition_variable mcond;
mutex mmutex;

void consumer()
{
    while(true) {
        unique_lock<mutex> lck{mmutex};
        mcond.wait(lck);

        auto m = mqueue.front();
        mqueue.pop();
        lck.unlock();
        // process m
    }
}

There is also a producer thread which pushes Message to the queue and notifies the waiting thread in a loop.

My question is: Is it required to create a new unique_lock in every iteration of the loop? It seems unneccesary to me, because in the next line, mcond.wait(lck), the lock gets unlocked directly after locking it in the line before.

From a performance point of view, couldn't the lck variable just be initialized before the loop starts?

Compilation

It's usually a good habit to keep things close to where they are used, close as in scope. It generally makes code easier to read, understand and most importantly to maintain. – Dewclaw

@Someprogrammerdude I edited my question because I am interessted in the performance aspect. – Compilation

Most likely the optimizer will hoist that lock variable out of the loop for you. In short; it doesn't matter. Don't worry about it and spend your time on something more productive. – Biddable

As for the "because in the next line, mcond.wait(lck), the lock gets unlocked directly after locking it in the line before" part, moving the definition of lck out of the loop would not really make much difference here. The lock would be locked outside the loop, then immediately inside the loop be unlocked by the very same wait call. – Dewclaw

@Someprogrammerdude In the first iteration of course. But how about the iterations after it? When the lock gets unlocked and m is processed, the loop starts again. However this time, the mutex does not get locked again before the wait call. Am I right with this one? – Compilation

Regarding performance worries, don't do that. Premature optimization is usually not a good way to start with a program. Instead write code that is simple, easy to read, understand, modify and maintain. Then if (and only if, remember that "good enough" usually is good enough) the performance is lacking you measure and profile to find the bottlenecks, and concentrate on the worst ones only. And remember to document and comment about the optimizations, since those tend to obfuscate code. – Dewclaw

Ah, but the wait function expects the lock to be locked! If it's not locked then you have undefined behavior. So the placement is not only about "closeness", but about avoiding problems. – Dewclaw

Okay, thanks for the advice. If wait expects the lock to be locked then of course it has to be done this way. – Compilation

From a performance point of view, locking and unlocking a mutex is far more expensive than creating a unique_lock. (You're basically worrying about whether driving barefoot might improve your gas mileage.) – Zimmerman

According to en.cppreference.com/w/cpp/thread/condition_variable/wait : "When unblocked, regardless of the reason, lock is reacquired and wait exits". It seems the thread still has ownership of the mutex but it releases it while it sleeps to allow other threads to enter critical parts of their code. As such, requiring that the mutex is locked beforhand is likely a way to ensure that only the current thread owns the mutex. – Jockey

Answer 1

As stated on cppreference, the std::unique_lock constructor is as such:

explicit unique_lock( mutex_type& m );

The constructor will do the following:

Constructs a unique_lock with m as the associated mutex. Additionally locks the associated mutex by calling m.lock(). The behavior is undefined if the current thread already owns the mutex except when the mutex is recursive.

So the code locks the mutex on every iteration. Moving it out of the loop would change the logic.

Larghetto

Kind resource acquisition by object creation? – Anjaanjali