bdarnell / tornado_http2

Apache License 2.0
78 stars 17 forks source link

Swap to using hyper-h2 to handle http2 logic #6

Open kahuang opened 5 years ago

kahuang commented 5 years ago

hyper-h2 is a great library for handling the state machine logic for the http2 protocol. It might be worth looking into swapping to using h2 instead of updating and maintaining this library's own version of all that logic.

bdarnell commented 5 years ago

The reason this package exists is for "its own version of all that logic" - I wanted to implement http/2 to learn about it and because it's fun. :) I think an adapter for hyper-h2 could be a separate project (indeed, there is one at https://github.com/yeraydiazdiaz/tornado_h2).

Which http/2 project, if any, ever gets integrated into Tornado proper remains to be seen.

kahuang commented 5 years ago

Ah :) I assumed that this was on track to become the default implementation in tornado. Are there any ongoing initiatives that have the support of the tornado maintainers? Or if not, has there been a discussion on the requirements etc. for getting http2 servers in the tornado repo?

bdarnell commented 5 years ago

Well, this one is written by a tornado maintainer, so in that sense it has support :)

My feeling is that it doesn't make sense to merge an HTTP/2 implementation into the main Tornado repo until we're ready to enable it by default. As long as it's something that you have to opt-in to, it makes more sense as a separate package so it can evolve on its own schedule. That means it needs to be well-tested, and most importantly we need to make sure that performance is good. I've long suspected (but haven't measured) that pure-python huffman coding will never be acceptable and we'll need to move that into a C extension. OTOH, maybe the network efficiencies of HTTP/2 will make up for a slower server-side implementation. It's a complicated question.

Basically, what needs to happen for any HTTP/2 implementation is for people to try it out and say "I've used this, it's good, and it should be merged into Tornado". What we have now (in tornadoweb/tornado#1438) is a lot of people saying "please merge this into Tornado so I can try it out", which is exactly backwards.

kahuang commented 5 years ago

Definitely agree with that approach, just curious what the status of http2 in tornado land is, and if it's worth it at this point to begin contributing. This one seems like the most mature out there AFAIK (though still early), so just wanted to get some clarification on that :).