rigtorp / MPMCQueue

A bounded multi-producer multi-consumer concurrent queue written in C++11
MIT License
1.15k stars 159 forks source link

Add try_consume_until_current_head #25

Closed ferranpujolcamins closed 3 years ago

ferranpujolcamins commented 3 years ago

Inspired by your try_consume PR I added a try_consume_until_current_head method that consumes all elements of the queue that were added before the current head.

My use case is a macro recorder. The queue captures commands from different threads. Then, when the user triggers the macro, all recorded commands are executed. However it's possible that while the commands belonging to the first recording are still being consumed, commands from a second recording are already being recorded, and they shouldn't be executed until next time the macro is triggered. With try_consume or try_pop I cannot guarantee this.

I'm not sure what the memory ordering should be for the head_.load at the beginning of the function. Can you help here?

I'm not sure what's you policy on new features, let me know if you think this doesn't belong to the project.

rigtorp commented 3 years ago

This will only work when there is a single consumer and will not solve your problem. Why not have a special item that is enqeued to indicate that the macro has finished recording.

ferranpujolcamins commented 3 years ago

In my use-case I indeed only have one consumer, but to be honest I didn't thought about using a special item, it might very well do the trick. Thanks for your input.

ferranpujolcamins commented 3 years ago

Well, there is a problem with the placeholder approach: I have to consider the special case when the queue is full, because I cannot insert the placeholder. I have to first pop one element to make room for the placeholder, and It might happen that someone else just adds a new element to the queue before I can push the placeholder.

With try_consume_until_current_head I don't have to worry about this edge case.

rigtorp commented 3 years ago

Wouldn't the consumer always be calling try_pop() so you will be able to append the place holder momentarily. Of course something else could be added in between, but that is inevitable with the design you have.

ferranpujolcamins commented 3 years ago

Wouldn't the consumer always be calling try_pop() so you will be able to append the place holder momentarily.

The problem is when a new element is appended to the queue after I call try_pop() but before the placeholder is appended, because then I'll have to retry the operation and the same thing could happen again. So there's no guarantee that the placeholder will ever be appended, and thus, no guarantee that the consuming operation will eventually stop.

Of course something else could be added in between, but that is inevitable with the design you have.

Yes, and from my point of view this is an acceptable behavior under normal circumstances, but there's the issue I comment above.

rigtorp commented 3 years ago

Honestly I think there is something wrong with your architecture, but maybe I'm misunderstanding something. I've been using this queue for many years in a trading system to trade billions of dollars of notional value of futures. It does what it says it does on the package: multi producer multi consumer queue.

ferranpujolcamins commented 3 years ago

I'll leave this in my fork then. Thanks for taking the time to have a look at the PR.

rigtorp commented 3 years ago

Happy to help. I hope find a good solution to your use case.