add spawn_blocking and use a blocking thread pool
#501
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HippoBaro
force-pushed
the
spawn_blocking
branch
from
2570c51
to
50b7e7a
Compare
HippoBaro
changed the title
spawn_blocking and use a blocking thread poolspawn_blocking and use a blocking thread pool
HippoBaro
changed the title
spawn_blocking and use a blocking thread poolspawn_blocking and use a blocking thread pool
glommer
reviewed
Jan 14, 2022
HippoBaro
force-pushed
the
spawn_blocking
branch
2 times, most recently
from
ae1496f
to
0af3708
Compare
HippoBaro
force-pushed
the
spawn_blocking
branch
from
0af3708
to
920682b
Compare
glommer
approved these changes
Jan 15, 2022
| placement: PoolPlacement, | ||
| sleep_notifier: Arc<SleepNotifier>, | ||
| ) -> io::Result<Self> { | ||
| let (in_tx, in_rx) = crossbeam::channel::unbounded(); |
There was a problem hiding this comment.
can we use bounded channels?
There was a problem hiding this comment.
Ideally I would like to find a channel that would have an async TX and a sync RX.
I don't think it is okay to panic if the que is full like we did before.
And it's also not okay to bubble up blocking to user code. This is meant to be async.
There was a problem hiding this comment.
So an ideal way to deal with this would be to provide async back pressure with an async TX. I haven't found one though.
I think there is a way to use tokio channels that way. I'll look into it.
There was a problem hiding this comment.
See if this does what you need:
https://docs.rs/async-channel/latest/async_channel/
There was a problem hiding this comment.
It doesn't. Tokio channel however does have a blocking_recv(), but that would imply us depending on Tokio. Going to merge as-is but I agree this is something we'll need to keep an eye on.
Sometimes there is no way around doing a long, unyielding job. That's the world we live in. These fall into two categories: * Busy blocking jobs: usually, these are calls to external libraries that aren't cooperative and can't be easily broken into bite-size pieces. Your usual suspects are compression/decompression and marshaling/unmarshalling. * Non-busy blocking jobs: stuff that acquires locks or syscalls waiting for events. `spawn_blocking` works for both cases but is best suited for the latter because they don't steal CPU cycles from the executor.
So far, we have had a single thread per executor used for blocking operations. This has been fine to issue the odd syscall that couldn't go into the ring, but now that we have `spawn_blocking,` we've opened pandora's box. Users will submit all sorts of things in there, including long-running jobs that could stall the entire system. This commit introduces a static thread pool per executor instead. The pool is static for now, meaning the threads are created once and live for as long as the host executor does. Later on, we may want to be smarter and automatically up/downscale the pool depending on usage.
Add knobs in the local executor builders to configure how many blocking threads to create in the pool and where to place them on the system. By default, an executor spawns a single blocking thread in its pool and collocates it with its thread. It could be a good idea to loosen that default placement strategy to bind the blocking thread to any CPUs on the same package as the executor thread. This would maximize resource utilization and minimize memory latency. However, there is no easy way to do that using our existing Placement APIs.
Small performance improvement. Don't allocate a dynamic blocking operation handler when it isn't necessary.
If a user submits a blocking operation from a latency-sensitive queue, the blocking thread should unconditionally wake the executor to trigger a preemption. This is the same principle that applies to foreign wakes.
HippoBaro
force-pushed
the
spawn_blocking
branch
from
920682b
to
dcbfd97
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sometimes there is no way around doing a long, unyielding job. That's
the world we live in. These fall into two categories:
that aren't cooperative and can't be easily broken into bite-size
pieces. Your usual suspects are compression/decompression and
marshaling/unmarshalling.
for events.
spawn_blockingworks for both cases but is best suited for the latterbecause they don't steal CPU cycles from the executor.
So far, we have had a single thread per executor used for blocking
operations. This has been fine to issue the odd syscall that couldn't go
into the ring, but now that we have
spawn_blocking,we've openedpandora's box. Users will submit all sorts of things in there, including
long-running jobs that could stall the entire system.
This commit introduces a static thread pool per executor instead. The
pool is static for now, meaning the threads are created once and live
for as long as the host executor does. Later on, we may want to be
smarter and automatically up/downscale the pool depending on usage.
This work is based #500.