-
Notifications
You must be signed in to change notification settings - Fork 1k
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
swarm/src/protocols_handler: Use FuturesUnordered in NodeHandlerWrapper #1775
Conversation
> Futures managed by FuturesUnordered will only be polled when they generate wake-up notifications. This reduces the required amount of work needed to poll large numbers of futures. https://docs.rs/futures/0.3.5/futures/stream/struct.FuturesUnordered.html Instead of iterating each inbound and outbound upgrade looking for one to make progress, use a `FuturesUnordered` for both pending inbound and pending outbound upgrades. As a result only those upgrades are polled that are ready to progress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the change in isolation is good, it might reveal buggy poll
implementations in ProtocolsHandler
s that wouldn't wake up the task when they should.
For Substrate in particular, we should thoroughly test it when updating libp2p.
Good point @tomaka . I will add a note in the changelog so we don't forget. |
@@ -2,7 +2,7 @@ | |||
name = "libp2p-swarm" | |||
edition = "2018" | |||
description = "The libp2p swarm" | |||
version = "0.22.0" | |||
version = "0.22.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am declaring this as a non-breaking change, thus bumping the patch version only. Let me know in case you prefer a minor version bump.
https://docs.rs/futures/0.3.5/futures/stream/struct.FuturesUnordered.html
Instead of iterating each inbound and outbound upgrade looking for one
to make progress, use a
FuturesUnordered
for both pending inbound andpending outbound upgrades. As a result only those upgrades are polled
that are ready to progress.