cameron314 / concurrentqueue

A fast multi-producer, multi-consumer lock-free concurrent queue for C++11
Other
9.52k stars 1.66k forks source link

Support for non default constructible types #345

Open Sephirothbahamut opened 1 year ago

Sephirothbahamut commented 1 year ago

Hi, I see try_dequeue expects a reference it copies the value over; however that may not be desirable especially for non default constructible types.

Instead of adding workarounds outside the library, why not just return an std::optional from try_dequeue?

cameron314 commented 1 year ago

First, std::optional didn't exist when I wrote this code.

While I agree this can be awkward, the reverse might also be true, with some types expensive to move-construct but cheap to move-assign, so I'm sure that would be actually be any better in general. Keep in mind an object can be constructed once and assigned to multiple times. Using std::optional would also make the bulk API less consistent (would have to allocate a vector of objects to return instead of taking an iterator?).

Sephirothbahamut commented 1 year ago

with some types expensive to move-construct but cheap to move-assign, so I'm sure that would be actually be any better in general

True, there may be different scenarios where different operations are faster; but for the scenarios where one operation is straight up impossible (default construction in particular), I'd argue any option is better than no option. In light of your comment it might be worth to support both move assignment and move construction with separate dequeuing functions; via overload or just different function names, both would make sense.

while(auto opt{concurrentqueue.try_dequeue()})
    {
    do_stuff(*opt);
    }

T instance;
while(concurrentqueue.try_dequeue(instance))
    {
    do_stuff(instance);
    }

That is unless there's a desire to keep the class compatible with pre-std::optional C++ standards.

I may give it a deeper look on free time and try to make it myself to then make a pull request, but I'm not confident enough in my skill, especially since I don't have experience coding anything lock-free.

cameron314 commented 1 year ago

Offering both APIs would mean copying a lot of code, and I do like C++11 as a baseline. I'll have to think about it.