dabeaz / curio

Good Curio!
Other
4.01k stars 240 forks source link

python asyncio create_task() odd behavior #342

Closed yanghao closed 1 year ago

yanghao commented 3 years ago

@dabeaz , if you are around, what do you think about this issue? https://bugs.python.org/issue43736

To summarize, below two code snippet behaves differently ...

This code took 10 seconds to complete:

async def task(name, n): 
    for i in range(n):
        print(f"task-{name}: ", i, time())
        await sleep(1)

async def top():
    t0 = create_task(task("T0", 10))
    t1 = create_task(task("T1", 10))
    await t0
    await t1 # actually await t1 is NOT needed anymore ...

And this one took 20 seconds to complete:

async def top():
    await create_task(task("T0", 10))
    await create_task(task("T1", 10))

And python asyncio maintainers doesn't seem to think this is an issue ... what do you think?

(of course I tested with curio and it works either ways).

dabeaz commented 3 years ago

Well, that indeed is interesting behavior. My only thought on this (in general) is that literally every operation in Curio that involves async is an async function without exception. So, there is no concept of a synchronous "create_task" function---you have to use t = await spawn(). Waiting for the task to finish is a separate operation entirely (await t.join()). So, I suppose in that sense Curio probably has a more refined decomposition of the problem of creating and waiting for tasks.

Fixing asyncio in ways that break backwards compatibility is a lost cause though. So, no advice on that. There's a lot of weird stuff in asyncio concerning task creation, cancellation, error handling, and waiting. I could never quite wrap my brain around all of it.

yanghao commented 3 years ago

Thanks @dabeaz , your async tutorials inspired me a lot, especially the analogy to OS kernel, you are one of the reasons I stayed in Python and getting deeper into it everyday.

I agree fixing create_task() does not make much sense but I thought creating a sane version create_task_sane() maybe doable. I never really used asyncio anyway (used curio and created a domain specific version of curio), I was just really surprised when I see such behavior while preparing a simple example code snippet for someone else.

I think this once again proves how important conceptual integrity is at the very beginning when creating a design, and curio totally wins over asyncio in this aspect. I will put a lot more effort in my organization introducing curio instead of asyncio from now on. And thanks again for the great tutorials and libraries you created!

arthur-tacca commented 1 year ago

Possibly of interest, but a helper class I've suggested for a Trio utility library would separate out task creation and actually running the task - see link above. You would never be able to accidentally do await Task(fn) because the task itself isn't awaitable, and await Task(fn).run() is more obviously wrong.

