Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

Document tradeoffs #36

Open
ulope opened this issue Oct 7, 2020 · 56 comments
Open

Document tradeoffs #36

ulope opened this issue Oct 7, 2020 · 56 comments

Comments

@ulope
Copy link

ulope commented Oct 7, 2020

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.

@blazespinnaker
Copy link

blazespinnaker commented Oct 19, 2020

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
Copy link

blazespinnaker commented Oct 19, 2020

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
Copy link

blazespinnaker commented Oct 19, 2020

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.

@erdewit
Copy link
Owner

erdewit commented Oct 20, 2020

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.

@blazespinnaker
Copy link

blazespinnaker commented Oct 20, 2020

@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.

@erdewit
Copy link
Owner

erdewit commented Oct 20, 2020

@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.

@blazespinnaker
Copy link

blazespinnaker commented Oct 20, 2020

@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.

@erdewit
Copy link
Owner

erdewit commented Oct 20, 2020

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.

@blazespinnaker
Copy link

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.

@erdewit
Copy link
Owner

erdewit commented Oct 20, 2020

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.

@blazespinnaker
Copy link

blazespinnaker commented Oct 20, 2020

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.

@erdewit
Copy link
Owner

erdewit commented Oct 20, 2020

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.

@blazespinnaker
Copy link

blazespinnaker commented Oct 20, 2020

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
Copy link

blazespinnaker commented Oct 20, 2020

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
Copy link

blazespinnaker commented Oct 20, 2020

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

@allComputableThings
Copy link

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
Copy link

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?)

@blazespinnaker
Copy link

blazespinnaker commented Oct 20, 2020

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 - google-parfait/tensorflow-federated#869

@allComputableThings
Copy link

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

@erdewit
Copy link
Owner

erdewit commented Oct 20, 2020

@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).

@blazespinnaker
Copy link

blazespinnaker commented Oct 20, 2020

@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
Copy link

blazespinnaker commented Oct 20, 2020

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.

@allComputableThings
Copy link

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?

@blazespinnaker
Copy link

blazespinnaker commented Oct 21, 2020

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
Copy link

Here's another example - pyppeteer/pyppeteer#99

@blazespinnaker
Copy link

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."

@allComputableThings
Copy link

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

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

@erdewit
Copy link
Owner

erdewit commented Oct 26, 2020

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
Copy link
Owner

erdewit commented Oct 26, 2020

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

@allComputableThings
Copy link

allComputableThings commented Nov 1, 2020 via email

@blazespinnaker
Copy link

blazespinnaker commented Nov 1, 2020

@erdewit

That's great that works, however you are not nesting asyncio in that case, or at least you're doing it in a cherry picked manner. Sorry, I had tried using 'await' but it didn't work. Starvation still occurs in the way I describe above (several times)

It completes in about 50 seconds, which makes sense because there are 50 sleeps of 1 seconds, all executed serially, blocking everything else as they go.

For example:


import asyncio
import nest_asyncio
nest_asyncio.apply()
import time

async def echo(x):
    for i in range(0,5):
        print(x,i)
        #this line is meant to be a proxy for a nested call to asyncio.  This will block everything else until it finishes running.
        #Even if I have called ensure_future 10 times already none of those tasks will execute until this async
        #sleep completes by itself.  Put another way, it almost seems like when I call run again (in sleep below) that it
        #creates a new event loop, completes the asyncio.sleep on that and then returns control back to the previous event loop
        #something else may be happening, but the effect is still the same.   The echo coroutines created are not being
        #given any execution until this sleep completes.   This is not how asyncio works.  When you're sleeping, other
        #tasks on the event loop should execute.
        mySleep(1)   

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

def inner():
    for i in range(5,10):
        asyncio.ensure_future(echo(i))
    mySleep(20)

async def asyncSleep(z):
    await asyncio.sleep(z) #<- happy?

def mySleep(z):
    asyncio.run(asyncSleep(z))

t0 = time.time()
testNested()
print("Time: ", time.time() - t0)

@stuz5000 just copy/paste code into latest python (3.8?) an it worked for me.

@blazespinnaker
Copy link

