okhttp: add full implementation of HPACK header compression

[voidzcy]

Copied implementation from OkHttp3 Hpack.kt. Most changes are on Writer.

Tests are copied from OkHttp3 HpackTest.java, but I was trying to avoid introducing dependency of google/Truth by preserving the usage of org.junit.Assert.assertEquals.

Upstream okhttp3 has pretty good coverage for unit tests. I also inspected packets after the change:

• Before:

:scheme: https, :method: POST and :authority: localhost:8080 all represented in the format of Literal Header Field without indexing - Indexed Name.

• After:

:scheme: https and :method: POST are represented in the format of Indexed Header Field and :authority: localhost:8080 in the format of Literal Header Field with Incremental Indexing - Indexed Name.

• As a reference, packets from Netty's:

Resolves #5985.

[vkryachko]

Hi, when is this expected to be merged? And subsequently released?

[voidzcy]

Hi, when is this expected to be merged? And subsequently released?

Currently we don't have enough priority reviewing this. Does this block anything for you?

[vkryachko]

We are considering to add a "user-agent"-like header to the Firestore android sdk(uses grpc-java and grpc-okhttp) whose rpc usage is pretty chatty. So the idea is to send it with every rpc, but the size overhead is a concern as the header could be quite long(hundreds of bytes).

So the underlying assumption was that, since grpc uses http2, it should not be a problem thanks to hpack. But it does not look to be the case yet, and this PR would address it.

Does this block anything for you?

Merging+releasing this would definitely help us as we would not have to come up with ways to work around this limitation of okhttp

[voidzcy]

/cc @ejona86

[ejona86]

This looks fine to me, although I would like to see what the second RPC looks like with huffman disabled. In general it may be reasonable to disable huffman.

[voidzcy]

This looks fine to me, although I would like to see what the second RPC looks like with huffman disabled. In general it may be reasonable to disable huffman.

Here is the result (the main gRPC frame) of running the interop-testing client with two successive empty call. Huffman encoding was disable (but the default is enabled in implementation).

• First:
  

• Second:
  

[ejona86]

I still think it would be good to disable huffman. But this looks fine.

[voidzcy]

I still think it would be good to disable huffman. But this looks fine.

Alright, I will disable huffman. Btw, may I ask why?

[ejona86]

It's a CPU vs bandwidth argument. The most compact huffman code is 5 bits long. So the theoretical maximum bandwidth reduction it could make is 37%, but it will generally be less than that. Also, for short strings (say 10 bytes) who cares if you save a few bytes? Your overhead is much higher than that. So we could enable it on just larger strings, but many of the long strings we tend to send are also those we only send once, like user-agent. Although grpc-trace could benefit a bit. I'd be fine enabling it for strings 32+ bytes or some such.