Move Spawn/AtomicWaker/ArcWake/FutureObj to futures-task

[taiki-e]

This is the idea I mentioned in #1893 (comment), and address some suggestions of #1893 (comment):

#1893 (comment)

Also, I was thinking of adding a new crate as a solution for #1830 (comment).
Personally, I think it might be a good idea to add a crate that provides "items contained in the futures::task module except for *Ext trait".

#1893 (comment)

• futures-core
  • Remove Spawn, LocalSpawn, SpawnError, and put them in futures-executor somehow. Separate Spawn/LocalSpawn from futures-core? #1830
  • Remove FutureObj and LocalFutureObj (they seem only needed for Spawn and friends?).
  • Remove AtomicWaker. It's only exposed in a doc(hidden) task::__internal module, but it doesn't seem like we need to compile it if only after the core traits.

Closes #1830

r? @cramertj
cc @seanmonstar @Nemo157

[cramertj]

This is great! What would you think about moving AtomicWaker back so that we can drop the dependency on futures-task from futures-channel? I think that FutureObj and Spawn are fairly unstable and possibly not long for this world, and it'd be a shame to build in a dependency on them from channels (though maybe it's not a huge deal).

cc @seanmonstar -- do you have thoughts here? Being able to share AtomicWaker seems good, and I think the way it's done currently leaves us lots of room to adapt in the future. I don't see much cost to having it shipped with futures-core as an unstable internal implementation detail.

[seanmonstar]

My personal feeling is that making futures-core as light as possible is a good goal. On one hand, AtomicWaker isn't that much code, but on the other, for example, I have no use for it in many things that rely on futures-core, so it'd seem nice to not need to compile it.

[cramertj]

it'd seem nice to not need to compile it.

Makes sense! I'm skeptical that it's introducing significant compile-time cost, and it seems likely that other things in an async project would potentially make use of it (e.g. anything using futures or futures-util). That said, since it isn't part of futures-core's stable API surface it seems fine to revisit in the future. I'd like to make as few commitments now as possible to give futures-rs room to evolve, which makes me concerned about offering AtomicWaker in futures-task or having futures-channel depend on futures-task.

[taiki-e]

What would you think about moving AtomicWaker back so that we can drop the dependency on futures-task from futures-channel?

I don't have a strong opinion. Also, by adding a new unstable feature like 'internal', you can compile it only when you need it. However, since stable crates should use futures-task, I don't think the benefits from this are big.

[cramertj]

Primarily I want to leave room for dropping the futures-task dependency from futures-channel in the future, which I'm concerned this wouldn't allow since this change would make futures-channel enable features in futures-task.

[carllerche]

Fwiw, long term, I plan to move away from using AtomicWaker in the Tokio channel types. I’ve found that there are more efficient, specialized strategies.

I mention this in case you want to avoid including AtomicWaker until you evaluate whether or not the futures types will use it long term.

[cramertj]

@carllerche Right, I think the right place to expose it is futures-util, which is where it was before this change, and is the crate that carries the loosest stability requirements.

[taiki-e]

@cramertj

Primarily I want to leave room for dropping the futures-task dependency from futures-channel in the future, which I'm concerned this wouldn't allow since this change would make futures-channel enable features in futures-task.

In this PR's approach, once we find an alternative to AtomicWaker, futures-channel can remove the dependency on futures-task.

Also, note that when defining AtomicWaker in futures-core, even if futures-channel no longer requires AtomicWaker, AtomicWaker cannot be removed from futures-core because need to maintain compatibility with older versions of futures-channel.

[Matthias247]

Regarding AtomicWaker and futures-core: Wouldn't futures-core be fully no-std compatible, only based on core, and not having any feature flags after those changes? If yes, then it would be great to keep it that way!

I would rather not recommend to expose AtomicWaker publically - purely for avoiding to send out the message that this is the preferred or recommended way to use a Waker API. It's a tool specific for some use-cases, and other use-cases might be better served with assigning Wakers in synchronized blocks (mutexes) that also protected the associated data. If someone wants to go the harder way use atomics in their application they should be fine with having their own copy of this code.

[cramertj]

@taiki-e

In this PR's approach, once we find an alternative to AtomicWaker, futures-channel can remove the dependency on futures-task.

I don't believe it can, though, since it enables feature flags in futures-task that wouldn't be present otherwise. If the dep gets dropped, those features will stop being enabled, and users using futures-task directly would see features go away.

I also think that AtomicWaker is not as fundamental as these traits, and is primarily a utility and should only be exposed in futures-util.

[taiki-e]

@cramertj
I agree with AtomicWaker should only be exposed in -util. so I've reverted dd664a3.

---

To keep a record, I'm still concerned about what I said in #1925 (comment):

Also, note that when defining AtomicWaker in futures-core, even if futures-channel no longer requires AtomicWaker, AtomicWaker cannot be removed from futures-core because need to maintain compatibility with older versions of futures-channel.

Given the current maintenance state of futures-channel, I don't think there will be any immediate changes, but I think this should be considered in the next minor (or major) version. (The way I said in #1925 (comment), or something else)

[cramertj]

Github seems to be having issues, so I've landed this manually.

[cramertj]

Thanks so much for the PR <3

[taiki-e]

By the way, this crate name (futures-task) is reserved by me, so I will hand off to @cramertj.