Closed Diggsey closed 3 years ago
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.
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);
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.
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
If you use LocalSet in some async future, this futures becomes !Send, so it cannot be spawned onto Tokio.
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
.
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
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.
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.
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.
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 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.
What happens if you call block_in_place on a thread where !Send futures are executed?
If you want to run futures on a separate thread from the ordinary worker threads, you can do that with LocalSet
.
@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.
@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.
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 https://github.com/graphql-rust/juniper/pull/840.
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 thejoin!
macro. To avoid this issue, usespawn_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
.
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.
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.
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 spawn
ed 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.
@alecmocatta you're right, I missed that you were suggesting spawn_pinned
and spawn_local
as two separate pieces of functionality.
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.
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?
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.
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:
(ie. the
Fut
does not implementSend
)The reason for this is that the future has some internal
!Sync
state that it will access across a yield point.