[Python-Dev] PEP 550 v4

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 = new_context_var()
>> >
>> >     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.get_event_loop()
>> >     loop.run_until_complete(main())
>>
>> I think this change is a bad idea. I think that generally, an async
>> call like 'await async_sub()' should have the equivalent semantics
>> to a synchronous call like 'sync_sub()', 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 ensure_future(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.wait_for(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.wait_for(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