Skip to content
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

Experimenting with Trio-based Sanic server #1662

Closed
wants to merge 36 commits into from
Closed

Conversation

Tronic
Copy link
Member

@Tronic Tronic commented Aug 27, 2019

Attempting to leave the AsyncIO horrors behind, I spent the last two days porting Sanic to Trio. It turned out to be less tedious to rewrite most of server.py from scratch, so here you have a HTTP server that can serve requests but that still lacks a lot of normal Sanic functionality. Also, I wanted to keep changes to other parts of Sanic minimal, such that this might be at some point be available as a switchable mode, e.g. SANIC_FRAMEWORK=asyncio|uvloop|trio.

Interesting highlights:

  • 70 % less code than in asyncio-based server.py (although currently less functionality too)
  • Async functions call and await async functions
    • no callbacks, loop.create_task and such hacks to get things done
  • Does not use HttpTools parser
    • because only OOP-style non-async callbacks are supported, writing my own HTTP parser was much easier than wrapping it
    • based on benchmarks it doesn't really make any difference that HttpTools is a few microseconds faster per request than pure Python solution
  • Cancellation and timeouts of course work extremely nicely with Trio
  • Binding multiple addresses without create_server internal magic (listeners is a Python list of sockets)
  • Graceful exits, restarting any worker by SIGHUP without interrupting others, etc.
  • Slightly slower than uvloop but should be faster than asyncio (need to benchmark more)
  • Should be easy to add proper HTTP/2 support (I bet to have that done by Friday)
    • improves real world performance drastically thanks to reduced round-trips and improved parallelism

There is a lot more to do, many things are missing or broken, but I am posting this here for discussion. Should there be a Trio-mode in Sanic at all? Trio may never match uvloop's performance in microbenchmarks, and two modes is added maintenance burden. Anyone else willing to participate?

@Tronic
Copy link
Member Author

Tronic commented Sep 3, 2019

