Feature request: reduce the Max RSS in streaming inflate

[harryqingyuzhao]

While it is great to have streaming support in inflating processing, the max RSS could be a little high, depends on GC frequency.

1. if we add manual GC cycle during streaming inflate, it reduces the max RSS from 150M to 26M.
2. if use gunzip, the max RSS is only 872K.

Question: can we further reduce the max RSS of streaming inflate? It seems we are using RB_ALLOC. While GC may not timely free the memory. Can we RB_ALLOC a small buffer, and reuse the same buffer without RB_ALLOC new buffer again?

Test steps:

$ dd if=/dev/zero of=/dev/stdout bs=1m count=1024 | gzip -c > 1G.gz

$ cat test.rb

require 'zlib'

f = File.open '1G.gz'

z = Zlib::Inflate.new Zlib::MAX_WBITS + 32

step = 0
total = 0

until f.eof? do
  gzipped = f.read(4096)
  z.inflate gzipped do |chunk|
    total += chunk.size

    step += 1
    # GC.start if step % 100 == 0 # commont out this line to see larger `max rss`
  end
end

puts "total: #{total}"

z.finish
# end of test.rb

The max RSS is around 150M

$ /usr/bin/time -l ruby test.rb
total: 1073741824
2.26 real 2.12 user 0.13 sys
156069888 maximum resident set size
0 average shared memory size
0 average unshared data size
0 average unshared stack size
81828 page reclaims
0 page faults
0 swaps
0 block input operations
0 block output operations
0 messages sent
0 messages received
0 signals received
0 voluntary context switches
930 involuntary context switches

Uncomment the line GC.start if step % 100 == 0, with more GC invoked, the max RSS is reduced to around 26M.

$ /usr/bin/time -l ruby test.rb
total: 1073741824
2.74 real 2.70 user 0.03 sys
26480640 maximum resident set size
0 average shared memory size
0 average unshared data size
0 average unshared stack size
6517 page reclaims
0 page faults
0 swaps
0 block input operations
0 block output operations
0 messages sent
0 messages received
0 signals received
1 voluntary context switches
862 involuntary context switches

Compare to gunzip. The max RSS is only 870K when using gunzip.

$ /usr/bin/time -l gunzip 1G.gz
1.58 real 1.23 user 0.28 sys
872448 maximum resident set size
0 average shared memory size
0 average unshared data size
0 average unshared stack size
236 page reclaims
0 page faults
0 swaps
0 block input operations
0 block output operations
0 messages sent
0 messages received
0 signals received
138 voluntary context switches
176 involuntary context switches

[jeremyevans]

In your example, we can't control what the user will do with chunk yielded to the block, so it isn't safe to reuse it. If we want to support buffer reuse, we should have Zlib::Inflate#inflate take a :buffer keyword argument, and use that as the reusable buffer, also yielded it to the block.

[harryqingyuzhao]

Thank you Jeremy!

I am not sure how to create a buffer in Ruby(Google tells something like buffer = ' ' * 81920). Do you mean we could do this?

buffer = ' ' * 81920   # create a 81920 bytes buffer for inflate output.

until f.eof? do
  gzipped = f.read(4096)

  z.inflate gzipped, buffer: buffer do |buffer_overflow_chunk|  # in case `buffer` is not big enough to hold decompressed data, the remaining part is returned by `buffer_overflow_chunk`
     total_decompressed_size += buffer.size
     total_decompressed_size += buffer_overflow_chunk.size if buffer_overflow_chunk
  end
end

[jeremyevans]

My thought was something like this:

buffer = ' ' * 81920   # create a 81920 bytes buffer for inflate output.

while gzipped = f.read(4096)
  z.inflate gzipped, buffer: buffer do |buf|
     # buf.equal?(buffer) is true
     # this yields potentially multiple times
     # buf could be larger than 81920 depending on decompression
     total_decompressed_size += buf.size
  end
end

[harryqingyuzhao]

Hi Jeremy, this looks cleaner than my version. 👍 It only need to use buf without worrying about buffer in the yield block. I have some questions if you do not mind:

Question 1:

buf.equal?(buffer) is true

To confirm:

• If decompressed size <= 81920, we will just put inflate value into buffer. In this case, we just yield with buf = buffer.
• If decompressed size is larger than 81920, will we realloc the buffer to a bigger size? In this case, for the later loop iterations, we actually passed in the buffer pointing to the reallocated memory which is larger than 81920?

Question 2:

this yields potentially multiple times

I do not understand this. Do you mean, this yields multiple times because of the the file batch read 4096 each time? Or there is something else I am not aware?

Question 3:
To confirm, when we decompress a given gzipped:

• we could call something like output_buffer = decompress(gzipped, output_buffer) where output_buffer will be reallocated if necessary?
• I hope the output_buffer is not dynamically allocated by decompress function call(not something like output_buffer = decompress(gzipped)). Otherwise we cannot pass in buffer to reuse it.

[jeremyevans]

Hi Jeremy, this looks cleaner than my version. 👍 It only need to use buf without worrying about buffer in the yield block. I have some questions if you do not mind:

Question 1:

buf.equal?(buffer) is true

To confirm:

• If decompressed size <= 81920, we will just put inflate value into buffer. In this case, we just yield with buf = buffer.
• If decompressed size is larger than 81920, will we realloc the buffer to a bigger size? In this case, for the later loop iterations, we actually passed in the buffer pointing to the reallocated memory which is larger than 81920?

The buffer object remains the same. If the capacity of the string was not large enough to store the decompressed data, then the string would allocate enough memory to store it. That could potentially happen on every call to inflate or every loop iteration, if the amount iterated was always too large.

Question 2:

this yields potentially multiple times

I do not understand this. Do you mean, this yields multiple times because of the the file batch read 4096 each time? Or there is something else I am not aware?

If you pass inflate a block, it will continually yield decompressed data, as described by the documentation:

 * If a block is given consecutive inflated chunks from the +deflate_string+
 * are yielded to the block and +nil+ is returned.

Question 3:
To confirm, when we decompress a given gzipped:

• we could call something like output_buffer = decompress(gzipped, output_buffer) where output_buffer will be reallocated if necessary?
• I hope the output_buffer is not dynamically allocated by decompress function call(not something like output_buffer = decompress(gzipped)). Otherwise we cannot pass in buffer to reuse it.

If a :buffer option is provided, we can probably pre-set z->buf and don't call rb_str_modify_expand in zstream_expand_buffer_into unless we need to expand the buffer. There are various places where z->buf is set to nil that we would also need to modify if a :buffer option is provided.

[harryqingyuzhao]

Thanks Jeremy for the knowledge sharing. 🙏 Your solution sounds great!

Are you ok with addressing this feature enhancement? Please tell me if you think I could do anything to accelerate the process.(I looked at the zlib.c code, I do not feel I can figure out the change myself quickly.)

This will be super helpful for our usage scenario. We had some Sidekiq workers which use Zlib::Inflate.inflate. For big gzip files, the inflating will bloat RSS in a period of time. With this enhancement, probably we can expect the max RSS to be reduced by 90% or more. For the 1G.gz test example, I would expect max RSS reduced from 150M to less than 15M(I believe this because if we do very aggressive GC in the test code this is achievable - though it becomes very slow) -- probably even better.

[jeremyevans]

@nurse Are you OK with this feature being added?

[nurse]

@jeremyevans It sounds nice!

buffer = ' ' * 81920 # create a 81920 bytes buffer for inflate output.

FYI it should be buffer = "\0" * 81920 to use String#*'s optimization to use calloc
https://github.com/ruby/ruby/blob/8e06075442ade74a0c23afad235742c41b3ceac8/string.c#L2050-L2060

[harryqingyuzhao]

Thank you @jeremyevans and @nurse ! 🙇

[harryqingyuzhao]

Hi @jeremyevans, it is great the change is merged to ruby:master! 🙏

Should I expect Ruby will include this change in all major releases(2.6.x, 2.7.x, 3.x).

We are using ruby-2.7.2. Do we have to wait for 2.7.3 and upgrade to have this enhancement?

BTW: get the ruby-head today, which is ruby 3.0.0dev (2020-11-04T04:30:27Z master f211f9cda4) [x86_64-darwin19]. This does not have this enhancement yet:

$ cat new_test.rb
require 'zlib'

f = File.open '1G.gz'
z = Zlib::Inflate.new Zlib::MAX_WBITS + 32

total = 0

buffer = "\0" * 81920   # create a 81920 bytes buffer for inflate output.

while gzipped = f.read(4096)
  z.inflate gzipped, buffer: buffer do |chunk|
    total += chunk.size
  end
end

puts "total: #{total}"

z.finish


$ ruby -v
ruby 3.0.0dev (2020-11-04T04:30:27Z master f211f9cda4) [x86_64-darwin19]

$ ruby new_test.rb
new_test.rb:12:in `inflate': wrong number of arguments (given 2, expected 1) (ArgumentError)
	from new_test.rb:12:in `<main>'

[harryqingyuzhao]

Another question: is there a way I can test this change on my local, without waiting for a new ruby release? Thanks!

[jeremyevans]

The code has only been updated in the zlib repository, it has not been merged to ruby itself. It will probably be merged into ruby itself before the ruby 3.0.0 release. You should not expect it in earlier releases, though you should be able to gem install zlib in those versions once this makes it into a new release (which should also be before the ruby 3.0.0 release).

To test these changes, clone the repository, run rake compile in repository, run your app with RUBYLIB=/path/to/zlib/lib.