nest_asyncio icon indicating copy to clipboard operation
nest_asyncio copied to clipboard

Document tradeoffs

Open ulope opened this issue 3 years ago • 56 comments

Please add some explanation on what tradeoffs one takes on when using this library.

For example from very briefly looking at the code it seems that using this forces pure Python asyncio code to used. Is there a performance (or other) impact?

How can things break with nested loops?

Thanks.

ulope avatar Oct 07 '20 11:10 ulope

Yeah, I'd second that. This library is very widely referenced across stackoverflow, @erdewit

After using lua coroutines, I have to say the asyncio folks really need to rethink things.

blazespinnaker avatar Oct 19 '20 11:10 blazespinnaker

Interesting rebuttal to dynamic coroutines: https://mail.python.org/pipermail/python-dev/2015-April/139695.html

The issue boils down to the fact that when you allow the event_loop to be accessed wherever, the author feels you start to lose grasp of how execution control is being passed. But I'm not entirely sure the control flow is always obvious with lexical coroutines either.

Coroutines, at least for me, enable the benefits of concurrency (mostly with IO, but there are others) while reducing race conditions and increasing performance. Like threads, I never expected to know exactly what code is executing at any particular time. For those that can follow their coroutine code flow perfectly, that's great, but I severely doubt that is the common case among those responsible for doing actual real world work, especially as third party libraries are integrated into larger projects.

So, really I think the issue is less about being able step through all the code, and more about expectations of safety, fairness and starvation.

This library doesn't use threads, so I don't see how safety could be impacted. However, with coroutine the issue of fairness can be tricky, especially when its usage becomes dynamic like lua. And on the issue of fairness, I don't think this patch quite works. But let me know if my code is making the wrong assumptions about what re-entrant means.

Try these two out:

import nest_asyncio
nest_asyncio.apply()
import asyncio

async def echo(x):
    for i in range(0,5):
        print(x,i)
        sleep(1)

def testNested():
    for i in range(0,5):
        asyncio.get_event_loop().create_task(echo(i))
    inner()

def inner():
    for i in range(5,10):
        asyncio.get_event_loop().create_task(echo(i))
    sleep(20)

def sleep(z):
    asyncio.run(asyncio.sleep(z))

testNested()

import asyncio

async def echoUnnested(x):
    for i in range(0,5):
        print(x,i)
        await asyncio.sleep(1)

async def testUnnested():
   for i in range(0,10):
        asyncio.get_event_loop().create_task(echoUnnested(i))
   await asyncio.sleep(20)

asyncio.run(testUnnested())  

The latter unpatched approach runs as expected.

Unfortunately, something very unexpected is going on with the former. Looking at the patch code, run is supposed to use the already created event_loop rather making a new one. But the scheduled tasks are clearly not being executed concurrently.

blazespinnaker avatar Oct 19 '20 12:10 blazespinnaker

That all said, I would really like dynamic / re-entrant coroutines. Without it, supporting asyncio will require significant refactoring of a lot of libraries, and for those trying to just get things done, that seems more important than philosophical discussions on what levels of theoretical code complexity are acceptable.

..which looks like gevent fits the bill.

blazespinnaker avatar Oct 19 '20 12:10 blazespinnaker

For example from very briefly looking at the code it seems that using this forces pure Python asyncio code to used. Is there a performance (or other) impact?

The performance impact depends entirely on the situation. It's negligible for what I use it for. With PyPy the nested variant can actually be quite a bit faster than the unnested variant for some reason. Best thing is to measure it for your own code.

How can things break with nested loops?

I don't know really. This library gets used in ways that I'd never expected and if that leads to any issues then they would supposedly be filed here in the issue tracker.

erdewit avatar Oct 20 '20 08:10 erdewit

@erdewit I think most people are using as a hack to get things to work in jupyter and aren't measuring the fairness / performance. Is there something wrong with the code above? As far as I can tell the hack doesn't work. Which isn't surprising, as I dug into the code asyncio is really not meant to be nested.

I also looked at another repo that you were using asyncio in nested manner, and there are bugs, quite possibly because of this contradicting use.

I'd suggest recommending that users look at gevent. It is meant to be nested and is similar to the lua approach to coroutines.

Also, I wouldn't just go by lack of reports. It's like the sherlock holmes mystery when the dogs didn't bark.

blazespinnaker avatar Oct 20 '20 08:10 blazespinnaker

@blazespinnaker Your nested example uses coroutines that never await. You need await as a kind of portal for the flow of control to jump into other coroutines (this is just how Python works). Without it the coroutines can be scheduled concurrently but will execute serially. Exception here is the first iteration of the coroutines, which is considered ready by asyncio and executed in the same iteration of the event loop.

Or in other words, to get cooperative concurrency to work, the coroutines must be cooperative.

erdewit avatar Oct 20 '20 08:10 erdewit

@erdewit Thanks. How would you write the code then? With a custom run routine? I was going to try that, but as I looked into the asyncio code itself I saw too many stumbling blocks. Many which would likely increase in number as asyncio is uprgaded in the stdlib.

My sleep above uses run, which I assumed (wrongly I guess) that it would turn control over to the event_loop similar to what happens in lua / gevent.

I get what you're trying to do, but I think the forces of asyncio are going to work against this. I'd use gevent which is specifically designed to make this use case work.

blazespinnaker avatar Oct 20 '20 08:10 blazespinnaker

My sleep above uses run, which I assumed (wrongly I guess) that it would turn control over to the event_loop similar to what happens in lua / gevent.