(Having said that, I strongly disagree with the premise of this issue. await communicates an important fact to the reader: this code might wait while something completes. Using await for something like create_task, which always returns instantly, is not just redudant but misleading. I spent a long time staring at your second snippet, the one that took 20 seconds, wondering what on earth you expected it to do. It's clear that each line creates a task and then waits for it to finish because it says so, that's what await means. I only figured out what you expected from reading you later comments. I'm afraid it's Curio's API that needs fixing here.)

yanghao commented 1 year ago

await does communicate with the user while doing await create_task(), it tells the user "I am waiting for the kernel/scheduler/os to create a new task", and this can be either successful or fail. And in certain specialized scheduler, this could also just hang/blocking if no "resources" are available to do so right now. And if not using await for such cases, it cannot block in the middle of a function execution. So no, I don't think it is mis-leading. This is about compose-ability, e.g. creating the task object and actually creating a runnable task should be two orthogonal things instead of intertwined into one thing.

The second example that took 20 seconds, is semantically identical to the first example but produces a different result. It is just a sign of a corrupted design. As in Python, everyone would expect the same result for yield from xyz() and t = xyz(); yield from t. This is how yield is designed, this is how await/async is designed, but this is not how python's asyncio is designed!

arthur-tacca commented 1 year ago

The second example that took 20 seconds, is semantically identical to the first ...

OK let's imagine asyncio works the way you want it to: create_task() doesn't run anything, even in the background, until you await. Then yes, your snippets are semantically identical. But they actually both wait for 0 seconds because (by your logic) each use of await is just starting the task and not waiting for it. I'm still not convinced that's what you meant. To wait any time at all, you'd need to await something else, the result of the first await maybe, so your first example would have to look like this:

t0 = create_task(task("T0", 10))  # Creates task but doesn't run it
t1 = create_task(task("T1", 10))  # Creates task but doesn't run it
a0 = await t0  # Starts running the first task in the background
a1 = await t1  # Starts running the second task in the background
await a0  # Waits for task t0 to complete (takes 10 seconds)
await a1  # Waits for task t1 (but it's already done by now)

await does communicate with the user while doing await create_task(), it tells the user "I am waiting for the kernel/scheduler/os to create a new task"

At this point, we're talking about what a human reader understands the code to mean, so it's subjective. In my opinion, the philosphically correct meaning of the second snippet is to wait for 20 seconds. To you it does not. I suppose we have to leave it at that.

This is about compose-ability, e.g. creating the task object and actually creating a runnable task should be two orthogonal things instead of intertwined into one thing.

NO! Separating task creation from task running is a very valid request, but your request about await is not about that at all. It's totally possible for task creation to be separate from task running without any extra await. It would look like this:

t0 = create_task(task("T0", 10))  # Creates task but doesn't run it
t0.start_in_background()   # Now it's running - no await in sight!
await t0   # Now we wait for it to complete.

In that linked Python bug, when the asyncio developers said "the ship had sailed" on being able to create a task without running it, they were talking about this sort of syntax. They were never considering adding a redudant await.

Funnily enough, curio is an example of the exact opposite: You need an extra await, which you like, but it's not possible to create a curio.Task object representing a task that has not yet started.

yanghao commented 1 year ago

But they actually both wait for 0 seconds because (by your logic) each use of await is just starting the task and not waiting for it

You obviously doesn't realize the event loop will wait until everything completes. In the minimal example, I do not have to wait for the task to complete, that is done by the asyncio event loop ... these examples only talking about semantic discrepancies of starting a task, not how to wait for completion. You have gather() in asyncio and join() in curio ......

At this point, we're talking about what a human reader understands the code to mean, so it's subjective. In my opinion, the philosphically correct meaning of the second snippet is to wait for 20 seconds. To you it does not. I suppose we have to leave it at that.

NO, not at all, and don't assume I am not human. It might mean wait for 20 seconds for you, it means starting the task for me (and many others like curio author). The waiting for 20 seconds thing is done in a completely different thread, so the "async def task()" function has nothing to do with it, it just creates the task, not doing the task.

NO! Separating task creation from task running is a very valid request, but your request about await is not about that at all. It's totally possible for task creation to be separate from task running without any extra await. It would look like this:

You don't get the point. I don't really care about if there is a separation at all for asyncio, I care because semantically identical code in asyncio shows completely different behavior, this is completely non-sense for any serious programming language or libraries. It is like "a == b" and "b == c" and now you tell me "a != c". This is complete total non-sense for me.

In that linked Python bug, when the asyncio developers said "the ship had sailed" on being able to create a task without running it, they were talking about this sort of syntax. They were never considering adding a redudant await.

Nope, not at all. I think you misread them too. It is saying "they could change the behavior", however "lots of existing code are depending on the wrong behavior to work" (hence the ship has sailed). I think they do realize the issue but simply do not want to break user's code that depends on the wrong behavior.

Funnily enough, curio is an example of the exact opposite: You need an extra await, which you like, but it's not possible to create a curio.Task object representing a task that has not yet started.

You don't really know what you are talking about ... please go home and do your homework before making conclusions.

nocturn9x commented 1 year ago

To be completely honest, I agree with @arthur-tacca in that await means "this thing may block", which is not the case when simply scheduling a task (not in any asyncio implementation that I'm aware of, anyway). In a toy event loop/library I made, task creation still requires an await simply because I didn't like the API asymmetry (much like some of trio's socket methods do not need to be asynchronous, but just are for consistency purposes). It also fits the idea of "just add await everywhere and it'll work", which is useful to newcomers to the paradigm, IMHO

yanghao commented 1 year ago

that await means "this thing may block", which is not the case when simply scheduling a task (not in any asyncio implementation that I'm aware of, anyway).

Everything may block. My task start in my event loop definitely may block. In fact, in certain types of event loop design, it has to block. In other cases like curio, it is simply trying to maintain the conceptual cleanness of an OS design, that every OS service is triggered with a trap using await.

But again, these are not the points. The points being:

In Python, or any programming language, semantic consistency is very important.

In C, you can do: y = !x();, which is identical to: t = x(); y = !t;.

In Python, you can do: y = await x(), which is identical to: t = x(); y = await t;.

This is no longer true for asyncio task creation ...

nocturn9x commented 1 year ago

In Python, you can do: y = await x(), which is identical to: t = x(); y = await t;.

This is no longer true for asyncio task creation ...

That's because asyncio is not well designed for modern asynchronous programing and it has no consistency in its API whatsoever. It's just how the library is, which is why more modern alternatives such as trio and curio exist. Nothing to do with some intrinsic rule with Python: in modern async python there is no way to break down the "await part" from the "create task" part: it's all a single unit (and it's kinda the point of post PEP-492 frameworks)

yanghao commented 1 year ago

That's because asyncio is not well designed for modern asynchronous programing and it has no consistency in its API whatsoever. It's just how the library is, which is why more modern alternatives such as trio and curio exist.

Exactly!

Nothing to do with some intrinsic rule with Python

Also true. The python language's async/await is a very good design, but asyncio library is not ... (and asyncio is a post PEP492 framework ...), no excuses for asyncio ;-)

arthur-tacca commented 1 year ago

@yanghao

In that linked Python bug, when the asyncio developers said "the ship had sailed" on being able to create a task without running it, they were talking about this sort of syntax. They were never considering adding a redudant await.

Nope, not at all. I think you misread them too. It is saying "they could change the behavior", however "lots of existing code are depending on the wrong behavior to work" (hence the ship has sailed). I think they do realize the issue but simply do not want to break user's code that depends on the wrong behavior.

Yes, they would change the behaviour but can't because it would break existing code. We already agree on that. What disagree about is what they would change.

The comment we're talking about talks about two different things. They even number them 1 and 2. To make clear they are different things.

Point 1 in that Python issue is use of await to create a task, which is the thing you originally brought up (here on github and also on that Python issue). So, just to be really clear, point 1 is about this:

# Point 1 in the Python comment: use of await

# Current asyncio: no await used to create task
t = create_task(foo())

# Suggested by yanghao: need await to create a task
t = await create_task(foo())

But if you look at what they say about this, they're just explaining to you why they think the current API is correct. There are no ships sailing here. They don't quite spell out the conclusion, but the implication is clear: they are saying that it doesn't make sense for create_task() to need await because it's not a 'yield point'.

Point 2 in that Python issue is something totally different (that's why they gave it a different number). It's about "the separation of task creation and start". That means, there should be some way to create a task - an actual asyncio.Task object - that has not yet started, with a separate method to start it. Something like this:

# Point 2 in the Python comment: separation of task creation and start

# Current asyncio: task starts as soon as it is created
t = create_task(foo())

# Would be considered by asyncio devs: task started only when requested
t = create_suspended_task(foo)  # Suspended initially
t.start()   # Now it's started

No need for await here; that is besides the point. They are clear that this is what they wish they had implemented. Not your request.

arthur-tacca commented 1 year ago

Funnily enough, curio is an example of the exact opposite: You need an extra await, which you like, but it's not possible to create a curio.Task object representing a task that has not yet started.

You don't really know what you are talking about ... please go home and do your homework before making conclusions.

(That language is not helpful, and actually just undermines your point. It's also factually not true. But putting that aside...)

If I'm wrong, please enlighten me: how do I create a curio.Task object that represents a task that has not started, and will not do so until I specifically request it?

This comes back to the difference between point 1 vs point 2. You have a preference about point 1. But, confusingly, you describe it as separating task creation and start, which it isn't at all (that description actually means code like point 2).

In Python, you can do: y = await x(), which is identical to: t = x(); y = await t;

This is no longer true for asyncio task creation ...

I already said this: those two things absolutely ARE identical. Again, you're harming your own argument here, because it's so obvious that those thing are equivalent that it's undermining yourself to claim they're not. (Yes, they are different if you insert a statement in the middle that has side effects; but that's true of your C example t = x(); y = !t; too.)

NO, not at all, and don't assume I am not human.

One thing I do want to correct here, sorry if my comment was open to confusion. I certainly did not mean to imply you are not human. Exactly the opposite: I was saying that since we are both human, it is reasonable to expect us to have different opinions about things. (This is the opposite stance than you have by declaring asyncio to have a "broken API" - that is another case of not showing yourself in the best light, which undermines your point.)

yanghao commented 1 year ago

@arthur-tacca

(That language is not helpful, and actually just undermines your point. It's also factually not true. But putting that aside...)

Sorry if that language is not helpful for you, but I mean it. you call curio spawn() and it gives you an awaitable task to be started. This is really simple and it is all over the place in curio's tests and examples ...

Besides that, I swear I read your lengthy comment 3 times and cannot understand much ... as a final try, I can only say that logical consistency is very key in any programming language. Programming language follows mathematics algebra a lot, so if y = await x(), then it is logically necessary that t = x(); y = await t also holds true (e.g. identical behavior), and this is the case in almost all programming languages I know of. If that breaks, the entire reasoning foundation is broken.

That is also the reason, me included, and many other I know have moved away from python's standard asyncio library.

nocturn9x commented 1 year ago

The point of this whole discussion is that most modern libraries specifically disallow separating task creation from task execution because they focus on a more structured concurrency paradigm: you don't "create tasks", but rather just call async functions and wait for them to complete. The idea is to make async code look like sync code as much as possible. If you want to do what you're looking for, maybe reimplementing functools.partial to work with coroutines would satisfy you? This way you're still treating coroutines as functions instead of tasks, which is what modern asynchronous frameworks set to achieve when they hide all the abstraction layers they rely on

yanghao commented 1 year ago

The point of this whole discussion is that most modern libraries specifically disallow separating task creation from task execution because they focus on a more structured concurrency paradigm: you don't "create tasks", but rather just call async functions and wait for them to complete

Yes, and that might be desirable too, just imagine how could this actually work for a slightly more complicated scenario: how does create_task() know which event loop it belongs to? e.g. creating a task is actually creating a task FOR a particular event-loop (or call it the operating system instance if you wish). As create_task() is a sync call, it can only refer to the default event loop. Normally we don't expose event loop instance for user created tasks, as they should be possible to run on any event loop implementation. If you look at other language like go-lang's goroutines they have the same problem. But of course, if this is not a limitation for your use case, no problem. I am just saying, asyncio's task creation mechanism is not scalable enough for more complicated use cases.

And if the task creation is using await instead, it will trap inside whichever event-loop that runs this task, so that particular event-loop instance knowns I should create a new task because I am executing it, the task will not get created in a default global event loop. Think about any prgram that spawns/starts new threads, you don't tell it if it runs on Linux 5.0 or Linux 4.0, because it issues a system call and asks the underlying OS (whichever it is) to create-and-start the threads. I think this is something I probably did not emphasize enough in previous posts.

And after a second thought, I think my previous argument regarding logical consistency is not valid for asyncio's create task, my apologies. Because if you wait for something to complete, it matters when and where you started waiting/blocking. I should have said is, await create_task() is confusing at least for me, if you try to translate it to English it would read "waiting for create a task", that's literally how it reads, there is no join() implicated... What would be better is if I see an explicity await task.join(), which will make the reasoning of the code much more clear and explicit. When I was talking about the logical consistency, my head was pinned with "await create_task()" is just creating a task ...

The idea is to make async code look like sync code as much as possible.

Yes, that was the whole point of async/await.

If you want to do what you're looking for, maybe reimplementing functools.partial to work with coroutines would satisfy you? This way you're still treating coroutines as functions instead of tasks, which is what modern asynchronous frameworks set to achieve when they hide all the abstraction layers they rely on

I have implemented my own event loop scheduler, and it was inspired a lot by curio.

nocturn9x commented 1 year ago

how does create_task() know which event loop it belongs to?

My understanding is most libraries use a hidden threading.local instance to store one event loop per thread and fetch it as needed. That's how I manage it anyway

yanghao commented 1 year ago

how does create_task() know which event loop it belongs to?

My understanding is most libraries use a hidden threading.local instance to store one event loop per thread and fetch it as needed. That's how I manage it anyway

That's when you are fine with not controlling which event loop runs the code. For my use case, I could create an arbitary number of event loops, and the control is under end-user, rather than pre-defined. e.g. user does not need to modify the library to be able to create new event-loops.

nocturn9x commented 1 year ago

That sounds like asyncio's old way of having loop arguments to many of its utility functions to choose which loop would execute the given task. I guess the priority for general purpose libraries is to abstract the event loop away from the user

yanghao commented 1 year ago

That sounds like asyncio's old way of having loop arguments to many of its utility functions to choose which loop would execute the given task. I guess the priority for general purpose libraries is to abstract the event loop away from the user

passing it explicitly is ugly too. That's why using "await new_task()" is great: you don't have to pass the event loop specifically and user code does not need to know the the event handler at all, but instead user code caused a "trap" into whichever event loop that is trying to execute this user code. This is very beautiful IMO.

nocturn9x commented 1 year ago

Doesn't that also mean that you need to explicitly decide which loop runs user code at some point or another anyway?

yanghao commented 1 year ago

let's say yes. maybe something like this:

loop_0_run(app0)
loop_1_run(app1)
loop_2_run(app0, a, b)

See the point? app0/app1 is not aware of which loop runs it, and each loop is having its own rules of running things and they can still talk to each other. ;-)