-
-
Notifications
You must be signed in to change notification settings - Fork 299
Improve performance with many connections #223
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
Conversation
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)).
| auto r = connections.equal_range(this); | ||
| for (auto it = r.first; it != r.second; ++it) { | ||
| if (*it == this) { | ||
| connections.erase(it); | ||
| break; | ||
| } | ||
| } |
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.
We must remove ourselves from connections before we delete refpacket, because the comparison uses refpacket.
|
|
||
| case IPPROTO_UDP: { | ||
| current = unknownudp->connections; | ||
| connList = &unknownudp->connections; |
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.
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; |
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.
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; |
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.
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.
|
FWIW I've been running nethogs for several days now with this change with no observed ill effects. |
raboof
left a comment
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.
Thanks!
Replace
ConnListlinked list withstd::multiset.Fixes (my instance of) #140.