-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
x/sys/unix: add ParseOneSocketControlMessage #54714
Comments
Change https://go.dev/cl/425916 mentions this issue: |
Another approach would be // ParseOneSocketControlMessage parses a single a single socket control message from b, returning the message header,
// message data (a slice of b), and the remainder of b after that single message. When there are no remaining messages,
// len(remainder) == 0.
func ParseOneSocketControlMessage(b []byte) (hdr Cmsghdr, data []byte, remainder []byte, err error) |
Change https://go.dev/cl/425917 mentions this issue: |
@ianlancetaylor I like your proposal, it's even quite a bit faster in my benchmark:
I submitted another CL with an implementation: https://go-review.googlesource.com/c/sys/+/425917. |
Is there anything else needed to get https://go-review.googlesource.com/c/sys/+/425917 merged, @ianlancetaylor? |
The proposal needs to be reviewed and approved. |
@ianlancetaylor Is there any timeline for review and approval of this proposal? https://go-review.googlesource.com/c/sys/+/425917 should be ready to go. |
There are many proposals, with no particular timeline. You can follow the proposals being reviewed at https://go.dev/issue/33502. |
This proposal has been added to the active column of the proposals project |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
Note, Oct 12 2022 Current proposal is #54714 (comment)
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
In quic-go, we read control messages to read the ECN bits from the IP header, and to read the network interface index.
We parse the OOB bytes using
unix.ParseSocketControlMessage
. See https://github.com/lucas-clemente/quic-go/blob/07412be8a02ef0e55580ebf8db9c38a759c0a0e5/sys_conn_oob.go#L166-L206 for the respective control message processing logic.What did you expect to see?
Ideally,
unix.ParseSocketControlMessage
would not allocate.What did you see instead?
Receiving 1 GB of data using QUIC creates a huge amount of allocations (as determined using the
allocs
function of pprof). Roughly 100 MB of those allocations come fromunix.ParseSocketControlMessage
.A simple back-of-the-envelope shows that this is roughly what we'd expect:
A data transfer of 1 GB requires receiving roughly 860,000 received packets, assuming a payload size of 1250 bytes per QUIC packet.
The
[]SocketControlMessage
slice allocates 24 bytes, and the size of each control message is 40 bytes. There are two control messages per packet (unix.IP_TOS
andunix.IP_PKTINFO
). Parsing the control messages therefore allocates of 104 bytes per packet, or 89 MB in total for the 1 GB transfer.Proposal
It would be nice to have an API that allows parsing socket control message that doesn't allocate at all.
The following API would fulfill that property:
UPDATE: Submitted a fix https://go-review.googlesource.com/c/sys/+/425916.
The text was updated successfully, but these errors were encountered: