feat(swarm): keep Connections on the same Task

[dignifiedquire]

We now upgrade a PendingConnection on the same Task, instead of moving it back to the main Task, and spawning another one.

@mxinden this is a first pass at what we talked about, in regards to keeping the connection on a single task once established.

Depends on #2535

[mxinden]

Mind splitting the PR in two? https://github.com/libp2p/rust-libp2p/pull/2536/commits/0c5fa28b5a97276cb4ccbb9335b6588253d551e7 we can merge soon. https://github.com/libp2p/rust-libp2p/pull/2536/commits/88cc599dfde60cecc06dd827dfc1add4c1be54ef I want to give a bit more thought.

[dignifiedquire]

@mxinden https://github.com/libp2p/rust-libp2p/pull/2535

[thomaseizinger]

Thanks @dignifiedquire ! Do you mind adding some rationale on why this is better? :)

Is it supposedly easier to understand? Less susceptible to certain error conditions? Better encapsulation?

[dignifiedquire]

Is it supposedly easier to understand? Less susceptible to certain error conditions? Better encapsulation?

The idea of doing this, arose from me working on trying to integrate rust-libp2p with https://github.com/DataDog/glommio/. With that executor, connections are not necessarily Send, and so I was piece wise figuring out how to avoid moving things between threads. With this change, in theory, a connection can be guranteed to be kept on a single thread after having been spawned (assuming that is what the executor does of course).

[thomaseizinger]

Thanks for that!

I'll have a look with that in mind later :)

[thomaseizinger]

I think in principle, I am okay with this direction.

The idea of doing this, arose from me working on trying to integrate rust-libp2p with DataDog/glommio. With that executor, connections are not necessarily Send, and so I was piece wise figuring out how to avoid moving things between threads.

Does this mean, only the final connection is !Send but the task for creating the connection is Send? With https://github.com/libp2p/rust-libp2p/pull/2652, we will be emitting individual "upgrade" futures from a Transport (and Transport will no longer be Clone). Consequently, there will be a task that polls the underlying transport but the upgrade will be put into its own task which might be on another thread.

[mergify[bot]]

This pull request has merge conflicts. Could you please resolve them @dignifiedquire? 🙏

[mergify[bot]]

This pull request has merge conflicts. Could you please resolve them @dignifiedquire? 🙏

[thomaseizinger]

I guess we can close this as stale.