Skip to content

perf: ensure full write buffer for *File.ReadFrom #623

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

Merged
merged 1 commit into from
Mar 25, 2025

Conversation

justfalter
Copy link
Contributor

In *File.ReadFrom, ensure that the buffer is filled to capacity (maxPacket length) before performing the write.

Prior to this change, the amount of data read into the buffer was dictated by the io.Reader's Read implementation, and write performance would suffer when the Read would return less than maxPacket bytes of data. An example source would be a net/http response Body from a TLS server, which seems to cap each read to 16384 bytes -- half the default max packet size of 32768 bytes.

In `*File.ReadFrom`, ensure that the buffer is filled to capacity (`maxPacket`
length) before performing the write.

Prior to this change, the amount of data read into the buffer was dictated by
the `io.Reader`'s `Read` implementation, and write performance would suffer
when the Read would return less than maxPacket bytes of data. An example
source would be a `net/http` response `Body` from a TLS server, which
seems to cap each read to 16384 bytes -- half the default max packet size of
32768 bytes.
Copy link
Collaborator

@puellanivis puellanivis left a comment

Choose a reason for hiding this comment

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

This does look like a solid improvement.

@puellanivis puellanivis merged commit 320d62f into pkg:master Mar 25, 2025
4 checks passed
puellanivis added a commit that referenced this pull request Mar 25, 2025
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.

2 participants