[Python-Dev] PEP 567 -- Context Variables

[Yury Selivanov]

On Tue, Dec 12, 2017 at 10:36 PM, Guido van Rossum <guido at python.org> wrote:
> Some more feedback:
>
>> This proposal builds directly upon concepts originally introduced
>> in :pep:`550`.
>
> The phrase "builds upon" typically implies that the other resource must be
> read and understood first. I don't think that we should require PEP 550 for
> understanding of PEP 567. Maybe "This proposal is a simplified version of
> :pep:`550`." ?

I agree, "simplified version" is better.

>
>> The notion of "current value" deserves special consideration:
>> different asynchronous tasks that exist and execute concurrently
>> may have different values.  This idea is well-known from thread-local
>> storage but in this case the locality of the value is not always
>> necessarily to a thread.  Instead, there is the notion of the
>> "current ``Context``" which is stored in thread-local storage, and
>> is accessed via ``contextvars.get_context()`` function.
>> Manipulation of the current ``Context`` is the responsibility of the
>> task framework, e.g. asyncio.
>
> This begs two (related) questions:
> - If it's stored in TLS, why isn't it equivalent to TLS?
> - If it's read-only (as mentioned in the next paragraph) how can the
> framework modify it?
>
> I realize the answers are clear, but at this point in the exposition you
> haven't given the reader enough information to answer them, so this
> paragraph may confuse readers.

I'll think how to rephrase it.

>
>> Specification
>> =============
>> [points 1, 2, 3]
>
> Shouldn't this also list Token? (It must be a class defined here so users
> can declare the type of variables/arguments in their code representing these
> tokens.)
>
>> The ``ContextVar`` class has the following constructor signature:
>> ``ContextVar(name, *, default=no_default)``.
>
> I think a word or two about the provenance of `no_default` would be good. (I
> think it's an internal singleton right?) Ditto for NO_DEFAULT in the C
> implementation sketch.

Fixed.

>
>>     class Task:
>>         def __init__(self, coro):
>
> Do we need a keyword arg 'context=None' here too? (I'm not sure what would
> be the use case, but somehow it stands out in comparison to call_later()
> etc.)

call_later() is low-level and it needs the 'context' argument as Task
and Future use it in their implementation.

It would be easy to add 'context' parameter to Task and
loop.create_task(), but I don't know about any concrete use-case for
that just yet.

>
>> CPython C API
>> -------------
>> TBD
>
> Yeah, what about it? :-)

I've added it: https://github.com/python/peps/pull/508/files

I didn't want to get into too much detail about the C API until I have
a working PR. Although I feel that the one I describe in the PEP now
is very close to what we'll have.

>
>> The internal immutable dictionary for ``Context`` is implemented
>> using Hash Array Mapped Tries (HAMT).  They allow for O(log N) ``set``
>> operation, and for O(1) ``get_context()`` function.  [...]
>
> I wonder if we can keep the HAMT out of the discussion at this point. I have
> nothing against it, but given that you already say you're leaving out
> optimizations and nothing in the pseudo code given here depends on them I
> wonder if they shouldn't be mentioned later. (Also the appendix with the
> perf analysis is the one thing that I think we can safely leave out, just
> reference PEP 550 for this.)

I've added a new section "Implementation Notes" that mentions HAMT and
ContextVar.get() cache.  Both refer to PEP 550's lengthy explanations.

>
>> class _ContextData
>
> Since this isn't a real class anyway I think the __mapping attribute might
> as well be named _mapping. Ditto for other __variables later.

Done.

Yury