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

don't stream certain in-memory io.Readers #44

Merged
merged 3 commits into from
Jan 3, 2024

Conversation

joeshaw
Copy link
Member

@joeshaw joeshaw commented May 24, 2023

bytes.Buffer, bytes.Reader, and strings.Reader are all io.Reader
implementations on an in-memory buffer. Their sizes are known ahead of
time, and these types tend to be used for small payloads. We can send
these payloads with a Content-Length rather than using chunked
encoding by sending them with SendAsync rather than
SendAsyncStreaming.

@joeshaw
Copy link
Member Author

joeshaw commented May 24, 2023

Marked this as a draft because I'm not 100% convinced we definitely want to do this, but it's the implementation following my comment in #38 (comment).

@joeshaw
Copy link
Member Author

joeshaw commented May 24, 2023

One possible other adjustment would be to actually examine the size of the buffers (all three types implement Len() int) and still stream if over some size threshold. I'm not sure how much that matters though.

@joeshaw
Copy link
Member Author

joeshaw commented Dec 8, 2023

I decided not to limit it based on length because the Rust SDK doesn't do anything like that currently. (It implements the From trait for several string/byte slice/byte vector types and they also just copy.) However, if we wanted to add it later, we could add a limit for it, set it to a sensible default (64k?) and only copy if it's within the limit.

@joeshaw joeshaw marked this pull request as ready for review December 8, 2023 19:39
@joeshaw
Copy link
Member Author

joeshaw commented Dec 12, 2023

Moving back to draft, as this needs some work.

@joeshaw joeshaw marked this pull request as draft December 12, 2023 20:26
`bytes.Buffer`, `bytes.Reader`, and `strings.Reader` are all `io.Reader`
implementions on an in-memory buffer.  Their sizes are known ahead of
time, and these types tend to be used for small payloads.  We can copy
these payloads into an `fsthttp.Body` and send them with a
`Content-Length` header rather than using chunked encoding by sending
them with `SendAsync` rather than `SendAsyncStreaming`.
@joeshaw joeshaw force-pushed the joeshaw/dont-stream-known-sizes branch from 2e3451c to f5340a7 Compare December 19, 2023 22:40
@joeshaw joeshaw marked this pull request as ready for review December 19, 2023 22:47
@joeshaw joeshaw merged commit a8ec914 into main Jan 3, 2024
10 checks passed
@joeshaw joeshaw deleted the joeshaw/dont-stream-known-sizes branch January 3, 2024 18:31
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.

3 participants