Performance of isawaitable

[ktosiek]

I've run a very simplistic benchmark, just returning a long list of single-field objects.
It seems graphql-core-next is 2.5x slower than graphql-core: https://gist.github.com/ktosiek/849e8c7de8852c2df1df5af8ac193287

Looking at flamegraphs, I see isawaitable is used a lot, and it's a pretty slow function. Would it be possible to pass raw results around more? It seems resolve_field_value_or_error and complete_value_catching_error are the main offenders here.

[Cito]

So far the focus has been on compatibility, correctness, and being up to date with GraphQL.js.

The next step will be to improve performance. There is probably a lot of room for improvement. I'd like to keep the code simple and very close to the GraphQL.js implementation though, in order to make it easier to stay in sync with changes over there, and not move us into a very performant, but inscrutable and unmaintainable dead end.

Maybe we can kind of inline some of the isawaitables() or split some of the code paths for sync/async to avoid such checks.

But before we start the optimization, we need good benchmarks tools. Your script would be a starting point. My idea is to port the existing benchmarks from GraphQL.js and maybe add some more Python specific ones if needed (see issue #3). I think it makes sense t to work on that first, and at least have some basic performance checks before changing the code.

It's on my list, just need some free time to work on this. Feel free to send contributions as PRs as well.

[Cito]

Update: We now have nice benchmarks and can continue working on performance.

[qeternity]

I'd like to keep the code simple and very close to the GraphQL.js implementation though, in order to make it easier to stay in sync with changes over there, and not move us into a very performant, but inscrutable and unmaintainable dead end.

Nice job so far on this project, thanks for all your efforts. I worry that keeping the implementation similar between two very different languages and interpreters is going to keep this project from being scalable. We're using this for a new project, and are considering returning to DRF because for relatively small calls we're struggling to get latencies below 300ms (profiling shows nearly all of the time is spent inside graphql-core). PyPy reduces this by about half, but that means backporting a few repos to 3.6 which I did to test out, but am not keen to maintain especially with 3.8 just around the corner.

[Cito]

If it really makes and big difference necessary we certainly can implement some parts that are crucial regarding performance differently. But actually JS and Python are not that different, and as I wrote above, I believe there is still a lot of room for improvement while keeping the implementation similar. Anybody who wants to work on this: Feel free to send PRs and suggestions.

[Cito]

While I said JS and Python are not that different, there is one difference that plays a role here, namely that in JS you can await any expression, but in Python you can await only awaitables. That's why we need so many isawaitable calls and have nested async functions which is not very efficient. Maybe we can find a better solution.

[qeternity]

Thanks @Cito indeed my point about Python vs. JS was that Python has been sync from day one with async bolted on whereas JS has been the other way around. As @ktosiek has done a very good job demonstrating, the isawaitable calls are very expensive and are only necessary due to the differences you mention.

[Hellzed]

@ktosiek Which versions of Python have you tried this on? I found huge improvements from Python 3.6 to Python 3.8 (it seems like 3.8 isawaitable() has almost 300% the performance of 3.6).

[ktosiek]

Thanks for the heads up, I'll try on 3.8 too. I've run my experiments on 3.6 and 3.7. Are you sure your 3.6 was compiled with PGO? It made a huge difference for me when I've installed 3.7 from pyenv (which disabled PGO to get better compilation times). śr., 20 lis 2019, 23:03 użytkownik Loris Zinsou <notifications@github.com> napisał:

…

@ktosiek <https://github.com/ktosiek> Which versions of Python have you tried this on? I found huge improvements from Python 3.6 to Python 3.8 (it seems like 3.8 isawaitable() has almost 300% the performance of 3.6). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#54>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACD37MRKKD3SBJ2XREOPSDQUWXZVANCNFSM4ITOIKPQ> .

[ktosiek]

@Hellzed wow, I've confirmed your tests. There's a big difference between 3.6 and 3.7, but 3.8 is even faster!

I've looked at the test_execute_introspection_query benchmark, and it went from ~756ms on 3.6 to ~265ms on 3.8. This is so huge isawaitable() might not be the bottleneck anymore, I'll need new flamegraphs now :-)

Benchmark results:

version	time
Python 3.8.0 (default, Nov 21 2019, 20:31:38)	370 nsec per loop
Python 3.7.5rc1 (default, Oct 8 2019, 16:47:45)	444 nsec per loop
Python 3.6.9 (default, Nov 21 2019, 21:00:04)	1.07 usec per loop

The benchmark:

$ python3 -VV
Python 3.6.9 (default, Nov 21 2019, 21:00:04) 
[GCC 9.2.1 20191008]
$ python3 -mtimeit -s $'from inspect import isawaitable \nclass A: pass \na = A()' 'isawaitable(a)'
1000000 loops, best of 3: 1.07 usec per loop