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

Add a method to the mpsc::Receiver for producing a non-blocking iterator #34724

Merged
merged 5 commits into from
Jul 22, 2016

Conversation

mitchmindtree
Copy link
Contributor

@mitchmindtree mitchmindtree commented Jul 8, 2016

Currently, the mpsc::Receiver offers methods for receiving values in both blocking (recv) and non-blocking (try_recv) flavours. However only blocking iteration over values is supported. This PR adds a non-blocking iterator to complement the try_recv method, just as the blocking iterator complements the recv method.

Use-case

I predominantly use rust in my work on real-time systems and in particular real-time audio generation/processing. I use mpsc::channels to communicate between threads in a purely non-blocking manner. I.e. I might send messages from the GUI thread to the audio thread to update the state of the dsp-graph, or from the audio thread to the GUI thread to display the RMS of each node. These are just a couple examples (I'm probably using 30+ channels across my various projects). I almost exclusively use the mpsc::Receiver::try_recv method to avoid blocking any of the real-time threads and causing unwanted glitching/stuttering. Now that I mention it, I can't think of a single time that I personally have used the recv method (though I can of course see why it would be useful, and perhaps the common case for many people).

As a result of this experience, I can't help but feel there is a large hole in the Receiver API.

blocking non-blocking
recv try_recv
iter 🙀

For the most part, I've been working around this using while let Ok(v) = r.try_recv() { ... }, however as nice as this is, it is clearly no match for the Iterator API.

As an example, in the majority of my channel use cases I only want to check for n number of messages before breaking from the loop so that I don't miss the audio IO callback or hog the GUI thread for too long when an unexpectedly large number of messages are sent. Currently, I have to write something like this:

let mut take = 100;
while let Ok(msg) = rx.try_recv() {
    // Do stuff with msg
    if take == 0 {
        break;
    }
    take -= 1;
}

or wrap the try_recv call in a Range<usize>/FilterMap iterator combo.

On the other hand, this PR would allow for the following:

for msg in rx.try_iter().take(100) {
    // Do stuff with msg
}

I imagine this might also be useful to game devs, embedded or anyone doing message passing across real-time threads.

Currently, the `mpsc::Receiver` offers methods for receiving values in
both blocking (`recv`) and non-blocking (`try_recv`) flavours. However
only blocking iteration over values is supported. This commit adds a
non-blocking iterator to complement the `try_recv` method, just as the
blocking iterator complements the `recv` method.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

Thanks for the PR! I was pretty skeptical at first glance but I think your description has won me over. My only heistation is the fact that we're hiding whether iteration was terminated due to "would block" or "disconnected", but perhaps that's not so bad in the long run? If a channel is disconnected then any attempt to receive on it again will again receive a disconnection

cc @rust-lang/libs

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 9, 2016
@mitchmindtree
Copy link
Contributor Author

mitchmindtree commented Jul 9, 2016

@alexcrichton thanks for taking a look!

My only heistation is the fact that we're hiding whether iteration was terminated due to "would block" or "disconnected, but perhaps that's not so bad in the long run?"

I attempted to make sure that this behaviour is clear in the new iterator's docs, but perhaps I could also add a note that recommends using try_recv, a loop and a match expr (with an example?) in the case that the user wishes to loop while distinguishing between reasons for termination?

@alexcrichton
Copy link
Member

@mitchmindtree ah yeah the docs are definitely quite clear in this regard, I was just hoping we could surface it at the API level as well (but I'm not sure that's possible here)

@brson
Copy link
Contributor

brson commented Jul 11, 2016

Thanks for the good writeup. Looks fine to me.

@aturon
Copy link
Member

aturon commented Jul 13, 2016

I'm happy to land this API on an unstable basis.

@alexcrichton
Copy link
Member

Discussed during libs triage the decision was to merge, thanks again for the PR @mitchmindtree!

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 19, 2016

📌 Commit b02b38e has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 19, 2016

⌛ Testing commit b02b38e with merge 376e0f9...

@bors
Copy link
Contributor

bors commented Jul 19, 2016

💔 Test failed - auto-linux-64-opt

@mitchmindtree
Copy link
Contributor Author

mitchmindtree commented Jul 20, 2016

Ahh, I'm missing the issue field for the unstable decorators. Would you like me to create a tracking issue for this? Or would you prefer it if I use the issue number of this PR?

@alexcrichton
Copy link
Member

Ah yeah that'd do it, if you want to open an issue I'll be sure to tag it appropriately and you can update this PR to reference it.

@alexcrichton
Copy link
Member

@bors: r+ aed2e5c

Thanks!

@bors
Copy link
Contributor

bors commented Jul 20, 2016

⌛ Testing commit aed2e5c with merge 72dae88...

@bors
Copy link
Contributor

bors commented Jul 20, 2016

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@alexcrichton
Copy link
Member

Looks like a failure perhaps in the tests?

On Wed, Jul 20, 2016 at 11:20 AM, bors [email protected] wrote:

💔 Test failed - auto-win-msvc-64-opt-rustbuild
https://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt-rustbuild/builds/1851


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

@mitchmindtree
Copy link
Contributor Author

Ahh sorry about that @alexcrichton, I just made the fix and successfully ran the tests locally - should be good to go now!

@alexcrichton
Copy link
Member

@bors: r+

No worries!

@bors
Copy link
Contributor

bors commented Jul 21, 2016

📌 Commit 05af033 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 21, 2016

⌛ Testing commit 05af033 with merge dbc9edd...

@bors
Copy link
Contributor

bors commented Jul 21, 2016

💔 Test failed - auto-win-gnu-32-opt

@alexcrichton
Copy link
Member

@bors: retry

sorry for the number of retries...

On Thu, Jul 21, 2016 at 11:47 AM, bors [email protected] wrote:

💔 Test failed - auto-win-gnu-32-opt
https://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt/builds/5014


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

@bors
Copy link
Contributor

bors commented Jul 22, 2016

⌛ Testing commit 05af033 with merge 0d75975...

bors added a commit that referenced this pull request Jul 22, 2016
…ichton

Add a method to the mpsc::Receiver for producing a non-blocking iterator

Currently, the `mpsc::Receiver` offers methods for receiving values in both blocking (`recv`) and non-blocking (`try_recv`) flavours. However only blocking iteration over values is supported. This PR adds a non-blocking iterator to complement the `try_recv` method, just as the blocking iterator complements the `recv` method.

Use-case
-------------

I predominantly use rust in my work on real-time systems and in particular real-time audio generation/processing. I use `mpsc::channel`s to communicate between threads in a purely non-blocking manner. I.e. I might send messages from the GUI thread to the audio thread to update the state of the dsp-graph, or from the audio thread to the GUI thread to display the RMS of each node. These are just a couple examples (I'm probably using 30+ channels across my various projects). I almost exclusively use the `mpsc::Receiver::try_recv` method to avoid blocking any of the real-time threads and causing unwanted glitching/stuttering. Now that I mention it, I can't think of a single time that I personally have used the `recv` method (though I can of course see why it would be useful, and perhaps the common case for many people).

As a result of this experience, I can't help but feel there is a large hole in the `Receiver` API.

| blocking | non-blocking |
|------------|--------------------|
| `recv` | `try_recv` |
| `iter` | 🙀   |

For the most part, I've been working around this using `while let Ok(v) = r.try_recv() { ... }`, however as nice as this is, it is clearly no match for the Iterator API.

As an example, in the majority of my channel use cases I only want to check for *n* number of messages before breaking from the loop so that I don't miss the audio IO callback or hog the GUI thread for too long when an unexpectedly large number of messages are sent. Currently, I have to write something like this:

```rust
let mut take = 100;
while let Ok(msg) = rx.try_recv() {
    // Do stuff with msg
    if take == 0 {
        break;
    }
    take -= 1;
}
```

or wrap the `try_recv` call in a `Range<usize>`/`FilterMap` iterator combo.

On the other hand, this PR would allow for the following:

```rust
for msg in rx.try_iter().take(100) {
    // Do stuff with msg
}
```

I imagine this might also be useful to game devs, embedded or anyone doing message passing across real-time threads.
@bors bors merged commit 05af033 into rust-lang:master Jul 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants