Micro-optimizations for Decompressor and Context

[maruth-stripe]

We found current_table_size, Decompressor#read_header and Context#decode to be expensive functions while profiling code that used protocol-http2. Microbenchmarks show a ~15+% improvement in decoding performance.

Types of Changes

• Performance improvement.

Contribution

We unrolled the loop in read_header to a case-when block. Specifically using constants for the when clauses so Ruby uses a hash lookup. We also remove some unnecessary hash lookups. We also do bookkeeping for the current table size instead of recomputing.

The changes passed all tests locally. For benchmarking we used this slightly modified version of the rfc7541 test case

require 'protocol/hpack/decompressor'
require 'yaml'


def fixtures(mode)
	Dir.glob(File.expand_path("rfc7541/*.yaml", __dir__)) do |path|
		fixture = YAML::load_file(path)
		
		if only = fixture[:only]
			next unless only == mode
		end
		
		yield fixture
	end
end

def timed(tag=nil)
	t = Time.now
	ret = yield
	d = Time.now - t
	d
end

t = 0
fixtures(:decompressor) do |example|
	example[:streams].size.times do |nth|
		t += timed do
			10_000.times do
				context = Protocol::HPACK::Context.new(huffman: example[:huffman], table_size: example[:table_size])
				buffer = String.new.b
				decompressor = Protocol::HPACK::Decompressor.new(buffer, context)
				(0...nth).each do |i|
					buffer << [example[:streams][i][:wire].gsub(/\s/, '')].pack('H*')
					decompressor.decode
				end
			end
		end
	end
end

puts t

• I tested my changes locally.
• I agree to the Developer's Certificate of Origin 1.1.

cc @froydnj

[ioquatix]

Wow, amazing!!

I've rebased your PR on top of a working CI, which includes external tests from protocol-http2 and async-http so we have a fair idea that this doesn't break real usage.

I'd love for you to add benchmarks to benchmarks/decompressor/... with an included readme.md which includes details of how to run the benchmark and any relevant details/analysis/results/conclusions. It's useful for future work. I'm happy for you to do this in a separate PR.

For benchmark timing, I recommend you avoid Time.now as this can be fairly imprecise, and instead consider using the Process.clock_gettime(Process::CLOCK_MONOTONIC) or similar.

[ioquatix]

Would it make sense to use the constants defined above, here?

e.g.

Suggested change

	indexed: {prefix: 7, pattern: 0x80},
	indexed: INDEXED_TYPE,

?

[ioquatix]

Small nit, if we are going to introduce this, can we rename it to @current_table_size? I'm okay to do this in a separate PR if you want to keep this PR hygienic.

[ioquatix]

Can you add a brief comment explaining how the lookup here works, and where the values are coming from?

[ioquatix]

We are missing coverage for this line - can you add a test for the failure case?

[maruth-stripe]

Never mind, this failure case is impossible (see comment)

[maruth-stripe]

Addressed comments, I'll add a benchmarking script in a follow-up!

[ioquatix]

Great, fantastic work.