Skip to content

Implement HPack dynamic compression #4715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Tratcher opened this issue Sep 23, 2018 · 7 comments · Fixed by #20058
Closed

Implement HPack dynamic compression #4715

Tratcher opened this issue Sep 23, 2018 · 7 comments · Fixed by #20058
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-kestrel HTTP2 Perf

Comments

@Tratcher
Copy link
Member

Tratcher commented Sep 23, 2018

Today Kestrel does not use any of the HPack compression features when serializing response headers. Dynamic compression is an advanced option we could look into.

Likely candidates:

  • Server: Kestrel
  • Content-Encoding: gzip
  • Content-Encoding: brotli
  • Content-Type (and a dozen common values)
  • strict-transport-security

Note the client gets more benefits from dynamic compression, especially browsers, they send a lot of repetitive headers. HttpClient would get some benefit for things like :authority, :method, :scheme, accept-encoding,

Related work:

@aspnet-hello aspnet-hello transferred this issue from aspnet/KestrelHttpServer Dec 13, 2018
@aspnet-hello aspnet-hello added this to the Backlog milestone Dec 13, 2018
@aspnet-hello aspnet-hello added area-servers enhancement This issue represents an ask for new feature or an enhancement to an existing one HTTP2 Perf feature-kestrel labels Dec 13, 2018
@JamesNK
Copy link
Member

JamesNK commented Jan 28, 2019

Impact of header compression on gRPC responses:

gRPC result from Kestrel, length 466

0000   02 00 00 00 45 00 00 e7 4f 76 40 00 80 06 00 00   ....E..çOv@.....
0010   7f 00 00 01 7f 00 00 01 c3 83 ca 66 b7 b5 90 cd   ........Ã.Êf·µ.Í
0020   0b e7 13 34 50 18 08 04 bc b7 00 00 00 00 6c 01   .ç.4P...¼·....l.
0030   04 00 01 e4 7d 88 00 04 64 61 74 65 1d 4d 6f 6e   ...ä}...date.Mon
0040   2c 20 32 38 20 4a 61 6e 20 32 30 31 39 20 30 35   , 28 Jan 2019 05
0050   3a 31 33 3a 32 37 20 47 4d 54 00 0c 63 6f 6e 74   :13:27 GMT..cont
0060   65 6e 74 2d 74 79 70 65 10 61 70 70 6c 69 63 61   ent-type.applica
0070   74 69 6f 6e 2f 67 72 70 63 00 06 73 65 72 76 65   tion/grpc..serve
0080   72 07 4b 65 73 74 72 65 6c 00 0d 67 72 70 63 2d   r.Kestrel..grpc-
0090   65 6e 63 6f 64 69 6e 67 08 69 64 65 6e 74 69 74   encoding.identit
00a0   79 00 00 05 00 00 00 01 e4 7d 00 00 00 00 1b 00   y.......ä}......
00b0   00 1b 00 00 00 01 e4 7d 0a 0b 48 65 6c 6c 6f 20   ......ä}..Hello 
00c0   57 6f 72 6c 64 12 0c 08 f7 9e ba e2 05 10 f0 b6   World...÷.ºâ..ð¶
00d0   95 fd 02 00 00 0f 01 05 00 01 e4 7d 00 0b 67 72   .ý........ä}..gr
00e0   70 63 2d 73 74 61 74 75 73 01 30                  pc-status.0

gRPC result from C implementation, length 244

0000   02 00 00 00 45 00 00 78 44 db 40 00 80 06 00 00   ....E..xDÛ@.....
0010   7f 00 00 01 7f 00 00 01 c3 83 cb 22 8d 70 12 a0   ........Ã.Ë".p. 
0020   49 66 66 5e 50 18 08 03 6c 87 00 00 00 00 04 01   Iff^P...l.......
0030   04 00 00 f9 bf 88 c2 c1 c0 00 00 20 00 00 00 00   ...ù¿.ÂÁÀ.. ....
0040   f9 bf 00 00 00 00 1b 0a 0b 48 65 6c 6c 6f 20 57   ù¿.......Hello W
0050   6f 72 6c 64 12 0c 08 e1 9f ba e2 05 10 88 ad c1   orld...á.ºâ....Á
0060   e9 02 00 00 04 01 05 00 00 f9 bf bf 0f 2f 00 00   é........ù¿¿./..
0070   00 04 08 00 00 00 00 00 00 00 00 0c               ............

Headers are 8 bytes for C implementation and 123 bytes from Kestrel.

// @davidfowl @Tratcher @JunTaoLuo @shirhatti

@Tratcher
Copy link
Member Author

Also static compression #4716

I expect we could get rid of these strings statically:

  • date
  • content-type
  • server

I'm a bit surprised they dynamically intern the date value.

@JamesNK
Copy link
Member

JamesNK commented Jan 28, 2019

C implementation doesn't include the date header. I'm guessing it is a micro-optimization.

@kdelorey
Copy link

I'm just going to leave a note here for the future implementation. I ran into this issue with nginx reverse proxing gRPC (HTTP/2) to a dotnet service through a Kubernetes Ingress. I originally thought it was a dotnet issue, but figured out that it was actually the linkerd service mesh.

nginx explicitly announces that it does not support dynamic header compression by sending the SETTINGS_HEADER_TABLE_SIZE value set to 0, see ​here. Any attempt of an upstream server to use indexes from the dynamic range is a bug in the upstream server (note that at least grpc-go implementation is known to be buggy, see commit log in 2713b2dbf5bb).

This seems to have been an issue with other HTTP/2 implementations, notably golang's gRPC library and I just wanted to also note it here so it doesn't get overlooked.

@Tratcher
Copy link
Member Author

Tratcher commented Aug 1, 2019

Looking at another GRPC trace, interning the date header starts to make sense. You can get a lot of requests through in 1 second.

.Hello world...........grpc-status.0..l.........date.Mon, 15 Jul 2019 13:56:15 GMT..content-type.application/grpc..server.Kestrel.
grpc-encoding.identity.............
.Hello world...........grpc-status.0..l.........date.Mon, 15 Jul 2019 13:56:15 GMT..content-type.application/grpc..server.Kestrel.
grpc-encoding.identity.............
.Hello world...........grpc-status.0..l.........date.Mon, 15 Jul 2019 13:56:15 GMT..content-type.application/grpc..server.Kestrel.
grpc-encoding.identity.............
.Hello world...........grpc-status.0..l.........date.Mon, 15 Jul 2019 13:56:15 GMT..content-type.application/grpc..server.Kestrel.
grpc-encoding.identity.............
.Hello world...........grpc-status.0..l.........date.Mon, 15 Jul 2019 13:56:15 GMT..content-type.application/grpc..server.Kestrel.
grpc-encoding.identity.............

Also the content-type value application/grpc and grpc-encoding identity

@analogrelay
Copy link
Contributor

This feels bigger than the largest allowed cost ("Medium"). Does that sound accurate? If so, we should look at breaking this up, or defer it to a future release.

@Tratcher
Copy link
Member Author

There are several degrees to this one. James is going to start with a few simple approaches and see what happens. Those are still within the Medium level.

@ghost ghost locked as resolved and limited conversation to collaborators May 3, 2020
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-kestrel HTTP2 Perf
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants