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

Spawn a !Send future onto any thread #2545

Closed
Diggsey opened this issue May 19, 2020 · 25 comments
Closed

Spawn a !Send future onto any thread #2545

Diggsey opened this issue May 19, 2020 · 25 comments
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-blocking Module: tokio/task/blocking M-task Module: tokio/task

Comments

@Diggsey
Copy link
Contributor

Diggsey commented May 19, 2020

I often want to create a task which will be pinned to a thread. However, when I'm creating the task, I don't care which thread it ends up pinned on.

Essentially I would like a spawn function that looks something like this:

fn spawn_pinned<Fut: Future>(f: impl FnOnce() -> Fut + Send + 'static);

(ie. the Fut does not implement Send)

The reason for this is that the future has some internal !Sync state that it will access across a yield point.

@carllerche
Copy link
Member

Your snippet has the future implementing Send?

The reason we don't support this right now is any arbitrary thread can become blocked indefinitely (because of block_in_place). A future pinned to a thread may not be able to execute.

@Diggsey
Copy link
Contributor Author

Diggsey commented May 19, 2020

It doesn't :) The Send bound is tied to the impl <Trait> clause. If I wanted to put the Send bound on the future it would look like this:

fn spawn_pinned<Fut: Future + Send + 'static>(f: impl FnOnce() -> Fut + Send + 'static);

@Darksonn Darksonn added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-blocking Module: tokio/task/blocking M-task Module: tokio/task labels May 19, 2020
@MikailBag
Copy link
Contributor

As far as I understand, this feature can be very useful for e.g. actix-web.
I don't know how one can spawn actix-web Server inside Tokio task without such function.

@jxs
Copy link
Member

jxs commented Jun 1, 2020

I don't know how one can spawn actix-web Server inside Tokio task without such function.

actix uses LocalSet:
https://github.com/actix/actix-net/blob/master/actix-rt/src/runtime.rs#L65

@MikailBag
Copy link
Contributor

If you use LocalSet in some async future, this futures becomes !Send, so it cannot be spawned onto Tokio.

@Darksonn
Copy link
Contributor

Darksonn commented Jun 2, 2020

LocalSet is not meant to be used inside a future, instead it should replace your block_on call, which is what appears to happen in actix. When inside a future spawned on a LocalSet, you can use spawn_local to spawn more things on that LocalSet.

@alecmocatta
Copy link

alecmocatta commented Jun 6, 2020

I've also wanted this from time to time. It's possible in async-std using a pattern like the following. I haven't figured out the logistics of LocalSet to make this work in tokio though.

pub fn spawn_pinned<F, Fut, T>(task: F) -> impl Future<Output = Result<T, JoinError>> + Send
where
    F: FnOnce() -> Fut + Send + 'static,
    Fut: Future<Output = T> + 'static,
    T: Send + 'static,
{
    spawn(future::lazy(|_| spawn_local(task())).flatten()).map(Result::unwrap)
}

https://play.rust-lang.org/?edition=2018&gist=32cdeabfec85ab510020e0efa9f69e95

@Darksonn
Copy link
Contributor

Darksonn commented Jun 6, 2020

Once you use tokio::spawn, the task will be outside the LocalSet, so spawn_local wont work anymore. You can only use spawn_local inside a future running on the LocalSet. You would have to use a channel that sends the closure to a task on the LocalSet and spawn it there.

@AzureMarker
Copy link
Contributor

Currently there doesn't seem to be a good way to spawn !Send futures in web frameworks which use the tokio multi-threaded runtime (e.g. hyper, warp, rocket 0.5). The API proposed in this issue would help out in cases where, for example, a future temporarily handles some internal Send + !Sync state (ex. database connection). The computation over that state involves !Send futures (due to holding a reference to the !Sync state), which would optimally be scheduled (spawn_pinned) onto the current thread's worker and pinned there (doesn't participate in work stealing).

This approach is different from spawning a new thread with a single-threaded executor (e.g. actix-web) because the worker can still execute normal Send futures while the !Send future is suspended. In this way the performance of work-stealing can be maintained while still allowing !Send futures to execute.

@Darksonn
Copy link
Contributor

Darksonn commented Jan 1, 2021

Well, there is no good way to spawn !Send futures on a multi-threaded Tokio runtime, and probably no such way will be added due to the existence of block_in_place, which is in conflict with this feature.

If you give more details on your database connection, there is likely another way around it. If you ask on our Discord or open a discussion about the specific problem, I'll be happy to help out.

@Darksonn
Copy link
Contributor

Darksonn commented Jan 1, 2021

I am closing this because the feature is incompatible with the existence of block_in_place, as if any other future currently executing on the same worker thread calls that method, all !Send futures on that worker thread are unable to run for the duration of that block_in_place call.

Please open a discussion if you need help getting around issues related to this.

@Darksonn Darksonn closed this as completed Jan 1, 2021
@Diggsey
Copy link
Contributor Author

Diggsey commented Jan 1, 2021

@Darksonn IMO, this is a more important feature than block_in_place, because it cannot be worked around. The reality is that sometimes you have a future that is !Send.

That said, I don't believe block_in_place prevents this feature being added: you just need the set of threads where block_in_place can be used to be distinct from the set of threads on which !Send futures are executed.

@sfackler
Copy link
Contributor

sfackler commented Jan 1, 2021

What happens if you call block_in_place on a thread where !Send futures are executed?

@Darksonn
Copy link
Contributor

Darksonn commented Jan 1, 2021

If you want to run futures on a separate thread from the ordinary worker threads, you can do that with LocalSet.

@Diggsey
Copy link
Contributor Author

Diggsey commented Jan 1, 2021

@Darksonn with LocalSet you have to manage the threads yourself. You have to create your own work-stealing queue for the tasks you want to run on those LocalSet(s) and do all the stuff that would normally be the job of tokio.

@Diggsey
Copy link
Contributor Author

Diggsey commented Jan 1, 2021

@sfackler block_in_place will prevent other futures running on the same thread, so any !Send futures that were already spawned to that thread will be stalled.

You need to make sure that you only spawn !Send futures onto threads that won't run tasks using block_in_place.

TBH, block_in_place is flawed as a concept in the first place, as it prevents other futures within the same task from making progress when you use eg select!, so yeah...

edit:
I should clarify, I don't think block_in_place is completely useless: I think there are cases where it may be useful, but it shouldn't be the go-to way to run blocking tasks.

Instead, there should be a separate thread-pool for "high latency" tasks where blocking work can be done, and block_in_place can be a way to improve latency in some cases on that high latency pool.

@AzureMarker
Copy link
Contributor

AzureMarker commented Jan 1, 2021

I don't think block_in_place necessarily prevents this feature from being added. The documentation notes that it already conflicts with things like join! in the same task. I think it would be up to the app developer to work around that possible conflict. Edit: @alecmocatta's reply below states this better.

Here's a playground link that demonstrates the issue with a fake database connection (Send + !Sync):
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=7b372721c2ce4bc894d5133d7a2be9c1
I've also included an alternative spawn_pinned API that restricts the future to execute on the current worker thread and allows !Send output:

fn spawn_pinned_alt<Fut: Future>(_f: Fut) -> task::JoinHandle<Fut::Output>;

Also (as mentioned above) there's some more context on my particular requirements in graphql-rust/juniper#840.

@alecmocatta
Copy link

I don't agree that block_in_place is incompatible with a spawn_pinned API, or, similarly, spawn_local on worker threads.

The docs for block_in_place make clear its general potential "footgun" nature, IMO:

Although this function avoids starving other independently spawned tasks, any other code running concurrently in the same task will be suspended during the call to block_in_place. This can happen e.g. when using the join! macro. To avoid this issue, use spawn_blocking instead.

I have found niche use for block_in_place, but it certainly requires careful consideration and shouldn't be used ubiquitously. It is in the same class as unsafe for me: best avoided, unless necessary, in which case take extra care. If it were to block spawn_pinned tasks I personally would not find that surprising given the pre-existing and already-dicumented footgun behaviour. Any fallout from the introduction of spawn_pinned would be limited as valid uses of block_in_place are niche and already require special attention to avoid blocking other futures.

Instead, there should be a separate thread-pool for "high latency" tasks where blocking work can be done, and block_in_place can be a way to improve latency in some cases on that high latency pool.

If tasks are high-latency then why not just use spawn_blocking? Regardless this sounds like a pretty specific use case that's probably best addressed outside of tokio, e.g. by spawning ones own worker threads and not needing block_in_place in the first place?

I suggest the most straightforward implementation would be to introduce a local task queue on worker threads, which would enable spawn_local on worker threads and thus spawn_pinned.

@Diggsey
Copy link
Contributor Author

Diggsey commented Jan 1, 2021

If tasks are high-latency then why not just use spawn_blocking

It's a bit of an edge case, but you might have a task which is a mixture of async and blocking code, where the blocking part is !Send. The whole thing should run on the high-latency pool, but it could still be advantageous to use spawn_blocking so that other tasks can be picked up by other threads in the pool when that part is running.

I suggest the most straightforward implementation would be to introduce a local task queue on worker threads,

This would solve a different problem from the one described in this issue. The idea is that you don't care which thread it runs on, as long as it sticks to that thread. Your suggestion would unnecessarily prohibit spawning such tasks from non-worker threads, and would also prevent tokio from doing any kind of "load balancing" to prevent a single thread from becoming the bottleneck.

@AzureMarker
Copy link
Contributor

This would solve a different problem from the one described in this issue. The idea is that you don't care which thread it runs on, as long as it sticks to that thread. Your suggestion would unnecessarily prohibit spawning such tasks from non-worker threads, and would also prevent tokio from doing any kind of "load balancing" to prevent a single thread from becoming the bottleneck.

I (perhaps naively) think that having a local task queue in the workers would still allow load balancing since the spawn_pinned API has a FnOnce + Send + 'static which generates the !Send future. Before generating the future via the closure, Tokio can find the best thread to use.

@alecmocatta
Copy link

Your suggestion would unnecessarily prohibit spawning such tasks from non-worker threads, and would also prevent tokio from doing any kind of "load balancing" to prevent a single thread from becoming the bottleneck.

The spawn_pinned impl using spawn and spawn_local I mentioned does in fact do "load balancing" via the work-stealing of the spawned intermediary task. This impl is naïve and tokio can no doubt optimise performance significantly, but the approach is sound and does not have the drawbacks you identify.

@Diggsey
Copy link
Contributor Author

Diggsey commented Jan 2, 2021

@alecmocatta you're right, I missed that you were suggesting spawn_pinned and spawn_local as two separate pieces of functionality.

@Darksonn
Copy link
Contributor

Darksonn commented Jan 2, 2021

Regarding block_in_place, it is currently a footgun because it requires non-local knowledge, but only about the current task (and whether the runtime is multi- or single-threaded), but by adding spawn_local, it suddenly starts to require knowledge about every other task on the runtime. The current state is already bad, no reason to make it even worse.

As for a spawn_pinned that uses a separate pool of threads, you should be able to write separate crate that adds this functionality by wrapping LocalSet now. Then you can have the main thread pool do work stealing of ordinary tasks, and have the extra threads try to distribute new tasks to the LocalSet with the fewest tasks or something.

@MikailBag
Copy link
Contributor

MikailBag commented Jan 2, 2021

As for a spawn_pinned that uses a separate pool of threads, you should be able to write separate crate that adds this functionality by wrapping LocalSet now. Then you can have the main thread pool do work stealing of ordinary tasks, and have the extra threads try to distribute new tasks to the LocalSet with the fewest tasks or something.

Is this worth adding to tokio-util?

@AzureMarker
Copy link
Contributor

I just opened a PR to add spawn_pinned to tokio-util. It uses the idea of spawning separate worker threads to handle the !Send futures.

#3370

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-blocking Module: tokio/task/blocking M-task Module: tokio/task
Projects
None yet
Development

No branches or pull requests

8 participants