blazespinnaker commented Nov 1, 2020

This does work however, but I'm not nesting the sleep. It completes in about 5 seconds. Which makes sense, as echo takes 5 seconds to execute (5 sleeps), and all the echos are executed in parallel fashion.

import asyncio
import nest_asyncio
nest_asyncio.apply()

async def echo(x):
    for i in range(0,5):
        print(x,i)
        #this is the only line I changed from the previous example, I don't nest the call to asyncio.sleep()
        #so it doesn't block everything in order to wait for this to finish.
        #in other words, the sleep gives control to the event loop which executes all of the ensure_futures
        await asyncio.sleep(1)

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

def inner():
    for i in range(5,10):
        asyncio.ensure_future(echo(i))
    mySleep(5)

async def asyncSleep(z):
    await asyncio.sleep(z) #<- happy?

def mySleep(z):
    asyncio.run(asyncSleep(z))

t0 = time.time()
testNested()
print("Time: ", time.time() - t0)

@blazespinnaker
Copy link

@erdewit

Actually, your code exhibits the same behavior as mine and performs the nested run starvation I mentioned. Try increasing the sleep delay, it's more obvious.

@erdewit
Copy link
Owner

erdewit commented Nov 1, 2020

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

Apart from a bit of renaming and obfuscation, it's still the same as the first example, still no await, still synchronous non-cooperative tasks, and still working as expected.

asyncio.run is a synchronous method, just like time.sleep, the only difference is that it does some useful work like spinning the event loop. It does not magically transfer the flow of control to other sync code.

@blazespinnaker
Copy link

blazespinnaker commented Nov 2, 2020

@erdewit Sigh. Your code does the same thing as my code. Have we just been saying the same thing all along?

Probably, but..

I am saying that this is undesired behavior. Nested calls which starve any previous tasks created on the thread global event loop is not what people would expect and would only be useful in very narrow circumstances.

I would say documenting this and making it clear would be a very good idea.

@blazespinnaker
Copy link

blazespinnaker commented Nov 3, 2020

If people want to make nested calls, a much better library for that is gevent. Which doesn't starve previously created tasks

@erdewit
Copy link
Owner

erdewit commented Nov 3, 2020

Gevent has the exact same type of behavior for non-cooperative (non-yielding) tasks. Good luck with it.

@blazespinnaker
Copy link

blazespinnaker commented Nov 4, 2020

!?

Absolutely it does not have the same type of behavior:

import gevent
import time

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

def testNested():
    for i in range(0,5):
        gevent.spawn(echo,i)
    inner()

def inner():
    for i in range(5,10):
        gevent.spawn(echo,i)
    mySleep(6)

def mySleep(z):
    gevent.sleep(z)

t0 = time.time()
testNested()
print("Time: ", time.time() - t0)

I don't think you understand coroutines or how they are being used, and this has lead to a monkey patch which is causing a lot of chaos in the ecosystem.

@erdewit
Copy link
Owner

erdewit commented Nov 4, 2020

Now try it with time.sleep instead of gevent.sleep to get an honest comparison. I've de-obfuscated the script a bit again:

import gevent
import time

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


t0 = time.time()
for i in range(0, 10):
    gevent.spawn(echo, i)
gevent.sleep(0)
print("Time: ", time.time() - t0)

Output:

...
8 0
8 1
8 2
8 3
8 4
9 0
9 1
9 2
9 3
9 4
Time:  50.05208897590637

It is seen that the tasks are executing serially, exactly as asyncio did with non-cooperative tasks.

@blazespinnaker
Copy link

blazespinnaker commented Nov 4, 2020

You would never use time.sleep(1) in a cooperative concurrency program, for the very obvious reason that it would block the thread. It would be almost as bad as doing a long running spin lock.

You use gevent.sleep or asyncio.sleep to sleep, as it will pass control to the event_loop handler and coroutines will be given execution time.

The whole reason people use nest_asyncio is because programs like jupyter (or spyder or tensorflow or etc) are running potentially sync functions in the context of thread which is currency running async. Async functions can, however, call sync functions. Once that happens, the wise lords of asyncio have decided to not allow you to run async. Rather they throw a runtime error. nest_asyncio gets rid of the runtime error and allows you to do this.

