Anand Shah
Anand Shah

Reputation: 143

Thread safe queue implementation : Efficient implementation of pop()

I am trying to implement thread safe version of queue and facing problem in implementing wrapper around pop(). Refer code below. Can't paste entire code due to restriction.

bool internal_pop_front_no_lock(T& item)
{
    bool isDataAvailable = false;

    if (!m_Queue.empty())
    {
        item = m_Queue.front();
        m_Queue.pop();
        isDataAvailable = true;
    }

   return isDataAvailable;
}

Now I feel the line item = m_Queue.front(); will make a copy of data. Is there a way I can avoid copy? or am I misunderstanding something?

Upvotes: 0

Views: 176

Answers (2)

sklott
sklott

Reputation: 2849

There is only one way to avoid copying, if you move this object and then return it. Because even if you can access m_Queue.front() without copying, as soon as you do m_Queue.pop(); this object will be destroyed, and if you need access to this object beyond this point you need to either copy or move this object. So something like this is your only chance:

std::optional<T> internal_pop_front_no_lock()
{
    std::optional<T> result;

    if (!m_Queue.empty())
    {
        result = std::move(m_Queue.front());
        m_Queue.pop();
    }

   return result;
}

Upvotes: 2

Yakk - Adam Nevraumont
Yakk - Adam Nevraumont

Reputation: 275650

As std::queue stores actual values, the best you can do is move the value out (as in std::move). For many complex objects, move is cheap even if copy is expensive.

In the event you have a complex object for whom moving is expensive, you can have a queue of unique_ptrs to that; those are cheap to move (and impossible to copy).

Test your thread-safe queue using a std::unique_ptr<int> or something and make sure it works. Then you'll know your queue does no copy operations. It is then up to the users of your queue (possibly yourself) to ensure that the items they put in it are sufficiently cheap to move.

A boost::optional<T> or std::optional<T> is very useful here, as you have a T& and a bool that indicates if it is populated or not. But the core is:

result = std::move(m_Queue.front()); // move

instead of

result = m_Queue.front(); // copy

when you extract the value.

...

Myself, I'd be leery of this method. Its long name is a red flag, and all it does is wrap a few std::queue methods up. It doesn't look like it reduces complexity much over just calling the std::queue methods directly.

The tricky parts of a thread-safe queue are, in my experience, deciding how the notification works, aborting, batching operations (pop_many, push_many), etc. The underlying interaction with the queue is really simple in comparison.

Upvotes: 1

Related Questions