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

[discussion] task::block_on -> thread::block_on? #289

Closed
yoshuawuyts opened this issue Oct 8, 2019 · 3 comments
Closed

[discussion] task::block_on -> thread::block_on? #289

yoshuawuyts opened this issue Oct 8, 2019 · 3 comments
Labels
api design Open design questions

Comments

@yoshuawuyts
Copy link
Contributor

Okay so this might be a terrible idea, and I'm not convinced myself -- but I think it's still worth bringing up because there might be value to it.

In #251 we're going to add task::blocking, but the name is kind of similar to task::block_on. The difference between the two is that one takes a closure that will block a thread, while the other one blocks the current thread:

fn block_on(fut: impl Future<Output = T>) -> T;
fn blocking(fut: impl Future<Output = T>) -> JoinHandle<T>;

What I'm slightly worried about is that people will not catch the difference between the two methods, and use block_on instead of blocking. And to be fair; this will probably work in most situations -- except performance will be terrible because this is not at all intended for this.

So instead I'm wondering if it would perhaps make sense to move block_on to be part of the thread submodule. It already has several other functions related to blocking the current thread, and this would be another one in that line.

// current
fn thread::park();
fn thread::park_timeout(dur: Duration);
fn thread::sleep(dur: Duration);
fn thread::yield_now();

// added
fn block_on(fut: impl Future<Output = T>) -> T;

A reason why I'm kind of wondering about using the thread submodule is because we probably want to give people access to the threadpool to configure it, and I feel thread may be a good option to put it (though I have no idea which knobs we'd want to allow people to turn). Which if we have that submodule anyway we may want to consider this direction also. I'm not sure though!

Perhaps another way to go about this would be to make block_on fail if called inside of a task context:

use crate::task::worker::get_task;

let task = get_task(|task| task.clone());
debug_assert!(task.is_none(), "`task::block_on()` called inside the context of a task. Did you mean to call `task::blocking?`");

Oh also; none of this is pressing at all, and just wanted to bring this up as something that may be interesting to consider. Thanks!

@yoshuawuyts yoshuawuyts added the api design Open design questions label Oct 8, 2019
@yoshuawuyts
Copy link
Contributor Author

Okay, riffing on this a bit more. Now that #299 is a thing, things have changed even more as spawn_blocking takes a regular closure rather than an async block now:

fn task::spawn(impl Future) -> JoinHandle;
fn task::spawn_thread(impl FnOnce) -> JoinHandle;

So there seems to be somewhat of a parallel here now with thread. I think we could do something similar there:

fn thread::spawn(impl FnOnce) -> JoinHandle;
fn thread::spawn_async(impl Future) -> JoinHandle;

This creates a nice parallel I think, and nicely bridges between the two concepts. We could perhaps even take the names slightly further, and call them each other's submodule:

fn task::spawn(impl Future) -> task::JoinHandle;
fn task::spawn_thread(impl FnOnce) -> task::JoinHandle;

fn thread::spawn(impl FnOnce) -> thread::JoinHandle;
fn thread::spawn_task(impl Future) -> thread:: JoinHandle;

The only concern here is that spawn_thread might sound like it's a one-off thread, while we in fact do use a thread pool to back things up. But I'm not sure if that's more important than the FANTASTIC API SYMMETRY WE COULD HAVE!

@yoshuawuyts
Copy link
Contributor Author

Okay, tried to implement this and thread::spawn_task can't and shouldn't return a JoinHandle.

That kind of begs the question whether it still makes sense the way it's done (or whether it should have a different kind of join handle), but in general I still kind of like the direction we're proposing, even without the JoinHandle being returned.

@yoshuawuyts
Copy link
Contributor Author

I don't think this is worth it at this time. Closing!

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

No branches or pull requests

1 participant