Provide optional features on tokio crate

[seanmonstar]

The main point of the top-level tokio crate is the runtime, many of the other parts of convenience imports. It would be useful to be able to disable any that are not needed, such as codec or udp, for example.

Motivation

I'd like to provide runtime features in other crates, like hyper, but don't need all the other dependencies of tokio (like tokio-udp, tokio-fs, tokio-uds, etc). If I could just depend on this sub-crate, the number of dependencies would get a bit smaller.

Tasks

• Make most parts of API optional
• Make tokio::run use current_thread::run if threadpool isn't enabled.

[seanmonstar]

@carllerche what do you think of the end result? If it seems like a good idea, I'll fix up the remaining pieces.

[carllerche]

I'd rather only have separate crates for types that are expected to be in the public API of downstream libraries.

Instead of a separate crate, what if we allow isolating tokio features in the main crate using feature flags? Then, to only pull in the runtime, you would depend on tokio w/ no default features + runtime.

Alternatively, I was thinking of moving all of tokio into tokio-util and have tokio-util be no default features...

I would prefer the first option as it is fewer crates.

[seanmonstar]

@carllerche Ok, I've redone it to be a set of cargo features.

• There is now (default to on) codec, fs, tcp, udp, and uds "extra" features.

• As a start of being able to disable the extra runtimes that a crate may not use, this also includes current-thread and threadpool features. The current-thread feature was pretty easy to add in. To prevent breaking changes in the future, a threadpool feature that's not really optional is also included. Adding in optional threadpool configs everywhere will be more annoying, so I left that out.

  The reason for this is so that we can at a future date make the threadpool runtime fully optional, without users "forgetting" to enable it. If we didn't have this feature, and later tried to make it optional, users would see breakage if they had disabled default features.

[carllerche]

Thanks for doing this. In your opinion, once a breaking change happens, what features should be on by default vs. off by default?

[seanmonstar]

I'm not sure what you mean. What about a breaking change? I don't believe this PR is breaking.

Do you mean for tokio 0.2, should we remove any of these from the default?

[carllerche]

Yes, in 0.2 should anything be removed from default?

[seanmonstar]

I don't have an opinion on if any should be removed by default...

[carllerche]

@seanmonstar could you elaborate on why the threadpool feature is not optional? Is it because tokio::run depends on it?

[carllerche]

I will try to summarize the discussion we had.

• no-default-features should only include futures as a dependency and should only enable code that can work with only futures as a dep.
• The *Ext traits (FutureExt, StreamExt) will always exist, even if they are empty.
• io will be a feature pulling in tokio-io.
• reactor will be a feature pulling in tokio-reactor.
• tcp, udp, uds will depend on reactor and possibly io.

Features in Tokio will be used to limit the number of dependencies. As such, code in tokio should be enabled if all required crate dependencies are enabled (via features).

[carllerche]

Tokio will only provide components as sub crates if those types are intended to be used in dependent crates' public API (i.e. crates not part of Tokio). All code that is intended to remain implementation details for Tokio users are provided as part of the main tokio crate.

The logic being that sub crates are useful for limiting breaking changes for downstream dependencies.

[carllerche]

For bonus points, it would be super awesome for tokio::run to default to threadpool, but fallback to the current-thread runtime if the threadpool feature is disabled but the current-thread feature is enabled.

[davidbarsky]

I like the levels of feature flags noted by Carl in #808 (comment).

As for:

For bonus points, it would be super awesome for tokio::run to default to threadpool, but fallback to the current-thread runtime if the threadpool feature is disabled but the current-thread feature is enabled.

I am a fan of this. I'm guessing it'll prevent bugs like hyperium/hyper#1736 in the future?

[seanmonstar]

I've tried making everything optional besides futures. Seems the CI failure is something about nightly and set_pushed not being used or something.

[carllerche]

This seems like a legit warning. Fix here #822.

[ghost]

@carllerche I like the plan for adding features as outlined. 👍

And I especially love how Tokio can be tuned for single-threaded or multithreaded use through a simple Cargo feature flag without having to figure out how to set up the Tokio runtime.

Choose features you need and launch with tokio::run(). That sounds wonderful!

[seanmonstar]

Latest commit adds tokio::runtime::current_thread::run, which is the counterpart to tokio::runtime::threadpool::run, and then makes tokio::run use it if threadpool is disabled.

CI failure is due to a broken master, waiting on #825.

I've updated the main comment with a task list, I believe that's everything.

[seanmonstar]

And all green!

[carllerche]

Besides the couple of thoughts I commented on above, this looks good to me.

Now, how do we make sure we didn't accidentally break anything....

[seanmonstar]

AppVeyor had a spurious error.

[carllerche]

To summarize the discussion in Gitter:

• threadpool and current-thread feature flags will not be used immediately and are reserved for building tokio such that tokio::run starts an executor with no drivers (no reactor, timer, etc...).
• A collection of rt-* feature flags will be used to track a few common combinations of tokio feature flags.
• This PR will only include a rt-full feature flag as an initial step (omitting threadpool and current-thread).

At a future time, we can add additional feature flags in a backwards compatible way.

[seanmonstar]

Done.

[carllerche]

LGTM 👍 Thanks for doing this.

[jonhoo]

I missed this over the winter break. Overall, it looks good, though I'm a bit worried about this:

The *Ext traits (FutureExt, StreamExt) will always exist, even if they are empty.

Cargo features are supposed to be additive, otherwise all sorts of weird and unpredictable errors start happening. In particular, a crate that depends on tokio with no-default-features should still work correctly even if features are added. This would not be the case if some feature flag changes the members of a trait. If the no-default-features crate impls that trait, it would no longer compile if used with a tokio that has other features enables, which cargo will commonly do to avoid duplicating dependencies.

See in particular all the pain around nom's verbose-errors feature

[carllerche]

@jonhoo I’m not following the problem vector.

Afaik, the features should be additive.

Keep in mind that FutureExt has Future as a super trait and it includes a blanket impl. This prevents any implementations outside of the crate.

[jonhoo]

Ah, I see, that was the missing piece! As long as it's effectively sealed, I don't think it raises an issue!