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 std::sync::mpsc::Receiver::recv_deadline() #45969

Merged
merged 3 commits into from
Nov 29, 2017

Conversation

ia0
Copy link
Contributor

@ia0 ia0 commented Nov 13, 2017

Essentially renames recv_max_until to recv_deadline (mostly copying recv_timeout
documentation). This function is useful to avoid the often unnecessary call to
Instant::now in recv_timeout (e.g. when the user already has a deadline). A
concrete example would be something along those lines:

use std::sync::mpsc::Receiver;
use std::time::{Duration, Instant};

/// Reads a batch of elements
///
/// Returns as soon as `max_size` elements have been received or `timeout` expires.
fn recv_batch_timeout<T>(receiver: &Receiver<T>, timeout: Duration, max_size: usize) -> Vec<T> {
    recv_batch_deadline(receiver, Instant::now() + timeout, max_size)
}

/// Reads a batch of elements
///
/// Returns as soon as `max_size` elements have been received or `deadline` is reached.
fn recv_batch_deadline<T>(receiver: &Receiver<T>, deadline: Instant, max_size: usize) -> Vec<T> {
    let mut result = Vec::new();
    while let Ok(x) = receiver.recv_deadline(deadline) {
        result.push(x);
        if result.len() == max_size {
            break;
        }
    }
    result
}

Essentially renames recv_max_until to recv_deadline (mostly copying recv_timeout
documentation). This function is useful to avoid the often unnecessary call to
Instant::now in recv_timeout (e.g. when the user already has a deadline). A
concrete example would be something along those lines:

```rust
use std::sync::mpsc::Receiver;
use std::time::{Duration, Instant};

/// Reads a batch of elements
///
/// Returns as soon as `max_size` elements have been received or `timeout` expires.
fn recv_batch_timeout<T>(receiver: &Receiver<T>, timeout: Duration, max_size: usize) -> Vec<T> {
    recv_batch_deadline(receiver, Instant::now() + timeout, max_size)
}

/// Reads a batch of elements
///
/// Returns as soon as `max_size` elements have been received or `deadline` is reached.
fn recv_batch_deadline<T>(receiver: &Receiver<T>, deadline: Instant, max_size: usize) -> Vec<T> {
    let mut result = Vec::new();
    while let Ok(x) = receiver.recv_deadline(deadline) {
        result.push(x);
        if result.len() == max_size {
            break;
        }
    }
    result
}
```
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 14, 2017
@shepmaster
Copy link
Member

r? @Kimundi

@kennytm
Copy link
Member

kennytm commented Nov 22, 2017

Thanks for the PR! We’ll periodically check in on it to make sure that @Kimundi or someone else from the @rust-lang/libs team reviews it soon.

@ia0
Copy link
Contributor Author

ia0 commented Nov 22, 2017

Thanks for the feedback!

@alexcrichton
Copy link
Member

Thanks for the PR @ia0! In general we never land new APIs as #[stable] so this'll want to switch to using #[unstable] along with a tracking issue for the API itself (I can tag it if you cc me on the issue).

Overall I like the idea of Instant-taking APIs to complement the Duration-taking APIs as well. I think it would be especially beneficial to do this for APIs like Condvar and std::thread in addition too. The tracking issue I think could be a good location to game out conventions here for more functions in the standard library and maybe even have a checklist for functions to implement.

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 27, 2017
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 27, 2017
@ia0
Copy link
Contributor Author

ia0 commented Nov 27, 2017

Thanks for your review @alexcrichton! I created #46316 to track a possible API convention for blocking-, timeout-, and deadline-related functions.

I switched to an unstable feature. And actually I wonder if I shouldn't have done the same for #45506 then.

@alexcrichton
Copy link
Member

Awesome, thanks @ia0! I'll comment on that soon.

It looks like tests are failing though? I think the doctests here will need to be tagged with the #![feature] directive

@ia0
Copy link
Contributor Author

ia0 commented Nov 28, 2017

Indeed, I forgot to update the tests. It should be fixed now (at least locally ./x.py test src/libstd --stage=1 is not failing anymore).

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

Oh also I forgot to say, but for the impl you added in #45506 it's safe to be "insta stable" and avoid the unstable attribute, trait implementations are "special" in that they're not currently covered by the stability system.

@bors
Copy link
Contributor

bors commented Nov 29, 2017

📌 Commit 8e025d8 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Nov 29, 2017

⌛ Testing commit 8e025d8 with merge 9546a1bc65fea0eb3d9e2c8d98508d566f230eff...

@bors
Copy link
Contributor

bors commented Nov 29, 2017

💔 Test failed - status-travis

@ia0
Copy link
Contributor Author

ia0 commented Nov 29, 2017

Thanks for the review and explanation about impl stabilization.

The homu test seems to have failed because the build of LLVM for x86_64-apple-darwin seems to fail because future_error is unavailable:

[00:12:30] In file included from /Users/travis/build/rust-lang/rust/src/llvm/lib/Support/ThreadPool.cpp:14:
[00:12:30] In file included from /Users/travis/build/rust-lang/rust/src/llvm/include/llvm/Support/ThreadPool.h:30:
[00:12:30] /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/future:1447:23: error: 'future_error' is unavailable: introduced in macOS 10.8
[00:12:30]                       future_error(make_error_code(future_errc::broken_promise))
[00:12:30]                       ^
[00:12:30] /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/future:502:63: note: 'future_error' has been explicitly marked unavailable here
[00:12:30] class _LIBCPP_EXCEPTION_ABI _LIBCPP_AVAILABILITY_FUTURE_ERROR future_error
[00:12:30]                                                               ^

The other tests seem to have been cancelled.

@kennytm
Copy link
Member

kennytm commented Nov 29, 2017

@bors retry — travis-ci/travis-ci#8821

kennytm added a commit to kennytm/rust that referenced this pull request Nov 29, 2017
Add std::sync::mpsc::Receiver::recv_deadline()

Essentially renames recv_max_until to recv_deadline (mostly copying recv_timeout
documentation). This function is useful to avoid the often unnecessary call to
Instant::now in recv_timeout (e.g. when the user already has a deadline). A
concrete example would be something along those lines:

```rust
use std::sync::mpsc::Receiver;
use std::time::{Duration, Instant};

/// Reads a batch of elements
///
/// Returns as soon as `max_size` elements have been received or `timeout` expires.
fn recv_batch_timeout<T>(receiver: &Receiver<T>, timeout: Duration, max_size: usize) -> Vec<T> {
    recv_batch_deadline(receiver, Instant::now() + timeout, max_size)
}

/// Reads a batch of elements
///
/// Returns as soon as `max_size` elements have been received or `deadline` is reached.
fn recv_batch_deadline<T>(receiver: &Receiver<T>, deadline: Instant, max_size: usize) -> Vec<T> {
    let mut result = Vec::new();
    while let Ok(x) = receiver.recv_deadline(deadline) {
        result.push(x);
        if result.len() == max_size {
            break;
        }
    }
    result
}
```
@kennytm kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 29, 2017
bors added a commit that referenced this pull request Nov 29, 2017
Rollup of 10 pull requests

- Successful merges: #45969, #46077, #46219, #46287, #46293, #46322, #46323, #46330, #46354, #46356
- Failed merges:
@bors bors merged commit 8e025d8 into rust-lang:master Nov 29, 2017
@ia0 ia0 deleted the mpsc_recv_deadline branch May 26, 2019 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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