smol-rs / concurrent-queue

Concurrent multi-producer multi-consumer queue
Apache License 2.0
254 stars 21 forks source link

See value stored in head/tail of queue #40

Closed gsegatti closed 1 year ago

gsegatti commented 1 year ago

Not sure if this is an anti-pattern towards what you guys have implemented. But I'd like to know if there's any reason as to why there's no functions such as get_head, get_tail for the Bounded and Unbounded queues?

I have a use case where I do not want to push an item onto the queue if said item was the last thing to be added. Hence the question.

Thanks!

notgull commented 1 year ago

Not sure if this is an anti-pattern towards what you guys have implemented. But I'd like to know if there's any reason as to why there's no functions such as get_head, get_tail for the Bounded and Unbounded queues?

It would be unsound to have these functions with the current implementation of concurrent-queue. If get_tail returned an &T, when a simultaneous pop operation would make it so that reference is invalidated. This would only work if get_tail took an &mut reference, where the borrow checker can assert that the current user is the only one who has access to the queue.

I would accept a PR adding methods to handle queue operations in an &mut way.

I have a use case where I do not want to push an item onto the queue if said item was the last thing to be added. Hence the question.

It might be worth it to de-duplicate the item at the point where it's popped, i.e. keep a copy of the item on hand and then ignore a pop() if it's equal to the last item. If your workload doesn't involve a lot of synchronization, a Mutex<VecDeque> would also be an option.

gsegatti commented 1 year ago

Appreciate your reply!

If get_tail returned an &T, when a simultaneous pop operation would make it so that reference is invalidated.

Oh, that actually makes a lot of sense, now that you said it.

I would accept a PR adding methods to handle queue operations in an &mut way.

I'd like to try that, will attempt to submit a draft PR for one of the queues! In this case we're talking about adding get_tail and get_head, each function receiving &mut self and returning the value in the queue, correct? Should it be a Result<T> or just the value T? (Maybe a result in case the queue is empty)

It might be worth it to de-duplicate the item at the point where it's popped, i.e. keep a copy of the item on hand and then ignore a pop() if it's equal to the last item.

Yep, that also makes sense. For now, I can live with duplicates, it was more a matter of premature optimization on my end. But if I can get the get_tail and get_head methods to work that would be ideal!

gsegatti commented 1 year ago

@notgull I've opened a draft PR #41 so you and the maintainers can give some early feedback!

taiki-e commented 1 year ago

Personally, I think something like an iterator that returns a mutable reference to an element is preferable to this. (like FuturesUnorded::iter_mut)

gsegatti commented 1 year ago

Yep, agree this seems like a bit of an anti-pattern. Will close the PR linked to it. On another note, something I'd like to see is a circular queue of fixed size. But perhaps this should be another issue.

I'll close this for now and I'll reopen if I ever come around implementing an iterator.