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

rt: Runtime refinements #2720

Closed
5 of 6 tasks
carllerche opened this issue Jul 30, 2020 · 17 comments · Fixed by #2956
Closed
5 of 6 tasks

rt: Runtime refinements #2720

carllerche opened this issue Jul 30, 2020 · 17 comments · Fixed by #2956
Assignees
Labels
A-tokio Area: The main tokio crate C-proposal Category: a proposal and request for comments M-runtime Module: tokio/runtime
Milestone

Comments

@carllerche
Copy link
Member

carllerche commented Jul 30, 2020

Summary

Based on experience gained in 0.2, there are a few aspects of the Runtime that can use improvements.

  • Remove basic_scheduler and threaded_scheduler variants from the public API.
  • Automatically use the basic_scheduler when core_threads is set to 0, or 1.
    • When core_threads is set to 1, use basic_scheduler and spawn it on a background thread.
    • When core_threads is set to 0, use basic_scheduler on the current thread.
  • Switch Runtime::block_on to take &self and remove tokio::runtime::Handle.
  • #[tokio::main] always uses the threaded scheduler and requires the rt-threaded scheduler.
    • Using #[tokio::main] panics without rt-threaded.
    • Using #[tokio::main(core_threads = 1)] results in a build failure.

Remaining

  • Initial pass rt: Refactor Runtime::block_on to take &self #2782
  • Support concurrent calls to block_on. Refactor basic_scheduler/shell to embed the block_on implementation within their own structs rather than within Runtime::block_on. This means making those block_on's take &self not &mut self. Improve stealing the root parker in situations where it gets reset back into the mutex and another thread needs to now drive it.
  • Refactor runtime::Builder to remove threaded_scheduler/basic_scheduler and refactor core_threads to do the behavior described in rt: Runtime refinements #2720.
  • For new_single_thread should spawn basic on a bg thread
  • Refactor #[tokio::main] as described in rt: Runtime refinements #2720.
  • Review runtime/macro docs to ensure consistency

Details

Removing basic_scheduler and threaded_scheduler

The details of which scheduler to pick isn't really what matters to the end-user. We already have core_threads to configure how many threads should be used to schedule tasks. The scheduler implementation can be selected internally based on the number of threads used.

Currently, basic_scheduler does not spawn a dedicated runtime thread. Instead, the current thread is used to schedule tasks. When block_on is called, the runtime starts running tasks and when block_on completes, the runtime is paused. This behavior can be gained again by setting core_threads = 0.

Using core_threads = 0 to signify running on the current thread might be a bit confusing. However, running the scheduler on the current thread is, in effect, requesting no runtime threads. This is also a somewhat specialized use case. Most users will use the threaded scheduler with default configuration settings.

Switch Runtime::block_on to take &self

The reason Runtime::block_on takes &mut self is to support the core_threads = 0 use case. When there are no runtime threads, the runtime is executed "in-place". The &mut self argument enforces that a single caller may "enter" the runtime so it is clear which thread executes the runtime.

However, there are valid cases where multiple threads may want to potentially share the responsibility of executing the runtime. In this case, the user is responsible for managing synchronization. Instead, Tokio can provide this functionality out of the box. When running with core_threads = 0 and calling block_on concurrently from multiple threads, the first thread that enters the runtime becomes responsible for executing the runtime. Other threads just block on the supplied task.

Making this change also has the property of removing the need for having a separate runtime::Handle. The Handle type was added because Runtime is !Sync. This change would make Runtime sync and threads could just use functions on Runtime instead of Handle.

Additional consideration must be taken to support concurrent calls to block_on when core_threads == 0. Consider the following:

  • Thread A calls block_on and takes the responsibility of driving the runtime
  • Thread B calls block_on and uses a regular thread parker.
  • Thread A's future completes and no longer drives the runtime.

At this point, thread B is waiting for its future to complete, but nothing is driving the runtime anymore, so the future will never unblock.

