Consider using asyncioreactor

[ShadowJonathan]

Idea

Use twisted.internet.asyncioreactor for synapse's twisted reactor.

Description

This is part of my idea that synapse should consider moving forward, beyond, or at least next to twisted, to also use asyncio's economy of libraries and functions. With the asyncioreactor, it's possible to mix twisted and asyncio functionality with eachother.

It's also possible to use uvloop in addition to this, allowing a big IO performance boost.

Some links:

asyncioreactor: https://twistedmatrix.com/documents/current/api/twisted.internet.asyncioreactor.html

asyncio/twisted intermixing: https://gist.github.com/ldjebran/4febf298232a6fd86871df25d4dc00dd

Deferred.asFuture(): https://twistedmatrix.com/documents/current/api/twisted.internet.defer.Deferred.html#asFuture

Twisted and Asyncio (blog post): https://meejah.ca/blog/python3-twisted-and-asyncio

Considerations

I have two concerns with doing this:

1. Will the existing twisted-based code be stable on a new reactor based out of asyncio?

2. Will the reactor impact performance (negatively)?

For 1, I want to run a server along my existing one, only joining some channels, and watch out for any crashes and the like, and/or perform test benches with asyncioreactor selected.

For 2, I'd want to know what the best way is for me to collect such statistics and/or how to compare them with a normal server.

[erikjohnston]

FWIW I tried using the asyncioreactor the other day and it mostly seemed to work, modulo a couple of test failures. The big difference seems to be that asyncioreactor.seconds() doesn't return a unix timestamp, which our Clock implementation assumes.

[richvdh]

That got fixed recently: twisted/twisted#1237.

@ShadowJonathan we're familiar with asyncioreactor (we use it in sygnal). Feel free to try it out on Synapse and let us know how it goes.

[ShadowJonathan]

@richvdh it looks like it's being planned for 20.11, but it's not out yet: twisted/twisted@be66f18 (look at the tags in the top row)

Said referenced releases also aren't available in pypi yet: https://pypi.org/project/Twisted/#history, so for testing, i'm going to use git+https://github.com/twisted/twisted@twisted-20.11.0.dev6#egg=twisted

[ShadowJonathan]

I've ran some experiments with 20.3, and while tests say that there's no problem with running it, @richvdh mentioned that it affects logging, so this'll be blocked till 20.11 is released, after which I'll make a PR that adds it, and makes enabling it an opt-in environment variable for now, as I have no idea how it affects performance rn, so then maybe a call could be made to test that. (e.g. "please run with SYNAPSE_USE_ASYNCIOREACTOR=yes and tell us if there is significant difference in performance")

[richvdh]

we've considered it, so I think this issue can be closed.

[ShadowJonathan]

Agreed, originally I wanted to use this to segway into more asyncio, but tbh I don't see that happening;

• a complete conversion to asyncio would require a full rewrite, which I don't see happening anytime soon
• "partial" asyncio support would be a pain in the ass due to obscure twisted-to-asyncio-and-back handover manoeuvres, and it'll result in coloured async functions/blocks, which is a no-no for maintainability

[richvdh]

given that twisted has now had a couple of releases since twisted/twisted#1237 landed, it might be good to consider this again. Even though you can't seamlessly mix twisted and asyncio stuff, it does open a route towards iteratatively moving away from twisted-specific stuff (and exploring whether uvloop actually helps performance)