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

[feature request] re-expose try_recv() #12

Closed
Roguelazer opened this issue Nov 2, 2016 · 8 comments
Closed

[feature request] re-expose try_recv() #12

Roguelazer opened this issue Nov 2, 2016 · 8 comments

Comments

@Roguelazer
Copy link

I'm looking at converting some code from std::sync::mpsc to chan, and the lack of try_recv kind of makes my code ugly. For example, consuming all current items on an asynchronous channel without blocking in std looks like the following:

while let Some(item) = item.containing.chan.try_recv() {
    // handle item
}

In chan, this ends up looking like

let receiver = item.containing.chan
loop {
    chan_select! {
        default => {
            break;
        },
        receiver.recv() -> item => {
             // handle item
        }
    }
}

If you're in a vaguely-OO context and the receiver is on self, this gets even uglier because there are gross contortions required to get a reference to the receiver whose lifetime doesn't spread through the entire method.

I appreciate that it's cleaner to just have select instead of also having the .try_* methods, but it seems that it might be more pragmatic to keep both interfaces, especially since it seems like upstream is never going to stabilize Select in std::sync::mpsc, so chan is going to be increasingly popular for people who want CSP semantics.

@BurntSushi
Copy link
Owner

BurntSushi commented Nov 2, 2016

I think my primary question is: why do you want to do a non-blocking recv in the first place? (I don't doubt that their are use cases, but they feel kind of rare in the context of blocking channels.)

Otherwise, I think I'm OK exposing this method since I do feel like you make a reasonably compelling argument. I wish chan_select! were more ergonomic, but I don't think that's going to happen until we get better macros/plugins on Rust stable. (Which I don't see happening any time soon.)

especially since it seems like upstream is never going to stabilize Select in std::sync::mpsc, so chan is going to be increasingly popular for people who want CSP semantics.

Yeah, Select in std::sync::mpsc is not so great. The fact that it's unstable isn't its only problem; it also can't synchronize across channel sends and I don't believe supports default. The tracking issue is here (where I've just suggested that we remove it entirely): rust-lang/rust#27800

I wonder if crossbeam could be made to add a "select" with its sync::MsQueue type. If crossbeam got that, then I think that would almost make this crate obsolete. (MsQueue is an asynchronous channel while chan exposes both asynchronous and synchronous channels.)

@Roguelazer
Copy link
Author

@BurntSushi I think trying to read all pending messages on a channel without going back into select! is a pretty common pattern. In this case, the application is (roughly) reading all of the pending messages on one channel, slightly modifying them, then adding them as a batch to another channel for some other thread to work on en masse.

@BurntSushi
Copy link
Owner

That actually sounds a little weird to me. I don't think I've ever had the
occasion to do that. (If I need batches then I would probably do it
explicitly with a vec or something.)

I don't want to belabor this too much. I am okay with exposing this, but i
would like to understand the use cases better.

On Nov 3, 2016 11:49 PM, "James Brown" notifications@github.com wrote:

@BurntSushi https://github.com/BurntSushi I think trying to read all
pending messages on a channel without going back into select! is a pretty
common pattern. In this case, the application is (roughly) reading all of
the pending messages on one channel, slightly modifying them, then adding
them as a batch to another channel for some other thread to work on en
masse.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAb34pI87UQElwClXb2jBokvMzY3m03pks5q6qs-gaJpZM4Kmyqn
.

@Roguelazer
Copy link
Author

Um, one of the places I use it (which I haven't actually tried to port to chan yet, but which I think is clearer) is a wrapper library for talking to Kafka. The C library I'm wrapping there (librdkafka) takes batches of messages for efficiency; I want to expose an asynchronous channel interface. So I use try_recv() to grab all of the single messages posted by various other producer threads, then pass that downstream as a single batch.

@Roguelazer
Copy link
Author

Incidentally, the code could be simplified a bit if I had a select with timeout (like select(2)/poll(2)/et all). I guess I could emulate that with an extra thread which emits a tick onto a channel periodically, but shrug. There's always another way to write something.

James Brown,
currently mobile

On Nov 3, 2016, at 20:52, Andrew Gallant notifications@github.com wrote:

That actually sounds a little weird to me. I don't think I've ever had the
occasion to do that. (If I need batches then I would probably do it
explicitly with a vec or something.)

I don't want to belabor this too much. I am okay with exposing this, but i
would like to understand the use cases better.

On Nov 3, 2016 11:49 PM, "James Brown" notifications@github.com wrote:

@BurntSushi https://github.com/BurntSushi I think trying to read all
pending messages on a channel without going back into select! is a pretty
common pattern. In this case, the application is (roughly) reading all of
the pending messages on one channel, slightly modifying them, then adding
them as a batch to another channel for some other thread to work on en
masse.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAb34pI87UQElwClXb2jBokvMzY3m03pks5q6qs-gaJpZM4Kmyqn
.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@BurntSushi
Copy link
Owner

OK, so this method already exists, but it's not exposed. Here is its signature:

fn try_recv(&self) -> Result<Option<T>, ()>;

The return type seems a little strange to me. Ideally, it would just return an Option<T>, but recv already returns an Option<T>. Specifically, in the case of recv, None is returned if the channel is closed.

An Option nested in a Result seems a little unfortunate. Should we instead return a custom enum? e.g.,

enum Try<T> {
    Some(T),
    WouldBlock,
    Closed,
}

That seems more explicit to me.

@Roguelazer
Copy link
Author

That seems way better!

James Brown,
currently mobile

On Nov 4, 2016, at 06:30, Andrew Gallant notifications@github.com wrote:

OK, so this method already exists, but it's not exposed. Here is its signature:

fn try_recv(&self) -> Result<Option, ()>;
The return type seems a little strange to me. Ideally, it would just return an Option, but recv already returns an Option. Specifically, in the case of recv, None is returned if the channel is closed.

An Option nested in a Result seems a little unfortunate. Should we instead return a custom enum? e.g.,

enum Try {
Some(T),
WouldBlock,
Closed,
}
That seems more explicit to me.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@BurntSushi
Copy link
Owner

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

No branches or pull requests

2 participants