nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.27k stars 322 forks source link

Python: Feature request: uvloop support for Python ASGI apps #1105

Open bunny-therapist opened 5 months ago

bunny-therapist commented 5 months ago

When running a python ASGI app with uvicorn, you have the option to install uvicorn with uvloop, which is then used to speed up the async event loop.

It appears that this code here is all they need for this: https://github.com/encode/uvicorn/blob/f39933c8501dd6bc2e8e0f5be45819d3ba2caad0/uvicorn/loops/uvloop.py If uvloop is installed, uvloop_setup is called and then the event loop is just faster.

When I realized that nginx unit uses the actual asyncio python library (although from C code), I tried to merely copy-paste uvloop_setup into my app and execute the function on the module level (having installed uvloop, of course). I could see a significant speed-up, same as I do for uvicorn with uvloop. (Requests took like 0.3-0.5 ms less time, which represents a significant part of the overhead.)

To me, it seems like it could possibly be very easy to implement this for unit; it seems like you can just do the same thing that uvicorn does.

ac000 commented 5 months ago

Having had a quick glance I don't immediately see where this requires changes to Unit. This seems to be more at the Python language level. You install some Python extension/module and use it... what am I missing?

bunny-therapist commented 5 months ago

I suppose you can set the event loop policy in you app, but given that other server runners for asgi does things like this "under the hood", it would mean that if you wanted to switch from, say, uvicorn to unit, you would have to modify your application code in order to get the uvloop speed-up. I am not entirely sure if there are any reasons why you would not want to do that, but given how uvicorn has done this, I would assume that the community standard is to not set any event loop policies in the application but instead just rely on the server runner to provide the event loop.

tippexs commented 5 months ago

I was just looking into the documentation of uvloop. I think what makes sense for now is to

  1. Document the options to enable uvloop and how to modify your existing project to make it runnable on Unit. I agree: The goal was and is always to migrate applications seamlessly from different application servers to Unit. Anything we can do to make this step easier for the user is definitely interesting.
  2. I would like to get some feedback / upvotes for this feature - so we can see if this is something other need as well. In the meantime - as mentioned in 1 - it would be great if we could have an uvloop example in our docs to make it easier for others.

Would you mind sharing the steps to add uvloop?

bunny-therapist commented 4 months ago

I just put this in my app so it ran at startup. I believe that made it go faster, so I believe it worked, but I have not done any deep analysis:

import asyncio
import uvloop

asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())

My understanding is that asyncio.set_event_loop_policy sets the type of loop to use for asyncio, in this case uvloop.EventLoopPolicy. Assuming that unit just uses asyncio (which I believe is the case, you just use it from C), I think this should have an effect. However, I have very little experience with C/python interfaces (even though I know the two languages separately quite well...).

ac000 commented 4 months ago

Do you have a basic hello world ASGI application using uvloop that you could share?

bunny-therapist commented 4 months ago
from litestar import get, Litestar
import asyncio
import uvloop

@get(path="/")
async def get_loop() -> str:
    return str(type(asyncio.get_event_loop_policy()))

# Configure to use uvloop:
asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())

app = Litestar(route_handlers=[get_loop])

If you run this with unit, then the / endpoint will tell you that it is using uvloop's event loop policy. If you comment out the asyncio.set_event_loop_policy line, it will tell you that it is using the default event loop policy.

This works with uvicorn too, the difference is that with uvicorn, you can tell it via cli/config to set the event loop policy for you, and you do not have to put in the app code.

ac000 commented 4 months ago

Thanks. Kind of in the middle of getting the 1.32 release ready, but I shall take a look (probably after), one thing I want to check is if I can see any sign of the uvloop stuff actually being used.

I installed the python-uvloop package and tried a simple script using uvloop, but could see no signs of the uvloop DSO even being loaded, hopefully your script will fair better...

stoiet commented 4 months ago

Currently we also use uvicorn with uvloop, but lately I've been thinking about switching to nginx unit, at least to give it a try. Unfortunately, I've also come across the fact that nginx unit uses the event loop from Python's asyncio module.

The asyncio' loop implementation is a "naive" one compared to what uvloop provides as it uses libuv under the hood.

