smol-rs / concurrent-queue

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

add get_head() and get_tail() methods #41

Closed gsegatti closed 1 year ago

gsegatti commented 1 year ago

The discussion that led to this pr is #40.

This pr adds a method get_head() and get_tail(), both methods returning the values stored in such positions. There's also a new error type introduced, GetError.

gsegatti commented 1 year ago

This PR is currently in draft because I want to validate what I've done so far, before creating the methods get_head() and get_tail() for all types of queues.

I'll leave a few preemptive comments below as well hoping to have some clarity on a few doubts.

gsegatti commented 1 year ago

I've applied all of the feedback proposed here. I just want to confirm one thing though.

For the tail_mut() implementation, when in the bounded queue: Do we want to return the actual tail of the queue, or the last inserted element T?

I say this, because in this example:

let mut q = ConcurrentQueue::bounded(2);
q.push(10);

The tail_mut() method returns 0 if we're just retrieving whatever's at the tail of the queue. But if we're returning the last inserted element, then the correct value to be returned is 10.

It seems a bit pointless for the bounded queue to return the tail, always, because when the queue is finally filled, the tail completes a lap and now points to the head. So'll we'll return the same value as head_mut().

If we want to return the last inserted element, I have to figure out some logic to retrieve the element at position tail - 1, specially when we complete a lap.

gsegatti commented 1 year ago

I've done tail_mut() returning the last inserted element at all times. Let me know if this is not what we're hoping to achieve and if I should revert to just returning the tail, irrespective of having contents or not.

If this looks ok and has minor adjustments to be made, I'd like to progress into implementing the same methods for the Single and Unbounded queues. I can create one branch for each on top of this one and, when ready, merge it all under this branch, using fix-up to not pollute the commit history.

cc: @notgull