Skip to content
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

Improve performance with many connections #223

Merged
merged 2 commits into from
Apr 4, 2022

Conversation

CyberShadow
Copy link
Contributor

@CyberShadow CyberShadow commented Mar 23, 2022

Replace ConnList linked list with std::multiset.

Fixes (my instance of) #140.

Keeping a sorted multi-set allows faster search by source /
source+destination address.

Fixes O(n^2) complexity when handling a lot of connections (now it's
O(n log n)).
Comment on lines +107 to +113
auto r = connections.equal_range(this);
for (auto it = r.first; it != r.second; ++it) {
if (*it == this) {
connections.erase(it);
break;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must remove ourselves from connections before we delete refpacket, because the comparison uses refpacket.

break;
}

case IPPROTO_UDP: {
current = unknownudp->connections;
connList = &unknownudp->connections;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why this change also changes the type of Process::connections and not just the top-level connections variable.

next = m_next;
/* compares Connection pointers by their refpacket */
struct ConnectionComparator {
using is_transparent = void;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This enables comparing Connection with Packet, so we don't have to construct a fake Connection when we only have a Packet, or use multimap instead of multiset.
https://www.fluentcpp.com/2017/06/09/search-set-another-type-key/

};

/* ordered set of Connection pointers */
typedef std::multiset<Connection*, ConnectionComparator> ConnList;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the end I opted to use multiset instead of unordered_map/set; the requirement for two levels of comparison (source-only and source+destination) would require nesting two levels of hash tables, and though may be that was OK for the connections global it seemed gratuitous for Process::connections.

@CyberShadow
Copy link
Contributor Author

FWIW I've been running nethogs for several days now with this change with no observed ill effects.

Copy link
Owner

@raboof raboof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants