swarm: Make API for creating a new Swarm executor aware

[thomaseizinger]

Description

This issue is extracted out of #2173 and proposes a solution to issues like #2230.

A Swarm needs to execute background tasks. Currently, each physical connection (i.e. a TCP connection) is executed on its own background task. This improves latency because it means the main event loop of the Swarm which also calls NetworkBehaviour::poll does not get blocked by IO. Also see #2885.

Today, the way a Swarm achieves this is very subtle.

By default, Swarm::new will try to create a futures::executor::Threadpool to execute these connection background tasks. For async-IO types from the tokio runtime, this does not work because they require a tokio reactor to run on the same thread which by default is only active on a worker thread of a tokio Runtime. To achieve this, a user has to use the SwarmBuilder instead and call executor(Box::new(|f| { tokio::spawn(f) })). See for example here:

rust-libp2p/examples/chat-tokio.rs

Lines 109 to 115 in b42f286

	SwarmBuilder::new(transport, behaviour, peer_id)
	// We want the connection background tasks to be spawned
	// onto the tokio runtime.
	.executor(Box::new(|fut| {
	tokio::spawn(fut);
	}))
	.build()

In case creation of the futures::executor::Threadpool fails, the connection tasks are polled on the current thread instead:

rust-libp2p/swarm/src/connection/pool.rs

Lines 398 to 404 in b42f286

	fn spawn(&mut self, task: BoxFuture<'static, ()>) {
	if let Some(executor) = &mut self.executor {
	executor.exec(task);
	} else {
	self.local_spawns.push(task);
	}
	}

rust-libp2p/swarm/src/connection/pool.rs

Lines 101 to 103 in b42f286

	/// If no `executor` is configured, tasks are kept in this set and
	/// polled on the current thread when the [`Pool`] is polled for new events.
	local_spawns: FuturesUnordered<Pin<Box<dyn Future<Output = ()> + Send>>>,

I am proposed to change the API for Swarm and SwarmBuilder to force the user to make a decision on which executor they'd like their tasks to be executed on. There are several options and it is not yet clear, what the best one is:

• Introduce a type parameter on Swarm that has a trait-bound for spawning new tasks. This would allow us to re-export type-aliases like libp2p::swarm::tokio::Swarm which would point to something like libp2p::swarm::Swarm<TokioRuntime>. A type-parameter-only solution is likely the easiest to migrate too because it doesn't require any signature changes. We can deprecate the current type in favor of libp2p::swarm::futures_threadpool::Swarm.
• Introduce a new parameter to libp2p::swarm::Swarm::new that requires to pass an executor-specific type:

  impl Swarm {
      fn new<T>(..., executor: T) -> Self { }
  }
  
  Swarm::new(..., TokioExecutor);

There are likely other solutions too. Whatever we choose, it must play nicely with cargo features, i.e. be completely additive in terms of APIs and not change any behaviour.

Motivation

• Make it easier for users to use rust-libp2p correctly.
• Less boilerplate to configure a custom executor.
• Treat all executors equally.

Open questions

Are you planning to do it yourself in a pull request?

Maybe.

[umgefahren]

I'm currently working on a PR to solve this issue. However, I have a small question: Solving this could remove the need for local spawns since FuturesUnordered could also be treated as an executor. I don't see any usages of local spawns besides being a fallback.

[thomaseizinger]

I'm currently working on a PR to solve this issue. However, I have a small question: Solving this could remove the need for local spawns since FuturesUnordered could also be treated as an executor. I don't see any usages of local spawns besides being a fallback.

It doesn't quite work unfortunately.
FuturesUnordered requires &mut self to spawn new futures and it needs to be polled to make progress. Both of these are not needed typically for the executors we are interested in.

[umgefahren]

I see, however, if there is an executor, there is no need for local spawns, right? So I could just construct an enum encapsulating the different cases.

[thomaseizinger]

I see, however, if there is an executor, there is no need for local spawns, right? So I could just construct an enum encapsulating the different cases.

That is correct. It is either one or the other :)

[umgefahren]

This issue is closed by #3097. Consider linking it.

[thomaseizinger]

This issue is closed by #3097. Consider linking it.

Done!