So big upvote! The event loop implementation should be configurable for nginx unit, too.

@bunny-therapist I am a bit skeptical that it is solvable at the application level. If I am right, starting the event loop is the responsibility of the ASGI server implementation, and I am not sure what happens if you set the policy at the application level on an already running event loop.

bunny-therapist commented 4 months ago

I don't think setting the event loop policy from inside a running app would not work, but I think it could possibly work when setting it at the module level in the app (like in my example) so that the policy is set when the app code is imported, which is (probably?) earlier than the loop is started. However, uvicorn and probably other ASGI server implementations too use dynamic import of app using importlib (I think) which means that nothing is guaranteed.

ac000 commented 3 months ago

Hi @bunny-therapist

Thanks for your example, I can now indeed see that the uvloop shared library is loaded

00007fdf24b6c000    112K r---- /usr/lib64/python3.12/site-packages/uvloop/loop.cpython-312-x86_64-linux-gnu.so
00007fdf24b88000   1220K r-x-- /usr/lib64/python3.12/site-packages/uvloop/loop.cpython-312-x86_64-linux-gnu.so
00007fdf24cb9000    124K r---- /usr/lib64/python3.12/site-packages/uvloop/loop.cpython-312-x86_64-linux-gnu.so
00007fdf24cd8000      4K r---- /usr/lib64/python3.12/site-packages/uvloop/loop.cpython-312-x86_64-linux-gnu.so
00007fdf24cd9000    124K rw--- /usr/lib64/python3.12/site-packages/uvloop/loop.cpython-312-x86_64-linux-gnu.so

and your script reports

$ curl localhost:8080
<class 'uvloop.EventLoopPolicy'>

Just to double check, your expectation would be that you would still get that output after commenting out

asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
bunny-therapist commented 3 months ago

Yes, exactly. At least given the right configuration.

Uvicorn has the --loop argument (see here)

--loop <str> - Set the event loop implementation. The uvloop implementation provides greater performance, but is not compatible with Windows or PyPy. Options: 'auto', 'asyncio', 'uvloop'. Default: 'auto'.

If you set it to "asyncio" it uses asyncio (i.e. the default loop policy), if you set it to "uvloop", it uses uvloop (i.e., it sets the event loop policy to uvloop.EventLoopPolicy, same as that line). If you leave it at "auto", it will use uvloop if uvloop is installed and we are not using Windows nor PyPy.

My suggestion is for something similar.

ac000 commented 3 months ago

Thanks, that all makes sense...

ac000 commented 3 months ago

Going to park this for now.

Looked like it ought to be simple enough, but it no worky.

I'll document what I dud in case anyone feels like poking it...

We setup the asyncio loops in src/python/nxt_python_asgi.c::nxt_python_asgi_ctx_data_alloc()

So starting around line 344 I did something like

uvloop = PyImport_ImportModule("uvloop");
uvloop_pol = PyDict_GetItemString(PyModule_GetDict(uvloop),
                                  "EventLoopPolicy");
pol = PyObject_CallObject(uvloop_pol, NULL);

aselp = PyDict_GetItemString(PyModule_GetDict(asyncio),
                             "set_event_loop_policy");

pargs = Py_BuildValue("(O)", pol);
loop = PyObject_CallObject(aselp, pargs);

Which I think should mimic the Python code. However that last line seems to fail with

TypeError: bad argument type for built-in operation

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib64/python3.12/asyncio/events.py", line 795, in set_event_loop_policy
    if policy is not None and not isinstance(policy, AbstractEventLoopPolicy):
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SystemError: <built-in function isinstance> returned a result with an exception set

I tried various other permutations which resulted in other different errors...

bunny-therapist commented 3 months ago

I have no experience with using python from C, I'm afraid. Does it matter at all that uvloop appears to be implemented with .pyx and .pyd files? https://github.com/MagicStack/uvloop/tree/master/uvloop

ac000 commented 3 months ago

I'm going to guess not in this case.

Seeing as uvloop doesn't expose a public C API, you have to call the uvloop python functions using the PyObject_* API, and we know that basically works because the python test app works.