http icon indicating copy to clipboard operation
http copied to clipboard

`HTTP::Response::Body#each` and `BUFFER_SIZE`

Open andyundso opened this issue 1 year ago • 3 comments

Hello!

The readpartial method, which each uses allows to overwrite the size of the chunks (defaults to BUFFER_SIZE). Would you accept a PR where the size can also be passed to each?

On a similar note: Right now, the BUFFER_SIZE is configured to 16KB. I tried to figure out how this value came to be, since 16KB is not a lot, but was not really able to find an answer in the code. Is there any downside to increasing this value to e.g. 4 MB?

Background: We provide an API in one of our applications where we stream large files from a 3rd party API using your gem and ActionController::Live. These files tend to be couple of gigabytes in size, so only sending 16 KB at a time results in many requests.

andyundso avatar Apr 09 '24 13:04 andyundso

All PRs are welcomed. :D

ixti avatar Jun 10 '25 18:06 ixti

4MB seems pretty large for a default buffer size.

8-16kB are exceedingly common as HTTP buffer sizes in other libraries, chosen to be larger than the underlying MTU.

Perhaps the default could be raised if benchmarking shows performance is generally improved.

tarcieri avatar Jun 10 '25 18:06 tarcieri

FWIW, we can change Body#each to:

module HTTP
  class Response
    class Body
      # ...

      # Iterate over the body, allowing it to be enumerable
      def each(buffer_size: Connection::BUFFER_SIZE
        while (chunk = readpartial(buffer_size))
          yield chunk
        end
      end

      # ...
    end
  end
end

Meanwhile, @andyundso you can do this in your code yourself:

# instead of body.each { |chunk| ... }
while chunk = response.body.readpartial(1.megabyte)
  ...
end

ixti avatar Jun 30 '25 15:06 ixti