From 02d87155a5a0b056217095de9fd2e28800e1b14d Mon Sep 17 00:00:00 2001 From: DrHuangMHT Date: Tue, 2 May 2023 19:13:41 +0800 Subject: [PATCH] feat(mdns): change `mdns::Event` to hold `Vec` In previous PR #3606 we've made `mdns::Event` `Clone`, but cloning single-use iterators doesn't sound right. Also you have to create an iterator from the actual data returned before putting it into events. So in this PR the iterators are replaced by `Vec`, as it's the type the data originally come from. Related #3612. Pull-Request: #3621. --- protocols/mdns/CHANGELOG.md | 5 ++ protocols/mdns/src/behaviour.rs | 72 +++------------------------ protocols/mdns/tests/use-async-std.rs | 20 ++++---- protocols/mdns/tests/use-tokio.rs | 16 +++--- 4 files changed, 29 insertions(+), 84 deletions(-) diff --git a/protocols/mdns/CHANGELOG.md b/protocols/mdns/CHANGELOG.md index 7cc9ed0f..c6c61c0c 100644 --- a/protocols/mdns/CHANGELOG.md +++ b/protocols/mdns/CHANGELOG.md @@ -1,8 +1,12 @@ ## 0.44.0 - unreleased +- Change `mdns::Event` to hold `Vec` and remove `DiscoveredAddrsIter` and `ExpiredAddrsIter`. + See [PR 3621]. + - Raise MSRV to 1.65. See [PR 3715]. +[PR 3621]: https://github.com/libp2p/rust-libp2p/pull/3621 [PR 3715]: https://github.com/libp2p/rust-libp2p/pull/3715 ## 0.43.1 @@ -10,6 +14,7 @@ - Derive `Clone` for `mdns::Event`. See [PR 3606]. [PR 3606]: https://github.com/libp2p/rust-libp2p/pull/3606 + ## 0.43.0 - Update to `libp2p-core` `v0.39.0`. diff --git a/protocols/mdns/src/behaviour.rs b/protocols/mdns/src/behaviour.rs index 92e38c04..5186ce91 100644 --- a/protocols/mdns/src/behaviour.rs +++ b/protocols/mdns/src/behaviour.rs @@ -285,7 +285,7 @@ where } } // Emit discovered event. - let mut discovered = SmallVec::<[(PeerId, Multiaddr); 4]>::new(); + let mut discovered = Vec::new(); for iface_state in self.iface_states.values_mut() { while let Poll::Ready((peer, addr, expiration)) = iface_state.poll(cx, &self.listen_addresses) @@ -304,15 +304,13 @@ where } } if !discovered.is_empty() { - let event = Event::Discovered(DiscoveredAddrsIter { - inner: discovered.into_iter(), - }); + let event = Event::Discovered(discovered); return Poll::Ready(ToSwarm::GenerateEvent(event)); } // Emit expired event. let now = Instant::now(); let mut closest_expiration = None; - let mut expired = SmallVec::<[(PeerId, Multiaddr); 4]>::new(); + let mut expired = Vec::new(); self.discovered_nodes.retain(|(peer, addr, expiration)| { if *expiration <= now { log::info!("expired: {} {}", peer, addr); @@ -323,9 +321,7 @@ where true }); if !expired.is_empty() { - let event = Event::Expired(ExpiredAddrsIter { - inner: expired.into_iter(), - }); + let event = Event::Expired(expired); return Poll::Ready(ToSwarm::GenerateEvent(event)); } if let Some(closest_expiration) = closest_expiration { @@ -342,67 +338,11 @@ where #[derive(Debug, Clone)] pub enum Event { /// Discovered nodes through mDNS. - Discovered(DiscoveredAddrsIter), + Discovered(Vec<(PeerId, Multiaddr)>), /// The given combinations of `PeerId` and `Multiaddr` have expired. /// /// Each discovered record has a time-to-live. When this TTL expires and the address hasn't /// been refreshed, we remove it from the list and emit it as an `Expired` event. - Expired(ExpiredAddrsIter), -} - -/// Iterator that produces the list of addresses that have been discovered. -#[derive(Clone)] -pub struct DiscoveredAddrsIter { - inner: smallvec::IntoIter<[(PeerId, Multiaddr); 4]>, -} - -impl Iterator for DiscoveredAddrsIter { - type Item = (PeerId, Multiaddr); - - #[inline] - fn next(&mut self) -> Option { - self.inner.next() - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - self.inner.size_hint() - } -} - -impl ExactSizeIterator for DiscoveredAddrsIter {} - -impl fmt::Debug for DiscoveredAddrsIter { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt.debug_struct("DiscoveredAddrsIter").finish() - } -} - -/// Iterator that produces the list of addresses that have expired. -#[derive(Clone)] -pub struct ExpiredAddrsIter { - inner: smallvec::IntoIter<[(PeerId, Multiaddr); 4]>, -} - -impl Iterator for ExpiredAddrsIter { - type Item = (PeerId, Multiaddr); - - #[inline] - fn next(&mut self) -> Option { - self.inner.next() - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - self.inner.size_hint() - } -} - -impl ExactSizeIterator for ExpiredAddrsIter {} - -impl fmt::Debug for ExpiredAddrsIter { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt.debug_struct("ExpiredAddrsIter").finish() - } + Expired(Vec<(PeerId, Multiaddr)>), } diff --git a/protocols/mdns/tests/use-async-std.rs b/protocols/mdns/tests/use-async-std.rs index 139fcca1..bfc3cd12 100644 --- a/protocols/mdns/tests/use-async-std.rs +++ b/protocols/mdns/tests/use-async-std.rs @@ -61,13 +61,13 @@ async fn test_expired_async_std() { loop { match futures::future::select(a.next_behaviour_event(), b.next_behaviour_event()).await { - Either::Left((Event::Expired(mut peers), _)) => { - if peers.any(|(p, _)| p == b_peer_id) { + Either::Left((Event::Expired(peers), _)) => { + if peers.into_iter().any(|(p, _)| p == b_peer_id) { return; } } - Either::Right((Event::Expired(mut peers), _)) => { - if peers.any(|(p, _)| p == a_peer_id) { + Either::Right((Event::Expired(peers), _)) => { + if peers.into_iter().any(|(p, _)| p == a_peer_id) { return; } } @@ -93,8 +93,8 @@ async fn test_no_expiration_on_close_async_std() { // 1. Connect via address from mDNS event loop { - if let Event::Discovered(mut peers) = a.next_behaviour_event().await { - if let Some((_, addr)) = peers.find(|(p, _)| p == &b_peer_id) { + if let Event::Discovered(peers) = a.next_behaviour_event().await { + if let Some((_, addr)) = peers.into_iter().find(|(p, _)| p == &b_peer_id) { a.dial_and_wait(addr).await; break; } @@ -130,13 +130,13 @@ async fn run_discovery_test(config: Config) { while !discovered_a && !discovered_b { match futures::future::select(a.next_behaviour_event(), b.next_behaviour_event()).await { - Either::Left((Event::Discovered(mut peers), _)) => { - if peers.any(|(p, _)| p == b_peer_id) { + Either::Left((Event::Discovered(peers), _)) => { + if peers.into_iter().any(|(p, _)| p == b_peer_id) { discovered_b = true; } } - Either::Right((Event::Discovered(mut peers), _)) => { - if peers.any(|(p, _)| p == a_peer_id) { + Either::Right((Event::Discovered(peers), _)) => { + if peers.into_iter().any(|(p, _)| p == a_peer_id) { discovered_a = true; } } diff --git a/protocols/mdns/tests/use-tokio.rs b/protocols/mdns/tests/use-tokio.rs index e18ae28f..22941843 100644 --- a/protocols/mdns/tests/use-tokio.rs +++ b/protocols/mdns/tests/use-tokio.rs @@ -59,13 +59,13 @@ async fn test_expired_tokio() { loop { match futures::future::select(a.next_behaviour_event(), b.next_behaviour_event()).await { - Either::Left((Event::Expired(mut peers), _)) => { - if peers.any(|(p, _)| p == b_peer_id) { + Either::Left((Event::Expired(peers), _)) => { + if peers.into_iter().any(|(p, _)| p == b_peer_id) { return; } } - Either::Right((Event::Expired(mut peers), _)) => { - if peers.any(|(p, _)| p == a_peer_id) { + Either::Right((Event::Expired(peers), _)) => { + if peers.into_iter().any(|(p, _)| p == a_peer_id) { return; } } @@ -86,13 +86,13 @@ async fn run_discovery_test(config: Config) { while !discovered_a && !discovered_b { match futures::future::select(a.next_behaviour_event(), b.next_behaviour_event()).await { - Either::Left((Event::Discovered(mut peers), _)) => { - if peers.any(|(p, _)| p == b_peer_id) { + Either::Left((Event::Discovered(peers), _)) => { + if peers.into_iter().any(|(p, _)| p == b_peer_id) { discovered_b = true; } } - Either::Right((Event::Discovered(mut peers), _)) => { - if peers.any(|(p, _)| p == a_peer_id) { + Either::Right((Event::Discovered(peers), _)) => { + if peers.into_iter().any(|(p, _)| p == a_peer_id) { discovered_a = true; } }