Excessive calls to `poll` and cloning of `Waker`s.
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 5.6k
- Forks
- 1.3k
- Avg merge
- 8h 47m
- Merged PRs (30d)
- 19
Description
Summary
I perfed my protocol and discovered major hot spots within the repo code mostly ending up in libp2p_swarm::connection::Connection<THandler>::poll and yamux implementation. From the looks of it the "nothing to do" paths are constantly being hit and subsequently the Arcs dropped and Wakers cloned. I quickly noticed that code usually tries to poll everything anytime one central Waker triggers.
Expected behavior
Ideally, code selecting many futures in parallel should branch the main Waker for each future to collect information on what actually needs polling.
Actual behavior
Most of the hot instructions are doing atomic operations within Waker::clone and code almost always looks like this:
pub fn set_waker(old: &mut Option<Waker>, new: &Waker) {
*old = Some(new.clone());
}
The most significant hot spot though is something different. here we unconditionally create Arc for each available protocol as the last thing before breaking the loop. The crux of the problem is that we call the poll to drop some Arcs and clone Wakers most of the time with no real work getting done.
Relevant log output
No response
Possible Solution
Simplest fix for updating cached wakers is refactoring the function to:
pub fn set_waker(old: &mut Option<Waker>, new: &Waker) {
if let Some(old) = old {
// clone_from will clone only if `new` won't wake the `old`
old.clone_from(new);
} else {
*old = Some(new.clone());
}
}
Although this is just fixing the symptom, its still a substantial improvement considering how easy it is to implement.
Another simple fix is avoiding the Arc creation in mentioned section of the code. I managed to pull it off but not without breaking the API (set intersection and difference iterators getting replaced with SmallVecs in ProtocolChange events).
Now for the main cause of all this...
Excessive Polling
All the mentioned code is almost exclusively placed at spots where handwritten futures ran out of things to do, this is also the code that is executed all the time. My conclusion is that we poll when we don't need to. Solving this though is hard and requires lot of changes, and careful testing (since eliminating excessive polls is painful since bugs exhibit as program getting blocked forever).
Version
b7914e407da34c99fb76dcc300b3d44b9af97fac
Would you like to work on fixing this bug ?
Yes
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at swarm/src/connection.rs near Connection::poll and inspect the yamux implementation, including set_waker and the ProtocolChange handling described in the issue. Use the reported perf hotspots as a baseline, then verify that unnecessary polling, Waker cloning, and Arc creation are reduced without causing futures to stop making progress.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- distributed-systems, networking, performance
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100