However, if you try to use nest_asyncio and run a coroutine from a sync function with either asyncio.run or loop.run_until_complete, it will block or starve every other coroutine created before the run call, even if you call asyncio.sleep() within the coroutine being ran.

Gevent doesn't do this to you. You can absolutely run a sleep coroutine regardless of where you are in the code and it will pass control to the event_loop handler. Every spawned coroutine will get a chance to execute. Gevent does not have the notion of run() that asyncio has.

@erdewit
Copy link
Owner

erdewit commented Nov 5, 2020

You would never use time.sleep(1) in a cooperative concurrency program,

That is obvious, but why then expect anything different from asyncio.run. It is just as synchronous as time.sleep. That's the whole point.

asyncio.run doesn't yield to the event loop, instead it starts spinning the event loop itself. That is what coroutine runners do and that is what makes them synchronous by nature.

However, if you try to use nest_asyncio and run a coroutine from a sync function with either asyncio.run or loop.run_until_complete, it will block or starve every other coroutine created before the run call, even if you call asyncio.sleep() within the coroutine being ran.

The ping coroutine given earlier already proves that this is not true. It runs just fine, all the time, since there is an event loop running all the time.

@blazespinnaker
Copy link

blazespinnaker commented Nov 5, 2020

asyncio.run doesn't yield to the event loop, instead it starts spinning the event loop itself. That is what coroutine runners do and that is what makes them synchronous by nature.

Yeah, I think I said that about 4 or 5 times above. This is not very useful, as run is the only way to do nested calls.

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.
..

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.

..

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.

..

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.

..

Actually, your code exhibits the same behavior as mine and performs the nested run starvation I mentioned.

..

I am saying that this is undesired behavior. Nested calls which starve any previous tasks created on the thread global event loop is not what people would expect and would only be useful in very narrow circumstances.

Gevent, however, let's you nest calls to coroutines and it doesn't spin a new event loop or starve any previously spawned coroutines.

@allComputableThings
Copy link

Perhaps related:
#44

@douglas-raillard-arm
Copy link

I had the same concern that nested asyncio.run() calls would starve pre-existing tasks so I came up with this little test. My conclusion is that nested asyncio.run() calls are cannot be distinguished from a normal await, i.e. pre-existing tasks keep being executed by nested asyncio.run().

Maybe I missed something as I have not tried to run other examples of this discussion, or maybe I'm in an edge case that happens to work for "wrong" reasons.

Tested on Python 3.7, 3.8, 3.9 and 3.10

import asyncio

import nest_asyncio
nest_asyncio.apply()

async def release(lock):
    # "ensure" we only release after asyncio.run() had a chance of starting to
    # run, so that we can see whether it starves us or not. If it does, we will
    # never release the lock and _acquire_again() will therefore deadlock.
    await asyncio.sleep(1)
    lock.release()
    print('released !')

async def acquire_again(lock):
    coro = _acquire_again(lock)
    # "await" or "asyncio.run()" can be used here to demonstrate what happens
    # in the "normal" await-based flow, or with a blocking call to
    # asyncio.run().
    # If asyncio.run() does not also runs the pre-existing tasks,
    # _acquire_again() will deadlock since the lock is released by a
    # pre-existing task.

    # await coro
    asyncio.run(coro)

async def _acquire_again(lock):
    print('trying to acquire')
    await lock.acquire()
    print('acquired !')
.
# If asyncio.rdoes not also runs the pre-existing tasks, _acquire_again() will
# deadlock since the lock is released by a pre-existing task.

async def main():
    lock = asyncio.Lock()
    await lock.acquire()

    t1 = asyncio.create_task(acquire_again(lock))
    t2 = asyncio.create_task(release(lock))

    # Just to show that the order in which we await t1 and t2 does not matter,
    # they both complete long before this sleep is finished.
    await asyncio.sleep(3)

    await t1
    await t2

asyncio.run(main())
# Expected output for both await and asyncio.run():
#
# trying to acquire
# <here a 1s gap>
# released !
# acquired !