The sleep does actually let the event loop run, which can then run other cooperative tasks. In your example there are no other cooperative tasks (there's nobody awaiting) so it appears nothing happens.

The coroutines can be made cooperative by using await asyncio.sleep(1) instread of sleep, but then you don't need any nesting to begin with.

To be clear, this library does not solve Python's colored function problem in the way gevent (or lua) has it solved.

erdewit avatar Oct 20 '20 09:10 erdewit

Well, there are no other tasks only if we're not using the same event_loop where the previous ones were created. I guess there is a new event loop in there somewhere I missed. If so, that can lead to serious starvation.

blazespinnaker avatar Oct 20 '20 10:10 blazespinnaker

No, there's only one event loop. There's also only one queue for the scheduled callbacks. It has time/insert-order priority, so every task gets its turn - no starvation there.

erdewit avatar Oct 20 '20 11:10 erdewit

Hmm, we seem to be going back and forth a bit here. If you look at the code above, create_task will schedule a task to run soon. Before sleep is executed in inner(), there should be about 10 tasks scheduled for execution on the event loop. Is the problem get_event_loop() call in asyncio.get_event_loop().create_task()?

If, as you say above:

"The sleep does actually let the event loop run, which can then run other cooperative tasks. In your example there are no other cooperative tasks (there's nobody awaiting) so it appears nothing happens."

Then the already scheduled tasks should run. I think the confusion here is that you are saying "there are no other cooperative tasks" and I'm not sure what that means, because asyncio.get_event_loop().create_task() should have scheduled 10 on the thread local event loop.

And indeed, when you run the code above, it runs the first 10 tasks very immediately on the first sleep but then starvation occurs when the internal nested sleeps in each task is hit, because they are not giving time / returning control to the thread local event loop during their sleeping period.

Note that I pasted in the patch code when I posted the bug, you can run it after the import asyncio.

blazespinnaker avatar Oct 20 '20 12:10 blazespinnaker

I think the confusion here is that you are saying "there are no other cooperative tasks" and I'm not sure what that means

It's mentioned a few times already, but it means that your coroutines contain no await statement.

await is the magic keyword for yielding and resuming coroutines and that is what makes cooperative concurrency possible. In gevent these yield/resume points are also present but hidden from the user.

erdewit avatar Oct 20 '20 16:10 erdewit

Ah, ok I see the confusion:

"My sleep above uses run, which I assumed (wrongly I guess) that it would turn control over to the event_loop similar to what happens in lua / gevent."

The sleep does actually let the event loop run, which can then run other cooperative tasks. In your example there are no other cooperative tasks (there's nobody awaiting) so it appears nothing happens.

Rather, I think what you meant to say was sleep would have worked if I used await instead of run. The bit about there not being other cooperative tasks threw me as there are tasks scheduled on the event_loop.

As I mentioned, I wrongly assumed that the patch library would allow for nested calls to the async library and use the same event loop. And of course I can't use await in a nested call. Using run, however, leads to the starvation I observed above.

Hopefully this makes sense to us both now.

blazespinnaker avatar Oct 20 '20 16:10 blazespinnaker

fwiw, this causes starvation as well:

async def sleepAsync(x):
    await asyncio.sleep(x)

def sleep(x):
    asyncio.run(sleepAsync(x))

As I mentioned above, I believe that the nested run is not using the same global event loop or at at the very least it's not slicing out time to tasks already scheduled on it.

When I say starvation, you just need to imagine any long running blocking IO being called on a nested run. Any tasks scheduled outside the run won't be fairly executed because the nested run doesn't give them any time.

blazespinnaker avatar Oct 20 '20 16:10 blazespinnaker

The problem that I see with this library is your run into things like this: https://stackoverflow.com/questions/57639751/how-to-fix-runtime-error-cannot-close-a-running-event-loop-python-discord-bot Where the sypder maintainer is suggesting to run the patch.

Better would be just checking for event_loop running, and if so use await, otherwise run:

https://stackoverflow.com/questions/50243393/runtimeerror-this-event-loop-is-already-running-debugging-aiohttp-asyncio-a

blazespinnaker avatar Oct 20 '20 17:10 blazespinnaker

I think most people are using as a hack to get things to work in jupyter

I think most people are using it upgrade legacy projects, and avoid having to inject async keywords through their entire codebase.

The idea that we can:

  • add event loops at the top of execution
  • money-patch blocking calls

... and not have to deal with a massive (backwards compatibility break) refactor is very appealing.

That Python asyncio is was an exclusive-or proposition was a bad design choice. It forces projects into two camps - those that use it, and use it exclusively, and those that do not. I like the middle-ground here.

allComputableThings avatar Oct 20 '20 17:10 allComputableThings

The latter unpatched approach runs as expected.

Perhaps, I think the behavior you see is what I would expect, unless I knew sleep was monkey-patched. (Wouldn't you see this behavior with vanilla Python3 asyncio + time.sleep?)

allComputableThings avatar Oct 20 '20 17:10 allComputableThings

The latter unpatched approach runs as expected.

Perhaps, I think the behavior you see is what I would expect, unless I knew sleep was monkey-patched. (Wouldn't you see this behavior with vanilla Python3 asyncio + time.sleep?)

Some people might, but I fail to why'd they want to use the library except in narrow circumstances. Any long running nested async task will deadlock their code.

eg - https://github.com/tensorflow/federated/issues/869

blazespinnaker avatar Oct 20 '20 17:10 blazespinnaker

Yeah -- clarity about what's monkey patched, and what is not seems essential. Is there a guide?

allComputableThings avatar Oct 20 '20 17:10 allComputableThings

@blazespinnaker:

...or at at the very least it's not slicing out time to tasks already scheduled on it.

Just like plain asyncio, gevent, lua, or any other type of cooperative concurrency.

It seems you're confused with preemptive concurrency (as used in multithreading for example).

erdewit avatar Oct 20 '20 18:10 erdewit

@blazespinnaker:

...or at at the very least it's not slicing out time to tasks already scheduled on it.

Just like plain asyncio, gevent, lua, or any other type of cooperative concurrency.

It seems you're confused with preemptive concurrency (as used in multithreading for example).

It might be less confusing for all involved if we are both more specific.

I am claiming that nested run calls with this library will not execute tasks scheduled before the nested run call.

So, if you have async library X which schedules async tasks in its event loop (after calling asyncio.run()) and then calls Library Y, and Library Y calls asyncio.run in a nested manner, it will block any scheduled tasks in library X until it completes in Library Y.

Agree or disagree?

blazespinnaker avatar Oct 20 '20 21:10 blazespinnaker

Btw, I am confused by this statement:

...or at at the very least it's not slicing out time to tasks already scheduled on it.

Just like plain asyncio, gevent, lua, or any other type of cooperative concurrency.

Gevent will absolutely execute the event_loop for tasks that are scheduled on it! This can be done from anywhere in your application, there is no notion of what is an async function. All you have to do is call gevent.sleep() and the event loop and attached tasks will execute.

If my phrasing "slicing out time" is what you are being semantic about, please don't :) I only use that term because many of my IO tasks are written in a way that they operate quickly and yield back control, and so only 'slices' of their over all execution lifetime occur before the next task in the event loop is executed - somewhat similar to the example I gave above, where each iteration of the for loop is a slice.

Giving each other the benefit of the doubt and focusing on the specific concerns will likely resolve this discussion much faster.

I have written event loop execution engines for lua coroutines and I'm fairly familiar with the issues involved here. In particular, starvation and fairness has always been the trickiest and often overlooked part.

blazespinnaker avatar Oct 20 '20 21:10 blazespinnaker

I am claiming that nested run calls with this library will not execute tasks scheduled before the nested run call.

If true, can you show failure case. The previous example you posted was ambiguous. Are you calling time.sleep? if so it will block all cooperative concurrency libraries?

allComputableThings avatar Oct 20 '20 22:10 allComputableThings

I am claiming that nested run calls with this library will not execute tasks scheduled before the nested run call.

If true, can you show failure case. The previous example you posted was ambiguous. Are you calling time.sleep? if so it will block all cooperative concurrency libraries?

import asyncio
import nest_asyncio
nest_asyncio.apply()

async def echo(x):
    for i in range(0,5):
        print(x,i)
        sleep(1)

def testNested():
    for i in range(0,5):
        asyncio.get_event_loop().create_task(echo(i))
    inner()

def inner():
    for i in range(5,10):
        asyncio.get_event_loop().create_task(echo(i))
    sleep(20)

def sleep(z):
    asyncio.run(asyncio.sleep(z))

testNested()

@stuz5000 sleep(z) is calling asyncio.run(asyncio.sleep(z))

asyncio.sleep() can be seen as a proxy for a long running blocking io task. Normally when the call isn't in a nested run, it will turn control over to the event_loop and run all scheduled async tasks. However, with this patch, the nested run doesn't execute any scheduled tasks that were created before on the 'outer loop'. You can just copy/paste this code into python

I think @erdewit understands this, but he has yet to confirm clearly that he does. That said, I don't think he appreciates the full implications of this problem and the subtle bugs that it is causing throughout the python ecosystem.

As a hack for jupyter, I suppose it has its uses, but much better would be to recommend users simply call asyncio routines like await instead of run.

blazespinnaker avatar Oct 21 '20 11:10 blazespinnaker

Here's another example - https://github.com/pyppeteer/pyppeteer/issues/99

blazespinnaker avatar Oct 21 '20 11:10 blazespinnaker

Going to submit a bug on python.org, but I thought it'd make sense to give @erdewit the opportunity to address it here first. Contents as follows:

"I'm seeing a high degree of confusion around nested asyncio calls in the ecosystem on both github and stackoverflow, where even large code base maintainers are not providing the correct advice

Eg, the error here - https://stackoverflow.com/questions/57639751/how-to-fix-runtime-error-cannot-close-a-running-event-loop-python-discord-bot

This has lead to a less optimal approach to addressing the error, nest_asyncio, rather than the correct approach of simply calling async routines via await.

The patch, which sort of works in narrow circumstances, has only exacerbated the community confusion and left code bases with subtle and hidden bugs which frequently appear on any library upgrades. Also, few of the patch users seem to appreciate that nest_asyncio blocks the outer event_loop when a nested run is called and is not re-entrant. The patch maintainer is so far unwilling to document and make clear this behavior.

Note also that that there are some members in the community that have a misguided desire to undermine asyncio because of a philosophical disagreement with how it was designed. This is unfortunate and advocating a sub optimal monkey patch which reduces the value of asyncio is not the correct way to address these theoretical differences of opinion.

I would recommend tweaking the nesting errors in asyncio and then documenting the new version early so search engines will pick it up. Having an early and well placed stackoverflow question would help significantly as well.

Ideally the official documentation/answers will explain succinctly and simply the logic and philosophy behind the so called "colored functions" constraint of asyncio to reduce code complexity as well as genuinely discuss alternatives (such as gevent) for those who wish to try a different approach.

This would be a mature and transparent solution which would not only educate the community but also allow for the best approach to succeed.

Differences of opinions can be healthy when addressed openly and head on rather than via methods that obfuscate their intentions. Listing alternatives provide an important safety valve that reduces heat and increases the light brought to a technical debate."

blazespinnaker avatar Oct 21 '20 12:10 blazespinnaker

sleep(z) is calling asyncio.run(asyncio.sleep(z))

If that starves a loop, that does smell like a bug.

allComputableThings avatar Oct 21 '20 18:10 allComputableThings

The same example seems to have been re-posted, making this this whole thread a bit circular. There's still only synchronous tasks, meaning with no await in them. It has been mentioned about four times already now to used await so that the tasks can play ball with each other.

Now these tasks, while being executed synchronously, will each spin the event loop. This is easily proven by adding a proper asynchronous task, one that actually awaits. Lets add a task called ping that prints the current time:

import asyncio
import time
import nest_asyncio
nest_asyncio.apply()

async def coro(x):
    for i in range(0, 5):
        print(f'coro{x}: {i}')
        sleep(0.1)

async def ping():
    t0 = time.time()
    while True:
        await asyncio.sleep(0.1)  # look I'm awaiting!
        print('PING', time.time() - t0)

def sleep(z):
    asyncio.run(asyncio.sleep(z))


asyncio.ensure_future(ping())
for i in range(0, 10):
    asyncio.ensure_future(coro(i))
sleep(5)

The example is also rewritten a bit for better clarity. It gives the following output:

erdewit avatar Oct 26 '20 10:10 erdewit

coro0: 0
coro1: 0
coro2: 0
coro3: 0
coro4: 0
coro5: 0
coro6: 0
coro7: 0
coro8: 0
coro9: 0
PING 0.10097742080688477
coro9: 1
PING 0.20212650299072266
coro9: 2
PING 0.3029673099517822
coro9: 3
PING 0.4039328098297119
coro9: 4
PING 0.5045821666717529
coro8: 1
PING 0.6054215431213379
coro8: 2
PING 0.7060284614562988
coro8: 3
PING 0.8067624568939209
coro8: 4
PING 0.907468318939209
coro7: 1
PING 1.0082740783691406
coro7: 2
PING 1.1092665195465088
coro7: 3
PING 1.2097752094268799
coro7: 4
PING 1.3103570938110352
coro6: 1
PING 1.4109225273132324
coro6: 2
PING 1.5113551616668701
coro6: 3
PING 1.6120128631591797
coro6: 4
PING 1.7126681804656982
coro5: 1
PING 1.81358003616333
coro5: 2
PING 1.914428949356079
coro5: 3
PING 2.0151619911193848
coro5: 4
PING 2.115638256072998
coro4: 1
PING 2.2162415981292725
coro4: 2
PING 2.317066192626953
coro4: 3
PING 2.4179599285125732
coro4: 4
PING 2.5189225673675537
coro3: 1
PING 2.619631290435791
coro3: 2
PING 2.720113754272461
coro3: 3
PING 2.8207244873046875
coro3: 4
PING 2.9210634231567383
coro2: 1
PING 3.021679401397705
coro2: 2
PING 3.12245512008667
coro2: 3
PING 3.2229690551757812
coro2: 4
PING 3.3236045837402344
coro1: 1
PING 3.4240927696228027
coro1: 2
PING 3.52492356300354
coro1: 3
PING 3.625823497772217
coro1: 4
PING 3.7266714572906494
coro0: 1
PING 3.8276121616363525
coro0: 2
PING 3.9286460876464844
coro0: 3
PING 4.0296266078948975
coro0: 4
PING 4.130501985549927
PING 4.2312047481536865
PING 4.3317811489105225
PING 4.432348251342773
PING 4.532992839813232
PING 4.633592128753662
PING 4.7342023849487305
PING 4.834784746170044
PING 4.935179710388184

erdewit avatar Oct 26 '20 10:10 erdewit

The only possible WTF moment is the first step of each task, which are executed all at the beginning. This is due to an optimization for asyncio.sleep(0) by asyncio. The main take-away is that there is an event loop running that is executing the asynchronous tasks scheduled on it.

erdewit avatar Oct 26 '20 10:10 erdewit