Clean up network interface and avoid a hashmap linear search#3239
Merged
stevenengler merged 5 commits intoshadow:mainfrom Nov 28, 2023
Merged
Clean up network interface and avoid a hashmap linear search#3239stevenengler merged 5 commits intoshadow:mainfrom
stevenengler merged 5 commits intoshadow:mainfrom
Conversation
8aae9af to
b3e2f13
Compare
We no longer add C sockets to the network interface, so there's no longer a need to store `CompatSockets` as tagged pointers. We can just store rust `InetSockets` instead.
b3e2f13 to
ea1e9e8
Compare
Two `InetSocket`s are equal only if they represent the same socket object.
This allows us to avoid a linear search through the priority queue's hash map.
ea1e9e8 to
f1897b8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This cleans up the network interface and removes a linear hashmap search in the network interface's priority queue.
The network interface used to need to deal with both rust and C sockets, so it stored tagged pointers for
CompatSocketobjects. Now that the TCP socket is the only remaining C socket, and it has a rust wrapperLegacyTcpSocket, C sockets aren't used in the network interface anymore. The network interface now only stores boxedInetSocketobjects. This also allows us to remove tagged pointers from our C code.The hashmap in the priority queue used by the fifo qdisc now uses custom hash and eq implementations, which allows us to avoid a linear search through the hashmap which used the canonical handles to identify the same socket.
The new hash function is more expensive since it uses a real hash function (
DefaultHasheras used by the stdlibHashMap), whereas the original glib code doesn't seem to actually use a hash function:https://docs.gtk.org/glib/type_func.HashTable.new.html
https://github.com/GNOME/glib/blob/40081f9b627904c2506b6d3837334e4ce05052da/glib/ghash.c#L2483-L2487
This PR contains a lot of changes along the rust/C boundary, but everything seems to still work so hopefully it's correct.
This may fix #3238.
Edit:
There doesn't seem to be much (if any) of a performance regression. The simulation results have remained unchanged. The following are from a slightly older version of this PR, but I don't expect it would make any difference.
Edit 2: One commit was split off into #3240.