Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancelling existing SendRef #70

Open
aldanor opened this issue Aug 26, 2022 · 4 comments
Open

Cancelling existing SendRef #70

aldanor opened this issue Aug 26, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@aldanor
Copy link

aldanor commented Aug 26, 2022

Wonder if allowing to cancel existing SendRef would be possible? E.g. something along the lines of

impl<'a, ...> SendRef<'a, ...> {
    /// Drops this send ref *without* sending the data. The slot is returned to the pool.
    pub fn cancel(self) {
        ...
    }
}

Here's a particular sample use case:

  • We're reading network messages from multiple threads into a single thingbuf, trying to avoid all allocations and copying if possible (and recycling really helps with that).
  • Reading those messages is a complicated process - they don't arrive as &[u8] but have to be combined and processed in a streaming fashion, so each message is a BufRead reader.
  • In order to avoid copying, we reserve a SendRef slot and .read_to_end() directly into it.
  • However, sometimes it turns out that those messages have to be filtered out and should never reach the thingbuf receiver...
  • ... but alas, it's too late. As soon as SendRef is dropped, it will be sent away.
  • We can either use some hacky convention like sending empty messages, or some enums, or maybe do reading into a separate buffer first - all of these will work as workarounds, but the cleanest way would be to able to send_ref.cancel() which would cancel the reservation and return the slot to the pool - hence this question.

Thanks!

@hawkw hawkw added the enhancement New feature or request label Aug 26, 2022
@hawkw
Copy link
Owner

hawkw commented Aug 26, 2022

Hmm, something like this would definitely be nice to have, however, I'm not sure if it's really possible with the concurrent ring buffer algorithm. When a SendRef is created, the tail index is advanced:

thingbuf/src/lib.rs

Lines 218 to 221 in 0cf1ea2

match test_dbg!(self
.tail
.compare_exchange_weak(tail, next_tail, SeqCst, Acquire))
{

so the next attempt to push/send a slot will claim the next index.

However, the head (readable) index is not advanced until the SendRef is dropped. I think the naive solution (just mem::forgetting the SendRef) would result in the queue being in an inconsistent state, where the head index is perpetually one slot behind where it "should" be.

I think if we wanted to implement a cancellation API like you're describing, we could, but the only way we could do it would be to essentially mark the slot as having been skipped --- sort of like the approach you proposed of sending an empty message, but with the benefit of not waking up the receiver when a slot is skipped. This would still "waste" the slot on this lap (it would be usable again when the ring buffer wraps around), but it would avoid spurious wakeups.

There might be a better solution. I'll keep thinking about it.

@aldanor
Copy link
Author

aldanor commented Aug 26, 2022

Thanks for the prompt reply - yea, it sounds like the only obvious way for this to work would be implementing a way of marking slots as skipped (as if you kept a SendRef for an entire lap).

For the memory usage perspective, as long as you don't abuse the cancellation procedure and it's more of a rare occasion, it should be negligible in most cases. IIUC, it may introduce some extra branching on the tx side (to handle the 'skipped' status), but with mpsc you'd be typically trying to avoid branching and wakeups on the rx side as much as possible.

@sgasse
Copy link

sgasse commented Mar 6, 2024

Hi! We have a somewhat similar situation where we obtain a SendRef but have loop iterations in which we have nothing to send and thus do not want to send it out. You probably found a workaround (maybe the same) by now @aldanor but I still want to share our workaround here, which works if you are working in a loop:

We created an Option<SendRef<'a, T>> outside of the loop. In every iteration of the loop, we first check if there is already a SendRef in the option and either use that or try to get a new one. If at the end of the loop iteration it turns out that we do not want to send the SendRef, we hand it back to the Option<SendRef<'a, T>>. This avoids empty sends without introducing much complexity or runtime cost.

@hawkw
Copy link
Owner

hawkw commented Apr 18, 2024

PR #81 added the capability for senders to skip slots which are still in use by the receiver on the current lap. We could probably implement slot cancelling using similar code. We would need to use an additional bit of state on the slot to mark it as "hey, this slot doesn't actually contain any data, don't try to read from it" from the receiver's perspective, to the HAS_READER bit that's currently used to tell senders to skip a slot that's currently in use by the receiver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants