[Python-Dev] PEP 550 v4 (original) (raw)

Nathaniel Smith njs at pobox.com
Sat Aug 26 19:13:24 EDT 2017

On Sat, Aug 26, 2017 at 7:58 AM, Elvis Pranskevichus <elprans at gmail.com> wrote:

On Saturday, August 26, 2017 2:34:29 AM EDT Nathaniel Smith wrote:

On Fri, Aug 25, 2017 at 3:32 PM, Yury Selivanov <yselivanov.ml at gmail.com> wrote: > Coroutines and Asynchronous Tasks > --------------------------------- > > In coroutines, like in generators, context variable changes are > local> > and are not visible to the caller:: > import asyncio > > var = newcontextvar() > > async def sub(): > assert var.lookup() == 'main' > var.set('sub') > assert var.lookup() == 'sub' > > async def main(): > var.set('main') > await sub() > assert var.lookup() == 'main' > > loop = asyncio.geteventloop() > loop.rununtilcomplete(main())

I think this change is a bad idea. I think that generally, an async call like 'await asyncsub()' should have the equivalent semantics to a synchronous call like 'syncsub()', except for the part where the former is able to contain yields. Giving every coroutine an LC breaks that equivalence. It also makes it so in async code, you can't necessarily refactor by moving code in and out of subroutines. Like, if we inline 'sub' into 'main', that shouldn't change the semantics, but... If we could easily, we'd given each normal function its own logical context as well.

I mean... you could do that. It'd be easy to do technically, right? But it would make the PEP useless, because then projects like decimal and numpy couldn't adopt it without breaking backcompat, meaning they couldn't adopt it at all.

The backcompat argument isn't there in the same way for async code, because it's new and these functions have generally been broken there anyway. But it's still kinda there in spirit: there's a huge amount of collective knowledge about how (synchronous) Python code works, and IMO async code should match that whenever possible.

What we are talking about here is variable scope leaking up the call stack. I think this is a bad pattern. For decimal context-like uses of the EC you should always use a context manager. For uses like Web request locals, you always have a top function that sets the context vars.

It's perfectly reasonable to have a script where you call decimal.setcontext or np.seterr somewhere at the top to set the defaults for the rest of the script. Yeah, maybe it'd be a bit cleaner to use a 'with' block wrapped around main(), and certainly in a complex app you want to stick to that, but Python isn't just used for complex apps :-). I foresee confused users trying to figure out why np.seterr suddenly stopped working when they ported their app to use async.

This also seems like it makes some cases much trickier. Like, say you have an async context manager that wants to manipulate a context local. If you write 'async def aenter', you just lost -- it'll be isolated. I think you have to write some awkward thing like:

def __aenter__(self):
    coro = self._real_aenter()
    coro.__logical_context__ = None
    return coro

It would be really nice if libraries like urllib3/requests supported async as an option, but it's difficult because they can't drop support for synchronous operation and python 2, and we want to keep a single codebase. One option I've been exploring is to write them in "synchronous style" but with async/await keywords added, and then generating a py2-compatible version with a script that strips out async/await etc. (Like a really simple 3to2 that just works at the token level.) One transformation you'd want to apply is replacing aenter -> enter, but this gets much more difficult if we have to worry about elaborate transformations like the above...

If I have an async generator, and I set its logical_context to None, then do I also have to set this attribute on every coroutine returned from calling anext/asend/athrow/aclose?

I think I see the motivation: you want to make await sub() and await ensurefuture(sub()) have the same semantics, right? And the latter has to create a Task What we want is for await sub() to be equivalent to await asyncio.waitfor(sub()) and to await asyncio.gather(sub()).

I don't feel like there's any need to make gather() have exactly the same semantics as a regular call -- it's pretty clearly a task-spawning primitive that runs all of the given coroutines concurrently, so it makes sense that it would have task-spawning semantics rather than call semantics.

wait_for is a more unfortunate case; there's really no reason for it to create a Task at all, except that asyncio made the decision to couple cancellation and Tasks, so if you want one then you're stuck with the other.

Yury's made some comments about stealing Trio's cancellation system and adding it to asyncio -- I don't know how serious he was. If he did then it would let you use timeouts without creating a new Task, and this problem would go away. OTOH if you stick with pushing a new LC on every coroutine call, then that makes Trio's cancellation system way slower, because it has to walk the whole stack of LCs on every yield to register/unregister each cancel scope. PEP 550v4 makes that stack much deeper, plus breaks the optimization I was planning to use to let us mostly skip this entirely. (To be clear, this isn't the main reason I think these semantics are a bad idea -- the main reason is that I think async and sync code should have the same semantics. But it definitely doesn't help that it creates obstacles to improving asyncio/improving on asyncio.)

Imagine we allow context var changes to leak out of async def. It's easy to write code that relies on this:

async def init(): var.set('foo') async def main(): await init() assert var.lookup() == 'foo' If we change await init() to await asyncio.waitfor(init()), the code will break (and in real world, possibly very subtly).

But instead you're making it so that it will break if the user adds/removes async/await keywords:

def init(): var.set('foo')

def main(): init()

It also adds non-trivial overhead, because now lookup() is O(depth of async callstack), instead of O(depth of (async) generator nesting), which is generally much smaller. You would hit cache in lookup() most of the time.

You've just reduced the cache hit rate too, because the cache gets invalidated on every push/pop. Presumably you'd optimize this to skip invalidating if the LC that gets pushed/popped is empty, so this isn't as catastrophic as it might initially look, but you still have to invalidate all the cached variables every time any variable gets touched and then you return from a function. Which might happen quite a bit if, for example, using timeouts involves touching the LC :-).

-n

-- Nathaniel J. Smith -- https://vorpus.org

More information about the Python-Dev mailing list