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

Consider defaulting Tokio features to off instead of on. #1791

Closed
carllerche opened this issue Nov 19, 2019 · 29 comments · Fixed by #1811
Closed

Consider defaulting Tokio features to off instead of on. #1791

carllerche opened this issue Nov 19, 2019 · 29 comments · Fixed by #1811
Labels
C-proposal Category: a proposal and request for comments

Comments

@carllerche
Copy link
Member

As per #1318, Tokio has been merged into a single crate and components are split by feature flag. Now, with all features enabled, the tokio crate is quite heavy.

Regardless of the direction, two things will happen:

  • documentation: the required feature flag for all components will be documented similar to how Tonic does this (see transport module here: https://docs.rs/tonic/0.1.0-alpha.6/tonic/).
  • meta feature: A full feature flag will be provided that enables all Tokio feature flags.

The question is, should default include all features, no features, or some features.

Default on

One of the main drawbacks mentioned in #1318 was that, when features are enabled by default, libraries will accidentally depend on more features than necessary. Doing so will force these features to be enabled by consumers of the library. Also, the end user can accidentally use features that were enabled by the dependency. When the dependency changes its feature flags, the application breaks as the required features are no longer available.

Default off

An alternative would be to default to no feature flags enabled by default. In this case, depending on tokio will only enable core traits (AsyncRead, AsyncWrite, ToSocketAddrs) and an empty Runtime type that doesn't do much when used. Getting started guides, examples, the README would instruct users to depend on tokio as:

tokio = { version = "0.2.0", features = ["full"] }

Libraries will be instructed to pick only the features they require. The primary drawback of this strategy is that it adds a bump to the getting started flow.

Default some

A middle ground would be to define a subset of features that should be enabled by default. It is unclear how to pick the features to enable by default as different Tokio users use significantly different feature sets. Because the choice is arbitrary, the end user will have no way to intuit if a feature is enabled by default or not.

@carllerche carllerche added the C-proposal Category: a proposal and request for comments label Nov 19, 2019
@jedisct1
Copy link

I'd vote for Default Off.

Having to add the set of features required by an example is unlikely to be an issue.

@anton-dutov
Copy link

anton-dutov commented Nov 19, 2019

IMO
default off
Better choice - Usually i use small subset of tokio features, may differs per project

default on
Bad choice, i think nobody use all features in single project

@tobz
Copy link
Member

tobz commented Nov 19, 2019

Proposal: end user vs author crates

There are two primary usage patterns: import tokio and have everything "just work" vs import only the absolutely required bits on tokio.

The first group typically consists of new users or people who are not bothered by extra dependencies, and they simply want to import one crate and be able to run examples, focus on building their application, and so on. The second group involves users who are either conscious of the extra dependencies being included that their applications do not require, or they're library authors looking to only pull in the absolute minimum of APIs to expose their crate's functionality.

Running with this idea of wanting to support both major use cases, I propose we expose two main crates: tokio and tokio-inner. (inner is just a filler word, really).

What would happen is that tokio-inner would effectively become what tokio is today: holding the majority of modules/code with feature flag differentiation. In turn, tokio would then re-export from tokio-inner. Crucially, tokio-inner would not have default features, while tokio would have defaults comparable to the existing full feature flag.

In this way, end users could continue to use tokio with a batteries-included set of default features, while more discerning users could choose to use tokio-inner and feature flag only what they needed.

Open questions:

  • would this technically work? (I think it would, but I'm not a cargo deps resolution expert)
  • do we actually think users in the "discerning" camp would be OK having to use a differently-named crate?
  • is there any versioning risk? (conceivably, both crates can be released in lockstep, but maybe not?)

@dekellum
Copy link
Contributor

I also think default-off is a better choice. The heavy use of features per #1318, warrants a more conservative, explicit opt-in to features at the application level.

I also wonder if meta-features like full in particular, and possibly others like net are really beneficial vs. a complication. I'll personally avoid using those, for example, for apps that don't use udp or uds. Given good documentation, it might be better for applications to make the upfront effort to specify their full set of required features. This is made easier for first time users by having the full set of features shown in a Cargo.toml dependency example of the guide/README. Then besides just a good list of features with descriptions at top level, the rustdoc should ideally note the feature required to gain access to a given module and/or its key items, e.g. "Foo is enabled by the special-foo feature flag".

@berkus
Copy link

berkus commented Nov 19, 2019

I'd say it's good to keep the default list to a bare necessary minimum.

full flag should be handy if you absolutely want to try everything out (also, for noobs just discovering what tokio has to offer). In the dependency graph, however, tokio is best kept to the required minimum footprint.

@swarmer
Copy link
Contributor

swarmer commented Nov 19, 2019

In case of default off it will be important to make instructions about these features highly visible:

  • In guides
  • Probably in API docs
  • in README
  • For people that jump straight to the example in README, maybe add a comment in the beginning like "// this example requires the following features: ...."

@hawkw
Copy link
Member

hawkw commented Nov 20, 2019

Running with this idea of wanting to support both major use cases, I propose we expose two main crates: tokio and tokio-inner. (inner is just a filler word, really).

how about...tokio and tokio-core 😉

@hwchen
Copy link

hwchen commented Nov 20, 2019

As mentioned in OP, default on is too heavy, default some is confusing.

I think that default off is the best, with good docs.

I also agree with some comments above; I would want the feature flag to be mentioned in the api docs, it would reduce the friction of having to jump to e.g. the readme.

@LucioFranco
Copy link
Member

I vote default off and shim crates with tokio itself bringing in everything and core default off. Good docs are key here and we should use the doc_cfg nightly feature like we do in tonic.

@swarmer
Copy link
Contributor

swarmer commented Nov 20, 2019

Isn't shim crate the same as default on in the sense that in both cases library authors need to be careful enough to do the right thing and not just depend on tokio, but with extra confusion of multiple crates?

@hawkw
Copy link
Member

hawkw commented Nov 20, 2019

Isn't shim crate the same as default on in the sense that in both cases library authors need to be careful enough to do the right thing and not just depend on tokio, but with extra confusion of multiple crates?

IMO, having two separate crates makes the distinction clearer and is much less error-prone. If the naming and documentation makes it clear when the two crates should be used, both application and library authors get the sensible behavior by default, with a simple $CRATE_NAME = "$VERSION" dependency in their Cargo.toml. On the other hand, with a single crate, regardless of whether we default on or default off, one category of users will always get the "wrong" behavior by default.

On a more general note, I think that optimizing for new-user experience is important and worth prioritizing. So much of the other work we've done for the 0.2 release has centered around improving new-user experience and accessibility: simplifying APIs, using names that are closer to std, and providing more batteries-included options like #[tokio::main] are all examples of this. If we default to disabling everything and don't provide some kind of obvious, easily-accessed batteries-included option, I feel like the #tokio-users chat channel will quickly become inundated with a steady stream of "I tried to compile this simple demo program, but it said it couldn't find any of these APIs?"-type questions. Having a batteries-included option is important, even if no "serious" or production users actually use it, just to avoid shipping something that appears "broken" out of the box for new users. IMO, having both a default-off "core" crate and a batteries-included facade crate is the best option for providing sensible defaults to both new users and experienced "power" users.

Just my 2¢!

@dekellum
Copy link
Contributor

IMO, the extra crate would be more of a complication than value in protecting new users from the feature concept. The main complication is that there would then be Cargo.toml dependency and rust code references to both crate names. For example, where would the examples live and which of the two names would they use?

New users may also be concerned by more dependencies and longer build times associated with all features, particularly when starting with a small application that only really needs minimal features.

Compare to tokio 0.1 where we needed to figure out which of many crates to depend on. Explicit features is much easier than that.

@hawkw
Copy link
Member

hawkw commented Nov 20, 2019

Hmm, I think choosing one of two crates is much easier than figuring out a large number of Cargo features, especially for new Rust users who may not even be aware that Cargo feature flags exist.
Basic examples and "Getting Started"-type documentation intended for newcomers would suggest the use of the batteries-included facade crate, and everything would "just work" out of the box. At the end of the documentation, there could be a note that users who want to pick and choose what features they depend on can use the default-off crate.

Again, I think it's really important that for someone just getting started, adding

tokio = "0.2"

to their Cargo.toml doesn't appear to be "totally broken" out of the box.

@myrrlyn
Copy link

myrrlyn commented Nov 20, 2019

Speaking solely as someone who periodically attempts to write a Tokio application essentially from scratch, I am more in favor of having "drop tokio = "version" in your manifest" be sufficient to "just work", even if it pulls more than I might immediately need.

From a UX perspective, encountering a "this doesn't exist" fault every time I want to try something that forces me to go look up how to enable it, and check if I even need it, is an unpleasant pattern. I would much prefer to have too much of a dependency graph by default, and once I have a better idea of what parts of Tokio I actually need, start disabling the things I don't.

Shipping a minimal-by-default crate that requires explicit lookup and opt-in for every new feature is, at least in my opinion, a premature optimization whose benefits in build expense are not sufficient to overcome the penalties on usability and exploration.

As an analogy, std is not opt-in either, even for libraries that might not ever use the parts of std that don't exist in core and alloc. It's a better experience to just have the kitchen sink, and then start turning things off that you realize you definitely don't need, than to have to pull each new component when you realize you need it. Removing things you've discovered you don't need at the end of a cycle is, again speaking purely personally, a preferable experience to having to select the things I do need at the start; by the end, I've generally learned enough to know what I do and don't need and can act more firmly, rather than speculatively.

@dekellum
Copy link
Contributor

I get your motivation, @hawkw, and others that want some form of default-everything option. But given tokio's current and likely future audience, I think the benefits might be overstated vs. the downsides (whether you choose to address those here or not).

To test my own suggestion, I turned examples/echo.rs into a free standing bin project, using master branch tokio and only the low-level features (default-features=false). While I'm not particularly familiar with the feature names and recent reorganization, it only took a few minutes to arrive at the following minimal features required:

features=["io-driver", "io-util", "tcp", "macros", "dns"]

Here is the build differences between the above "minimal" vs. using the "full" meta-feature:

minimal full diff
build time (user, secs) 27.4 37.3 +36%
total crates 18 24 +33%
build size (target/ MiB) 100 135 +35%

Each example could list the minimal features required as a comment, and/or this could be in the example's Cargo.toml.

I would agree that this, as a build optimization is in some sense "premature" for a new user—but the alternatives introduce a second way of doing it ("full" feature or a different crate). For the same user to take an application to production, they will most likely need to learn two different ways of doing it, as opposed to just the more optimal way.

@carllerche
Copy link
Member Author

@dekellum out of curiosity, what is the build size when macros is removed?

@dekellum
Copy link
Contributor

examples/echo.rs uses the macros, so I'd need to inline the setup stuff to test that?

@carllerche
Copy link
Member Author

Replace the macro w/ tokio::runtime::Runtime::new().unwrap().block_on(async { ... });

@carllerche
Copy link
Member Author

I wonder if we should have a runtime::main(async { ... }) function, it was removed because the above incantation isn't too hard.

@hawkw
Copy link
Member

hawkw commented Nov 21, 2019

I wonder if we should have a runtime::main(async { ... }) function, it was removed because the above incantation isn't too hard.

I think that could be a good idea. There are probably still enough use cases where the macro doesn't work for whatever reason...

@dekellum
Copy link
Contributor

I'll test this more-minimal build suggestion, tomorrow. If memory serves, the build size/time for similar example looks pretty good vs. tokio 0.1.

@hawkw
Copy link
Member

hawkw commented Nov 21, 2019

But given tokio's current and likely future audience, I think the benefits might be overstated vs. the downsides

@dekellum FWIW, I totally agree that we should optimize for a majority of tokio's user base, but I'm not convinced that it's accurately sampled by folks posting in this thread. Now that async/await syntax is stable, async Rust has gotten a lot more accessible, and I think we should be prepared to handle an influx of new users.

To test my own suggestion, I turned examples/echo.rs into a free standing bin project, using master branch tokio and only the low-level features (default-features=false). While I'm not particularly familiar with the feature names and recent reorganization, it only took a few minutes to arrive at the following minimal features required:

features=["io-driver", "io-util", "tcp", "macros", "dns"]

Here is the build differences between the above "minimal" vs. using the "full" meta-feature:
minimal full diff
build time (user, secs) 27.4 37.3 +36%
total crates 18 24 +33%
build size (target/ MiB) 100 135 +35%

Thanks for performing the tests, BTW — it's very helpful to be able to put actual numbers to these things instead of abstract concepts.

I notice that the of the features you enabled, io-driver requires the rt-core feature. Since rt-threaded is not enabled, the example would be run on the single-threaded scheduler. However, with all the features enabled, the thread pool scheduler would be used instead, which I expect adds a lot of code. Since there's a meaningful difference in performance between rt-core and rt-threaded, I'd be interested to see what the compile time and binary size look like if you also enabled rt-threaded.

As a side note, this kind of surprising behavior is the sort of thing that makes me wonder whether defaulting everything to off and expecting users to always pick and choose with feature flags is a good idea.

@hwchen
Copy link

hwchen commented Nov 21, 2019

I guess we’ll find out soon, but I’d be really curious as to whether Tokio’s new users will be more or less experienced in Rust. My feeling is that most people don’t reach for Tokio unless they have solid experience in rust (including features), as well as specific needs for lower-level async. I’d expect most people new to rust to be reaching for hyper or reqwest.

But, I suppose there could be an influx of systems programmers without rust experience who have been waiting for async/await.

In any case, I think it’s useful to think of onboarding specific groups as a part of deciding what features to have as a default. “New users” to tokio may be a pretty broad group.

@ultrasaurus
Copy link

I'd vote for default off and then cluster tutorials around features covering what should be turned on for each tutorial. (That may also help tokio authors validate whether the grouping of code into "features" is practical.)

Btw: I'm now to Rust and trying tokio is one of the first things I did. I don't think many people learn Rust in order to learn Rust. Native clients and edge servers traditionally needed to be built in C/C++ (when they require high performance and low memory footprint). Rust offers a way to do that with dramatically lower security risks. For those uses cases, a new Rust programmer would be looking at tokio as part of learning Rust.

@hawkw
Copy link
Member

hawkw commented Nov 21, 2019

To test my own suggestion, I turned examples/echo.rs into a free standing bin project, using master branch tokio and only the low-level features (default-features=false). While I'm not particularly familiar with the feature names and recent reorganization, it only took a few minutes to arrive at the following minimal features required:

features=["io-driver", "io-util", "tcp", "macros", "dns"]

Here is the build differences between the above "minimal" vs. using the "full" meta-feature:
minimal full diff
build time (user, secs) 27.4 37.3 +36%
total crates 18 24 +33%
build size (target/ MiB) 100 135 +35%

Thanks for performing the tests, BTW — it's very helpful to be able to put actual numbers to these things instead of abstract concepts.

I notice that the of the features you enabled, io-driver requires the rt-core feature. Since rt-threaded is not enabled, the example would be run on the single-threaded scheduler. However, with all the features enabled, the thread pool scheduler would be used instead, which I expect adds a lot of code. Since there's a meaningful difference in performance between rt-core and rt-threaded, I'd be interested to see what the compile time and binary size look like if you also enabled rt-threaded.

Out of curiosity, I ran a set of tests pretty similar to @dekellum's, but with the rt-threaded feature enabled to get an equivalent featureset when using --no-default-features. My understanding is that the compiler is better able to eliminate unused code from dependencies when link-time optimization (LTO) is enabled, so I also ran with and without LTO.

Note that, in order to properly determine the impact of LTO, I measured the size of the output executable, rather than the size of the entire target/ dir as @dekellum did.

Here are my results:

No LTO:

config time (debug) size (debug) time (release) size (release)
full 30.20s 3.2 MB 38.70s 732 KB
rt-core 13.58s 2.0 MB (-38%) 29.28s 572 KB (-21%)
rt-threaded 19.51s 3.0 MB (-6%) 35.50s 685 KB (-6%)

LTO:

config time (release+LTO) size (release+LTO)
full 55.64s 577k
rt-core 47.73s 437k (-24%)
rt-threaded 48.07s 540k (-6%)
Build & environment configurations

Environment

:; cargo --version && rustc --version
cargo 1.39.0 (1c6ec66d5 2019-09-30)
rustc 1.39.0 (4560ea788 2019-11-04)

:; uname -mprs
Darwin 18.6.0 x86_64 i386

Feature sets

full

[dependencies.tokio]
git = "https://github.com/tokio-rs/tokio.git"

rt-core

[dependencies.tokio]
git = "https://github.com/tokio-rs/tokio.git"
features = ["io-driver", "io-util", "tcp", "macros", "dns"]
default-features = false

rt-threaded

[dependencies.tokio]
git = "https://github.com/tokio-rs/tokio.git"
features = ["rt-threaded", "io-driver", "io-util", "tcp", "macros", "dns"]
default-features = false

LTO enabled

[profile.release]
lto = true

While there is still a difference in output binary size between the default features (everything on) and the set of feature flags required to enable the same behavior (rt-threaded), it's significantly less pronounced than when the threaded scheduler is disabled.

The percent difference in output executable size between "full" and "rt-threaded" is about 6%. This difference could be very important in some use-cases, but I think that most users to whom a 6% size difference is meaningful will already pick and choose a minimal set of features regardless of what the defaults are. It's probably much less significant to a beginner writing their first Rust program.

There is a noticeable difference in compile time, as well. I was interested to note that the compile time impact was much more pronounced in debug mode then in release or release+LTO — perhaps a lot of time is being spent generating debuginfo for the extra code? For new users, I suspect compile time differences probably matter a lot more than binary size. There's definitely a tradeoff between the ergonomic benefits of faster compilation vs those of not having to manually choose what features to enable.

Hopefully folks find this information at least somewhat helpful!

@dekellum
Copy link
Contributor

Your results may vary, but I also extended my original echo example build comparisons below! Stepping back a little, no one has suggested here that they want to take away the ability to explicitly opt-in to the low level features. From this early testing experience, I think that's what I'll be doing and recommended to others, including any new users.

no-macro minimal threaded full
build time (user, secs) 12.0 27.4 30.1 37.3
total crates 13 18 19 24
build size (target/ MiB) 46 100 111 135

no-macro

As per "minimal" described above, but removing the "macros" feature and replacing use of #[tokio::main] with an explicit Runtime constructor.

threaded

As per "minimal", but adding the rt-threaded feature. This only adds num_cpus as an external crate dependency.

I'd be interested to see what the compile time and binary size look like if you also enabled rt-threaded. As a side note, this kind of surprising behavior is the sort of thing that makes me wonder whether defaulting everything to off and expecting users to always pick and choose with feature flags is a good idea.

The echo server doesn't need a thread pool to function and serve its purpose as an example, so I wasn't surprised that it worked or that I wasn't using the thread pool, having skipped over that feature with my original goal of "minimal". Is the the non-threaded Runtime not a reasonable starting point? Then rt-threaded becomes a nice optimization option, and one can then consider if non-default Builder options are appropriate (number of threads, stack size, etc.)

@carllerche
Copy link
Member Author

@tobz What benefit does the "two crate" strategy offer over "default on"? The problem is providing guidance to lib authors that they should not depend on the kitchen sink. If lib authors have to know to depend on tokio-inner that sounds the same as knowing to depend on tokio w/ default-features = false.

@hawkw
Copy link
Member

hawkw commented Nov 21, 2019

I'd be interested to see what the compile time and binary size look like if you also enabled rt-threaded. As a side note, this kind of surprising behavior is the sort of thing that makes me wonder whether defaulting everything to off and expecting users to always pick and choose with feature flags is a good idea.

The echo server doesn't need a thread pool to function and serve its purpose as an example, so I wasn't surprised that it worked or that I wasn't using the thread pool, having skipped over that feature with my original goal of "minimal". Is the the non-threaded Runtime not a reasonable starting point? Then rt-threaded becomes a nice optimization option, and one can then consider if non-default Builder options are appropriate (number of threads, stack size, etc.)

I definitely don't think that the echo server needs the thread pool, or that the basic scheduler isn't an acceptable starting point. I just wanted to point out that the feature set you enabled was resulting in noticeably different behaviour than the feature set enabled by default, and that it would be interesting to also measure an option that resulted in an equivalent executable. :)


Unrelatedly, wow, the macros have a really big impact. Thanks for testing that, @dekellum! We should definitely consider bringing back a tokio::run function.

With that said, I would guess that the difference is mostly due to the syn/quote dependencies, not the tokio-macros crate itself. So, in a larger project that's using other proc macros as well, the impact of adding tokio-macros would be much less, provided the syn and quote dependency versions are compatible. But since #[tokio::main]/tokio::run are at least partially intended as batteries-included beginners features, we should definitely think about their impact in a project with no other dependencies.

@dekellum
Copy link
Contributor

The problem is providing guidance to lib authors that they should not depend on the kitchen sink.

If it can't be avoided completely, kitchen-sink, everything, omnibus, or smörgåsbord might be better extra crate suffixes or meta-feature names than full, so that the practical effect is more apparent, and serving as a reminder to go back and figure out the minimal features needed. 😀

…Okay, I had some fun with that, and I realize that only everything is practical for an international audience!

We should definitely consider bringing back a tokio::run function.

Thanks. Personally I think the Runtime constructor and block_on (or some spawn variant) is fine, actually preferable, because it composes with later adding a Builder to customize.

in a larger project that's using other proc macros as well, the impact of adding tokio-macros would be much less

Agreed. If the guidance is for explicit feature opt-in, then the barrier is reduced and it becomes easier for users to decide if they want the macro convenience or the smaller/faster build. It doesn't need to be a single answer, so much.

I was a bit skeptical regarding the move to the single tokio crate (and quiet on #1318), but while there is a lot of feature flags, even in advance of documentation, it seem to give the user a lot more control. Also in tokio 0.1 I was frequently confused between the leaf crates (e.g tokio-threadpool) types and wrapping types in the tokio crate. Now that is all gone. Thanks for all your work!

carllerche added a commit that referenced this issue Nov 22, 2019
Changes the set of `default` feature flags to `[]`. By default, only
core traits are included without specifying feature flags. This makes it
easier for users to pick the components they need.

For convenience, a `full` feature flag is included that includes all
components.

Tests are configured to require the `full` feature. Testing individual
feature flags will need to be moved to a separate crate.

Closes #1791
carllerche added a commit that referenced this issue Nov 22, 2019
Changes the set of `default` feature flags to `[]`. By default, only
core traits are included without specifying feature flags. This makes it
easier for users to pick the components they need.

For convenience, a `full` feature flag is included that includes all
components.

Tests are configured to require the `full` feature. Testing individual
feature flags will need to be moved to a separate crate.

Closes #1791
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-proposal Category: a proposal and request for comments
Projects
None yet
Development

Successfully merging a pull request may close this issue.