Status update:

  • Protocol sniffing just for fun (HTTP/1, HTTP/2 and websockets w/ and w/o SSL on the same port)
  • New streaming API (see WIP streaming response #1661 for discussion)
  • HTTP/1.0 dropped (everybody, even robots, quit using it in the last two years)
  • HTTP/1.1 almost feature-complete but significantly slower than uvloop
  • HTTP/2 needs to be updated to the new stream API, and probably requires queues and h2 flow control to avoid blocking parallel streams; seems faster than HTTP/1.1 already.

I've encountered a connection glitch causing failure to read request, followed by connection timeout 20 seconds later, on asyncio and uvloop. The glitch occurs on fresh connections (maybe one in thousand), never in keepalive requests, and it never happens in trio implementation. Haven't yet figured out how to track it down.

Next steps:

  • Refactor HTTP/1 code (probably as a self-contained stream object to avoid push_back and passing already parsed headers around).
  • Implement proper HTTP/2 stream handling and make it use app.handle_request_trio too.
  • Port the cool non-trio stuff back to Sanic master.
  • Benchmark and profile more, both on uvloop and on trio, to find where the bottle necks are.

@Tronic
Copy link
Member Author

Tronic commented Sep 8, 2019

Today I managed to clean up most of the big mess I had created while sketching various features. Still much cleanup and refactoring to be done, too. Most of my recent work on normal Sanic, such as the header optimizations, have also sprung off this branch.

@pgjones
Copy link

pgjones commented Sep 9, 2019

(Sorry this is a bit of an aside, but I think it is worth considering).

As Sanic is an ASGI framework this PR should run with a Hypercorn ASGI server utilizing the Trio worker. I think it would be worth testing this when considering this PR.

@Tronic
Copy link
Member Author

Tronic commented Sep 10, 2019

Benchmarked 5500 requests per second on Windows, single-threaded, with a
simple handler returning only sanic.response.text("Hello World!\n").

These are keep-alive requests, so connection handshake and such are negligible.

A request is processed in 180 µs, and cProfile shows us where most time is
spent. Rough call pattern, in request processing order, with per-request times:

  • receive request: 25 µs

    • trio receive_some: 18 µs
    • all other processing: 7 µs
  • parse_h1_request: 9 µs (FFFAAAST pure Python)

  • H1Stream init: 2 µs

  • request init: 4 µs

  • set_timeout: 27 µs (only used once per request but still too slow!)

    • trio deadline setter: 22 µs
      • trio _might_change_registered_deadline: 14 µs
  • handle request: 80 µs

    • router.get: 3 µs (INSANELY FAST)
    • receive_body: 6 µs (wasted time - request has no body - have to look into this)
    • request handler: 5 µs
      • response.text: 4 µs (HTTPResponse init etc)
    • isawaitable (testing if request handler is coro - it wasn't): 3 µs
    • send 52 µs:
      • data_to_send 9 µs:
        • format_http1_response: 6 µs (FAST!)
      • trio send_all: 40 µs
  • http1 function: 7 µs (some Python stuff between the calls above)

  • a few microseconds worth of misc calls

  • About 17 µs of work not reported in context by cProfile

    • some of the I/O (waiting on select)
    • trio reschedule 6 µs
    • random shuffling of tasks (trio internals) 2 µs

@Tronic
Copy link
Member Author

Tronic commented Sep 10, 2019

Fixing receive_body bumped it from 5500 req/s to 5800 req/s... But disabling timeout updates gives 7300 req/s.

@Tronic
Copy link
Member Author

Tronic commented Sep 29, 2019

Sanic has nine sites where it calls handlers or middleware functions and then result = await result if a coroutine object was received, allowing handlers to be defined as async or normal functions. The isawaitable check is slow, as seen in profiling results, and hasattr(result, "__await__") would be a lot faster and equivalent in our use.

Another alternative would be to wrap all non-async functions in async wrappers that can be awaited without this check, as they are registered to the app, avoiding the checks every time the functions are called. This would have roughly equivalent performance to the hasattr approach but allows for cleaner calls without such checks. OTOH, the wrapping itself would be somewhat complicated.

In any case, it would be good to refactor middleware and routing handling so that no code is duplicated between asgi.py, app.py and server.py, reducing the number of these call sites, and largely avoiding the inconvenience while also better preserving identical semantics for native and asgi hosting.

Comments & help regarding these issues would be appreciated.

@Tronic Tronic mentioned this pull request Sep 30, 2019
@lichray
Copy link

lichray commented Oct 29, 2019

I would love it if this can work.

@Tronic
Copy link
Member Author

Tronic commented Oct 29, 2019

I expect to resume working on this in December.

@ahopkins
Copy link
Member

@Tronic I think you are hitting on a couple of big items that need to be addressed probably outside of this PR. Namely the streamlining of the multiple calls to the router/middleware/listeners. I like the hasattr idea. I think I might borrow that for usage on sanic-jwt.

Personally, I think we need to resolve these other items before attempting a third server option.

@Tronic
Copy link
Member Author

Tronic commented Dec 21, 2019

Sure, feel free to use it. I already forked a few PRs (most already merged) off things I found while experimenting on this. As stated in the beginning, this is more of a hacking ground than something meant to be merged as is.

@Tronic
Copy link
Member Author

Tronic commented Jan 21, 2020

I'm closing this as the work was never meant to be merged and I don't have current plans to keep working on this branch. At this point it seems easier to copy&paste usable parts of code rather than try merging with the last six months of changes in master, in case one wishes to pursue with Sanic native Trio server. Also worth noting is the simplicity of implementing Trio support over ASGI, which suggests that a native server might possibly plug into the ASGI API instead.

There are also various bits in this that should still be picked up and implemented on asyncio server as well, like SSL SNI support. Expect to see separate PRs on that.

As for now, anyone wishing to do Trio webapps on Sanic is encouraged to use hypercorn -k trio your.app, which now has experimental support in master and in upcoming 20.3 release.

@Tronic Tronic closed this Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants