Minimize extra memory allocations in Stream#read and Stream#read_partial

[janko]

These changes minimize extra memory allocations, making it possible to read from very large streams with near-constant memory usage.

For some reason the following change made the difference in this effort:

- result = @read_buffer.dup
- @read_buffer.clear
+ result = @read_buffer
+ @read_buffer = BinaryString.new

I'm not sure I understand why, I was mostly just trying different things until something worked.

When I tested Async::HTTP::Body::Fixed and Async::HTTP::Body::Chunked with these changes, they also showed very little memory allocation. BTW, I'm about to submit a PR which adds specs for both of those body classes.

For some reason this change didn't bring a positive effect on falcon itself. I will still have to investigate that.

[coveralls]

Coverage increased (+0.4%) to 84.64% when pulling 7c14963 on optimize-reading-stream into f79be6f on master.

[ioquatix]

Can you investigate that and once you are happy I will merge. It would be good to confirm that changes bring about improvements. Looks good thought!

[janko]

Oh, this change definitely reduces memory usage of Async::IO::Stream, that's for sure. Here is proof:

require "async/io/stream"
require "stringio"

io     = StringIO.new("a" * (5*1024*1024))
stream = Async::IO::Stream.new(io)

GC.disable

start_memory = `ps -p #{Process::pid} -o rss`.split("\n")[1].chomp.to_i

while (chunk = stream.read_partial)
  chunk.clear
end

end_memory = `ps -p #{Process::pid} -o rss`.split("\n")[1].chomp.to_i
puts "#{(end_memory - start_memory).to_f / 1024} MB"

Before: 5.2421875 MB
After:  0.00390625 MB

I've also seen it have a positive effect on falcon in certain variations of the Falcon::Adapters::Input implementation, just not in the current one. So, it certainly will bring benefits eventually, we just have to figure out where Ruby's "smart" string management is not working for us (I say "smart" because actual memory usage doesn't seem to be directly linked to allocations). In either case, this will certainly bring benefits for anyone/anything using Async::IO::Stream 😃

[janko]

The effect is the same on Async::HTTP::Body::Fixed and Async::HTTP::Body::Chunked that wrap Async::IO::Stream – if you String#clear the result of Body#read, the memory usage is also essentially 0 MB.

[ioquatix]

Thanks for following up.

I appreciate the memory usage information. I might also run the same check comparing the number of sys calls just to make sure we don't have a regression there.

[ioquatix]

I checked using strace and I couldn't see any major difference. So, I'm very happy with this improvement. Well done and many thanks.

[ioquatix]

Included in release v1.12.3.

[ioquatix]

I added your script into examples/allocations.rb too :)