[Python-Dev] [python/peps] PEP 567 review and copyedits (#503)

[Yury Selivanov]

On Tue, Dec 12, 2017 at 9:07 PM, Chris Angelico <rosuav at gmail.com> wrote:
> Redirecting comments from the PR to the ML. Everything that was
> tightly bound to the PR has been dropped.
>
> On Wed, Dec 13, 2017 at 12:15 PM, Yury Selivanov
> <notifications at github.com> wrote:
>> Most of your questions should be asked on python-dev. I'll answer them here, but if you have any follow-ups, please raise the on the ml.
>>
>>> What happens if you set, set again, then reset from the first one's token?
>>
>> The context will be reset to the state it was in before the first set, w.r.t. that variable's value.
>>
>>> A peek at the implementation shows that it simply resets the value, so aside from having magic that allows it to represent "no value", the token buys nothing that you couldn't get by simply returning the old value - which is a valuable API to have. Am I reading this correctly?
>>
>> "no value" is the main feature of Token, it what makes the PEP future compatible with PEP 550.
>
> A lot of APIs are built to return the old value, not wrapped in any
> sort of opaque token. If the "no value" magic were to be exposed
> (contextvars.NO_VALUE as a specific sentinel value), this would allow
> deliberate use of "previous value" as an actual part of the API. Does
> futureproofing require that the token be opaque?

To get the previous value just use 'ContextVar.get()' before you call
'ContextVar.set()'.

I don't see a lot of value in further enhancing "Token".
Future-proofing requires us to have no ContextVar.delete() method, and
that's why we have the set/reset API.


>
>>> Implementation, _ContextData class - I don't often see "self.__mapping" in PEPs unless the name mangling is actually needed. Is it used here? Would "self._mapping" be as effective?
>>
>> __ is used to highlight the fact that all those attributes are private and inaccessible for Python code.
>
> Just to clarify, then: this is an artifact of the Python reference
> implementation being unable to perfectly represent the behaviour of C
> code, but had you been writing this for actual implementation in the
> stdlib, you'd have used a single underscore?

I'd still use the dunder prefix, there's nothing wrong with it IMO.

>
>>> The HAMT for Context boasts O(log N) 'set' operations. What is N? Number of variables used? Number of times they get set? Number of times set w/o being reset?
>>
>> Number of ContextVars set in the context.
>
> Thanks. Might be worth mentioning that;

Yeah, I'll update the PEP.

> with the
> semantically-equivalent Python code, it's obvious that the cost of the
> copy scales with the number of unique variables, but without knowing
> what the actual HAMT does, it's not so obvious that that's true there
> too. The net result is that repeatedly setting the same variable
> doesn't increase the cost - right?

Yes!

I had to balance the implementation around the following constraints:

1. get_context() must be fast, as it will be used in
asyncio.call_soon() all the time.

2. ContextVar.get() must be fast, as modules like numpy and decimal
won't use it otherwise.

3. ContextVar.set() must not be slow, or become slower and slower as
we have more and more vars on the context.

Having an immutable dict (as opposed to using 'dict.copy()') allows us
to have a super fast 'get_context()'.  We could move 'dict.copy()' to
'ContextVar.set()', but then it would make it an O(N) operation, which
isn't acceptable either.  HAMT is a way to implement an immutable
mapping with a fast O(log N) 'set()' operation.

> The PEP looks pretty good to me.

Thank you, Chris.

Yury