coop: expose an unconstrained() opt-out

[krallin]

Motivation

Some code doesn't work too well with Tokio coop.

For more context, see the discussion in:
#3516

Solution

This adds an opt-out for code that might not be compatible with Tokio
coop (e.g. heavily nested FuturesUnordered).

Notes to reviewer:

• I made the coop module public for this.
• I removed a few docs in said coop module that reference methods that
  don't exist yet.

[Darksonn]

This seems correct to me.

[Darksonn]

Please prefer pushing additional commits to commit --amend + push --force. It is easier to follow up with the history that way, and your commits are squashed on merge.

[krallin]

Please prefer pushing additional commits to commit --amend + push --force.

Sure; thanks for the heads up.

Should you need it, the update was just me fixing the typo that was pointed out earlier.

[jonhoo]

Should this only be available under cfg_coop! @carllerche ?

[krallin]

Wouldn't that mean that if you need to use this in a library you'd force your users to enable coop? (granted, I'm not sure familiar with features in Cargo, as I mentioned in that other thread we don't really use Cargo at $work :) )

[jonhoo]

Ah, I remember now. cfg_coop! is actually a short-hand for "if any of these features are enabled". It's not a feature in and of itself. The idea is that if you don't bring in a runtime or any resource types, then there's no reason to build the coop stuff. So I think cfg_coop! should also apply to these types.

[Darksonn]

That's a relatively complicated feature list. I might just prefer always exposing it, having it be a no-op without coop in the runtime.

[jonhoo]

I think this was at the request of @carllerche, though his opinions may have changed since?

[carllerche]

My gut is that this should live in the task module for the public API. I like what @Darksonn suggests, expose the fn anytime the task module is enabled but the impl changes based on cfg_coop. Either it does something or it is a noop.

[krallin]

My gut is that this should live in the task module for the public API. I like what @Darksonn suggests, expose the fn anytime the task module is enabled but the impl changes based on cfg_coop. Either it does something or it is a noop.

Should I be making this update in this PR then? I'm not certain exactly what y'all would like me to change to get this moving forward :)

[Darksonn]

Yes, please go ahead and make this change.

[krallin]

Sounds good; thanks; I just updated the PR.

[Darksonn]

Since we moved the function to tokio::task, this shouldn't be made public.

[krallin]

I kept it public so we could have access to some documentation for the coop module. Otherwise, there isn't much to explain what the wrapper does, and documenting the coop mechanism in the wrapper to disable it seems a bit counter-intuitive.

Thoughts?

[Darksonn]

Hm, good point. I think I would still prefer to put it on the module docs of an existing module.

[krallin]

I don't mind — where should it live then?

[Darksonn]

It should probably go on tokio::task or tokio::runtime is my thought.

[krallin]

I moved this into tokio::task and made tokio::coop pub(crate) again.

[Darksonn]

I think this looks pretty good to me. I have some doc-related comments below:

[Darksonn]

I'm not sure which version I prefer doc-wise, but this could also be written without the Unpin bound:

Suggested change

	//! # use tokio_stream::{Stream, StreamExt};
	//! async fn drop_all<I: Stream + Unpin>(mut input: I) {
	//! while let Some(_) = input.next().await {}
	//! }
	//! # use tokio_stream::{Stream, StreamExt};
	//! async fn drop_all<I: Stream>(input: I) {
	//! tokio::pin!(input);
	//! while let Some(_) = input.next().await {}
	//! }

[krallin]

I really just ported this one from the existing docs, but I'm happy to change it, sure

[Darksonn]

I don't love this being lowercase.

[krallin]

I pattern-matched from the other functions in there, e.g. yield_now:

tokio/tokio/src/task/mod.rs

Lines 185 to 192 in 704de8c

	//! #### yield_now
	//!
	//! In addition, this module provides a [`task::yield_now`] async function
	//! that is analogous to the standard library's [`thread::yield_now`]. Calling
	//! and `await`ing this function will cause the current task to yield to the
	//! Tokio runtime's scheduler, allowing other tasks to be
	//! scheduled. Eventually, the yielding task will be polled again, allowing it
	//! to execute. For example:

Should we change them all?

[Darksonn]

The two doc things I commented are not that important. Merging.