To handle this, when the thread that is driving the runtime completes the call to block_on, another blocked thread must be notified to start driving the runtime.

This can probably be implemented using a SyncParker<T: Park> utility that handles the hand off and implements Park for &Self.

#[tokio::main] always uses the threaded scheduler and requires the rt-threaded scheduler.

The behavior of #[tokio::main] changes based on feature flags that are enabled. When rt-threaded is not included, #[tokio::main] uses the basic scheduler. When it is enabled, the threaded scheduler is used. In general, changing behavior based on feature flags is not great. Feature flags are intended to be additive.

Additionally, Runtime::new() is only defined with rt-threaded.

Instead, #[tokio::main] will require rt-threaded and always have the same behavior. If rt-threaded is not the program will fail to build. The main macro can stll be used without the rt-threaded feature flag enabled, but core_threads will need to be set to 0 or 1. In this case, the basic scheduler is used.

The goal is to ensure consistent behavior regardless of which feature flags are selected.

Open question

Should #[tokio::main] / Runtime::new() be available if io-driver and time feature flags are not enabled? I.e. should the default functions be available when all feature flags are not enabled? I'm leaning towards allowing #[tokio::main] being available w/ only rt-threaded and not require 'io-driver/time`.

Refs: #2698

@carllerche carllerche added C-proposal Category: a proposal and request for comments A-tokio Area: The main tokio crate M-runtime Module: tokio/runtime labels Jul 30, 2020
@carllerche carllerche added this to the v0.3 milestone Jul 30, 2020
@carllerche carllerche mentioned this issue Jul 30, 2020
10 tasks
@hawkw
Copy link
Member

hawkw commented Jul 31, 2020

  • Using #[tokio::main] panics without rt-threaded.

Couldn't we just only expose main if rt-threaded is enabled? We can still expose other macros without the rt-threaded feature.

@LucioFranco
Copy link
Member

Can we still provide a single threaded macro too? Otherwise this looks good.

Should #[tokio::main] / Runtime::new() be available if io-driver and time feature flags are not enabled? I.e. should the default functions be available when all feature flags are not enabled? I'm leaning towards allowing #[tokio::main] being available w/ only rt-threaded and not require 'io-driver/time`.

I am curious what the use-case here would be with no io driver or timer?

@hawkw
Copy link
Member

hawkw commented Aug 4, 2020

I am curious what the use-case here would be with no io driver or timer?

IMO, yeah, it seems pretty reasonable to require users who want the shell runtime to have to actually go through the builder API...it's not a common configuration or what most folks are going to want out of the box.

I'm open to being convinced otherwise, of course.

@LucioFranco
Copy link
Member

We also likely want to remove runtime::Builder::num_threads in this release.

@Darksonn
Copy link
Contributor