@erdewit
Copy link
Owner

erdewit commented Mar 16, 2022

My conclusion is that nested asyncio.run() calls are cannot be distinguished from a normal await, i.e. pre-existing tasks keep being executed by nested asyncio.run().

Yes, absolutely true. This also holds for "live patching", that is applying nest_asyncio after a task has already started.

@blazespinnaker
Copy link

blazespinnaker commented Mar 20, 2022

@douglas-raillard-arm

Why would you bother using nest patching? Your code is all async already - just use asyncio. The point of using something like nest async is not to have async everywhere, but rather being able to return control to event loop handler from synchronous code. This is what things like gevent provide.

Also, who would write code with asyncio.run in an async function? That makes zero sense. That should error out, IMHO. The fact that patching gets rid of that error seems more confusing than helpful.

@blazespinnaker
Copy link

blazespinnaker commented Mar 20, 2022

Ok, so hopefully final version.

This is async:

def mySleep(z):
    gevent.spawn(gevent.sleep, z)

This is sync / blocking (even after monkey patching):

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

To meet folks half way, agreed, doesn't starve previous, but because it blocks it can't nest libraries that use asyncio.run and get cooperative behavior.

The patch is great if you have to just make something work and there is no other way, but folks should be aware of the limitations as compared to other cooperative concurrency approaches. Sadly, the repo owner refuses to alert people to this.

@douglas-raillard-arm
Copy link

Why would you bother using nest patching?

Literally the first line of my post:

I had the same concern that nested asyncio.run() calls would starve pre-existing tasks so I came up with this little test.

Also, who would write code with asyncio.run in an async function? That makes zero sense. That should error out, IMHO.

One can be interested of gradually converting blocking code to async code. In that context, an coroutine function could well still call the blocking API, regardless of how the blocking API is implemented (i.e. actually blocking or as a blocking shim on top of a coroutine function).
There are other use case that are very real as well, e.g. async code using a blocking library that asks for a callback. If that callback needs to make uses of coroutine functions, it can be run with asyncio.run().

Anyhow, all I wanted is to clear out for myself and for others wondering the same question whether this package was starving other tasks. Turns out it does not. Since the package does not pretend to provide a way to spawn tasks concurrently other than the standard asyncio.create_task(), which is documented to ask for a coroutine and still works as intended, I don't see any issue here, i.e.

async def non_yielding():
    asyncio.run(...)

await asyncio.gather(
    coroutine_function(),
    non_yielding(),
)

One might think that coroutine_function() will be starved because non_yielding() never awaits and therefore hogs the loop. It does not. The nested asyncio.run() will take care of running running the loop while it's "blocked". This means no starvation, and no risk of deadlock coming from it. I don't think there is more to it than that.

Thanks @erdewit for uploading this package on PyPI and maintaining it.

@blazespinnaker
Copy link

blazespinnaker commented Mar 21, 2022 via email

@devstein
Copy link

Hi @erdewit I'm considering using this library to help adapt the Sparkmagic Jupyter Kernel to support ipykernel>=6, which executes cells asynchronously.

Are there any obvious issues I should be aware of? What are the trade-offs between this library and https://pypi.org/project/asgiref/?

Any help is appreciated!

@erdewit
Copy link
Owner

erdewit commented Apr 27, 2022

@devstein

asgiref's AsyncToSync is probably not suitable for your use case, since it can't run in one thread that already has a running event loop.

For nest_asyncio I do not see any obvious issues. Perhaps it can be done without nesting, using straight asyncio.

@devstein
Copy link

Thanks for the quick reply @erdewit!

How would you suggest using straight asyncio without nesting?

@erdewit
Copy link
Owner

erdewit commented Apr 27, 2022

@devstein I don't know, perhaps look how other kernel magic projects do it.

@auxsvr
Copy link

auxsvr commented Apr 27, 2022

SqlAlchemy does something similar with greenlets, allowing you to run a coroutine without starting a new thread.

@devstein
Copy link

Relevant commit to a jupyter/notebook fork: penguinolog/notebook@11615d2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants