Overhaul stats: refactor `Swarm::remove_inactive` method #1527

josecelano · 2025-05-19T15:36:37Z

Some days ago, I refactored the method:

pub struct Swarm {
    info_hash: InfoHash,
    peers: BTreeMap<SocketAddr, Arc<PeerAnnouncement>>,
    metadata: SwarmMetadata,
    event_sender: Sender,
}

impl Swarm {
    // ...
    pub async fn remove_inactive(&mut self, current_cutoff: DurationSinceUnixEpoch) -> usize {
        let peers_to_remove = self.inactive_peers(current_cutoff);

        for peer_addr in &peers_to_remove {
            self.remove_peer(peer_addr).await;
        }

        peers_to_remove.len()
    }

I made some changes, but there is one change relevant to this issue. It was using the "retain" collection method, and it's not using it anymore.

This is the previous version where it was still using the peers.retain:

pub fn remove_inactive(&mut self, current_cutoff: DurationSinceUnixEpoch) -> u64 {
    let mut inactive_peers_removed = 0;

    self.peers.retain(|_, peer| {
        let is_active = peer::ReadInfo::get_updated(peer) > current_cutoff;

        if !is_active {
            // Update the metadata when removing a peer.
            if peer.is_seeder() {
                self.metadata.complete -= 1;
            } else {
                self.metadata.incomplete -= 1;
            }

            inactive_peers_removed += 1;
        }

        is_active
    });

    inactive_peers_removed
}

Then I continue using retain but the event sender function changed to asynchronous, and "retain" does not allow asynchronous code. Therefore I had to extract the asynchronous part:

    pub async fn remove_inactive(&mut self, current_cutoff: DurationSinceUnixEpoch) -> usize {
        let mut number_of_peers_removed = 0;
        let mut removed_peers = Vec::new();

        self.peers.retain(|_key, peer| {
            let is_active = peer::ReadInfo::get_updated(peer) > current_cutoff;

            if !is_active {
                // Update the metadata when removing a peer.
                if peer.is_seeder() {
                    self.metadata.complete -= 1;
                } else {
                    self.metadata.incomplete -= 1;
                }

                number_of_peers_removed += 1;

                if let Some(_event_sender) = self.event_sender.as_deref() {
                    // Events can not be trigger here because retain does not allow
                    // async closures.
                    removed_peers.push(*peer.clone());
                }
            }

            is_active
        });

        if let Some(event_sender) = self.event_sender.as_deref() {
            for peer in &removed_peers {
                event_sender
                    .send(Event::PeerRemoved {
                        info_hash: self.info_hash,
                        peer: *peer,
                    })
                    .await;
            }
        }

        number_of_peers_removed
    }

The problem is that after that change you need two loops:

One to delete the peers
Another to send the events

In the current version, we have one loop to get the list of removed and another loop to remove them without "retain".

This solution is worse because:

It requires two loops.
Depending on the implementation, we must store a temporary list of inactive peers or peers pending removal. It can be a long list.

I was [researching alternatives](https://github.com/josecelano/learning-
rust/issues/3). It seems we can use a simple "while" to implement a "retain" if you need async code in the condition. I have not tried it.

cc @da2ce7

The text was updated successfully, but these errors were encountered:

josecelano added Code Cleanup / Refactoring Tidying and Making Neat Optimization Make it Faster labels May 19, 2025

josecelano self-assigned this May 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Overhaul stats: refactor `Swarm::remove_inactive` method #1527

Overhaul stats: refactor `Swarm::remove_inactive` method #1527

Overhaul stats: refactor Swarm::remove_inactive method #1527

Overhaul stats: refactor Swarm::remove_inactive method #1527

Comments

Overhaul stats: refactor `Swarm::remove_inactive` method #1527

Overhaul stats: refactor `Swarm::remove_inactive` method #1527