Some thoughts: (partially from #2876 (comment))

core_threads

I think the suggested behavior with core_threads makes sense, however I have a suggestion: Do not provide a core_threads method on the runtime builder at all if rt-threaded is disabled. The behavior currently suggested in #2876 is to ignore it when rt-threaded is disabled, but this means we are back to the situation where behavior of code that compiles changes when feature flags are added.

#[tokio::main]

We could still provide a core_threads option on #[tokio::main] without rt-threaded, because here we can fail at compile time if it is set to a value larger than 1. In fact, setting it to 0 or 1 should probably be required.

time and io drivers

I think it makes perfect sense to provide the runtime without them. As for whether this counts as a change of behavior based on feature flags, any code that tries to use the drivers would fail to compile without the feature, so I think it's ok in this case.

A question

Should #[tokio::main(core_threads = 1)] be silently converted to core_threads = 0 on the builder?

@Darksonn
Copy link
Contributor

What about max_threads? I think we should consider saying that it should be strictly larger than core_threads, and perhaps rename it, as many new people think that this is the thing they need for configuring the number of core threads.

@Darksonn
Copy link
Contributor

As for the shell runtime, I think we should consider removing it entirely from the public API.

@carllerche
Copy link
Member Author

carllerche commented Sep 25, 2020

@Darksonn I generally agree w/ the sentiment. We should avoid changing behavior w/ feature flags.

One problem w/ removing core_threads when rt-threaded is missing is that core_threads=0 and core_theads=1 mean different things and neither option uses the work-stealing scheduler. We could have core_threads > 1 panic w/o the rt-threaded option...

Or, we change the API completely and add a "runtime flavor" option defined as an enum

enum Flavor {
    InPlace,
    SingleThreaded,
    WorkStealing(num_workers),
}

Another option:

enum Flavor {
    Isolated(num_workers),
    WorkStealing(num_workers),
}

We could then enable the variants using the feature flags. We could also consider renaming rt-threaded to rt-work-stealing. This would also open up the API to supporting multi-threaded basic-scheduler (fully isolated workers w/o stealing) as an option.

We could also pass the flavor to the builder new function.

@carllerche
Copy link
Member Author

@Darksonn I think I am OK w/ removing the shell runtime entirely.

@Darksonn
Copy link
Contributor

Note that the enum would have to be marked #[non_exhaustive].

@LucioFranco
Copy link
Member

Yeah, I think some way to enforce this stuff at compile time would be really nice. I think its extremely rare someone would want this runtime selection to be done implicitly like it currently does.

@Darksonn
Copy link
Contributor

Here's another option:

impl Builder {
    fn new_in_place() -> Builder;
    fn new_single_threaded() -> Builder;
    
    // Defaults to num-cpus threads
    #[cfg(feature = "rt-threaded")]
    fn new_work_stealing() -> Builder;
    
    #[cfg(feature = "rt-threaded")]
    fn core_threads(&mut self, num: usize) -> &mut Builder;
}

± naming

@carllerche
Copy link
Member Author

carllerche commented Oct 1, 2020

Edit: I updated this to factor in @Darksonn's feedback below.

In an effort to keep things simpler (hopefully), I propose we go roughly w/ what @Darksonn proposed:

impl Builder {
    fn new_in_place() -> Builder;
    fn new_single_threaded() -> Builder;
    
    // Defaults to num-cpus threads
    #[cfg(feature = "rt-work-stealing")]
    fn new_work_stealing(num_workers: usize) -> Builder;
}

We rename the rt-threaded feature flag to rt-work-stealing.

Then, for the main macro, we just keep a flat space:

#[tokio::main(flavor = "work_stealing", num_workers = 4)]

#[tokio::main(flavor = "in_place")]

We leave off single_threaded from the macro. And #[tokio::main] defaults to work_stealing and only succeeds when rt-work-stealing is present.

@Darksonn
Copy link
Contributor

Darksonn commented Oct 1, 2020

It looks good to me. My only concern is the naming of single threaded vs in place, since it seems like people who really wanted in_place might often go for single_threaded based on its name when using the #[tokio::main] macro. In some sense, single_threaded is actually two-threaded when including the main function.

@Darksonn
Copy link
Contributor

Darksonn commented Oct 1, 2020

One option is to only provide work_stealing and in_place, at least initially.

@LucioFranco
Copy link
Member

Why work-stealing over threaded? I personally like threaded since it feels easier to digest?

@Darksonn
Copy link
Contributor

Darksonn commented Oct 5, 2020

I have no strong feelings either way. I assume you're talking about the name of the Cargo feature.

LucioFranco added a commit that referenced this issue Oct 13, 2020
This PR updates the runtime module docs to the changes made in `0.3`
release of tokio.

Closes #2720
carllerche pushed a commit that referenced this issue Oct 13, 2020
This PR updates the runtime module docs to the changes made in `0.3`
release of tokio.

Closes #2720
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-proposal Category: a proposal and request for comments M-runtime Module: tokio/runtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants