PEP 567 (contextvars) idea: really implement the current context
Hi, It seems like many people, including myself, are confused by the lack of concrete current context in the PEP 567 (contextvars). But it isn't difficult to implement the current context (I implemented it, see below). It might have a *minor* impact on performance, but Context mapping API is supposed to only be used for introspection according to Yury. With contextvars.get_context(), it becomes possible to introspect the current context, rather than a copy, and it becomes even more obvious than a context is mutable ;-) vstinner@apu$ ./python
import contextvars ctx=contextvars.get_context() name=contextvars.ContextVar('name', default='victor') print(list(ctx.items())) []
name.set('yury') <Token object at 0x7f4cc71f8ad8> print(list(ctx.items())) [(<ContextVar name='name' default='victor' at 0x7f4cc71f18d8>, 'yury')]
With my changes, the running context remains up to date in Context.run(). Example: --- import contextvars name = contextvars.ContextVar('name', default='victor') def func(): name.set('yury') print(f"context[name]: {context.get(name)}") print(f"name in context: {name in context}") context = contextvars.copy_context() context.run(func) --- Output: --- context[name]: yury name in context: True --- Compare it to the output without my changes: --- context[name]: None name in context: False --- If we have contextvars.get_context(), maybe contextvars.copy_context() can be removed and add a new Context.copy() method instead: new_context = contextvars.get_context().copy() *** I implemented contextvars.get_context() which returns the current context: https://github.com/vstinner/cpython/commit/1e5ee71c15e2b1387c888d6eca2b08ef1... from https://github.com/vstinner/cpython/commits/current_context I added a context thread local storage (TLS): class PyThreadState: context: Context # can be None context_data: _ContextData I modified Context mapping API to use the context variables from the current thread state if it's the current thread state. PyContext_Enter() now not only sets PyThreadState.context_data, but also PyThreadState.context to itself. Pseudo-code for Context.get(): --- class Context(collections.abc.Mapping): def __init__(self): self._data = _ContextData() def _get_data(self): ts : PyThreadState = PyThreadState_Get() if ts.context is self: return ts.context_data else: return self._data def get(self, var): # FIXME: implement default data = self._get_data() return data.get(var) --- And Context.run() is modified to set also the context TLS: --- def run(self, callable, *args, **kwargs): ts : PyThreadState = PyThreadState_Get() saved_context : Optional[Context] = ts.context # NEW saved_data : _ContextData = ts.context_data try: ts.context_data = self._data return callable(*args, **kwargs) finally: self._data = ts.context_data ts.context = saved_context # NEW ts.context_data = saved_data --- Victor
Victor:
I modified Context mapping API to use the context variables from the current thread state if it's the current thread state.
Oops, I mean: "if it's the current context". Nathaniel: """ - BUT it doesn't allow mutation through the MutableMapping interface; instead, the only way to mutate it is by calling Context.run and then ContextVar.set(). Funneling all 'set' operations through a single place makes it easier to do clever caching tricks, and it lets us avoid dealing with operations that we don't want here (like 'del') just because they happen to be in the MutableMapping interface. """ If a context knows that it's the current context, Context.set() can delegate the change to ContextVar.set(), since Context access directly the thread local storage in this case (with my suggested changes), and so the optimization is kept. If the context is not the current context, the cache doesn't have to be invalidated. Moreover, PyContext_Enter() and PyContext_Exit() already increases ts->contextvars_stack_ver which indirectly invalidates cached values. Nathaniel: Would you be ok to implement the MutableMapping API if the optimization is kept? "del context[var]" would raise a TypeError('Context' object doesn't support item deletion) exception, as it does currently. Victor
On Wed, Jan 3, 2018 at 5:42 PM, Victor Stinner <victor.stinner@gmail.com> wrote:
Hi,
It seems like many people, including myself, are confused by the lack of concrete current context in the PEP 567 (contextvars). But it isn't difficult to implement the current context (I implemented it, see below).
The problem with such an API is that it doesn't work (or at the very least creates a lot of complications) in a potential future PEP 550 world, where the "current context" becomes something like a ChainMap-of-Contexts instead of just the last Context that had run() called on it. This isn't a big problem for contextvars.get_context(), which returns a snapshot of the current context -- in a PEP 550 world it would return a snapshot of the current "effective" (flattened) context. Maybe it would help a little to rename get_context() to something like snapshot_context()? -n -- Nathaniel J. Smith -- https://vorpus.org
2018-01-04 3:15 GMT+01:00 Nathaniel Smith <njs@pobox.com>:
The problem with such an API is that it doesn't work (or at the very least creates a lot of complications) in a potential future PEP 550 world, (...)
Hum, it's annoying that many design choices of the PEP 567 are motivated by the hypothetical acceptance of the PEP 550, whereas this PEP is not scheduled for Python 3.7. What if the PEP 550 is rejected because it's too complicated? We might have a nicer PEP 567 API without the big shadow of the PEP 550. Is it possible to run a generator in a context explicitly using PEP 567 Context.run() API? -- I never looked seriously at the PEP 550, because I'm scared by its length and its complexity :-) While I easily see use cases for asyncio web frameworks with the PEP 567, it's harder for me to see the use cases of the PEP 550. I never had to keep a context in a generator. I guess that developers who have to do that learnt how to workaround this Python limitation, maybe passing manually the context to the generator and reapply it at each step? Victor
On Wed, Jan 3, 2018 at 6:45 PM, Victor Stinner <victor.stinner@gmail.com> wrote:
Is it possible to run a generator in a context explicitly using PEP 567 Context.run() API?
Yes. You execute 'ctx = copy_context()' once, and then wrap all .next() calls in ctx.run(). This is what the PEP proposes to do for asyncio.Task -- constructing the task calls copy_context() _step() is always wrapped in a run() call for that context. (This was a good question because it teases out more of the semantics of Context.run().) -- --Guido van Rossum (python.org/~guido)
On 4 January 2018 at 02:15, Nathaniel Smith <njs@pobox.com> wrote:
On Wed, Jan 3, 2018 at 5:42 PM, Victor Stinner <victor.stinner@gmail.com> wrote:
Hi,
It seems like many people, including myself, are confused by the lack of concrete current context in the PEP 567 (contextvars). But it isn't difficult to implement the current context (I implemented it, see below).
The problem with such an API is that it doesn't work (or at the very least creates a lot of complications) in a potential future PEP 550 world, where the "current context" becomes something like a ChainMap-of-Contexts instead of just the last Context that had run() called on it. This isn't a big problem for contextvars.get_context(), which returns a snapshot of the current context -- in a PEP 550 world it would return a snapshot of the current "effective" (flattened) context.
But PEP 567 is specifically a restricted version of PEP 550 that doesn't try to solve the case of generators and async generators - precisely because it was proving impossible to gain consensus on how to handle those cases. We can't expect PEP 567 to satisfy an unstated requirement that "it must be possible to extend it to provide full PEP 550 functionality later". Is there a reason within the stated design goals of PEP 567 why Victor's implementation is incorrect? In PEP 567 the *only* point of the Context is to provide a means of implementing the consideration described in the Introduction: "The notion of "current value" deserves special consideration: different asynchronous tasks that exist and execute concurrently may have different values for the same key". Having said this, I don't really see the need for Victor's re-implementation. The wording of the PEP needs some work, as it mixes implementation details with design in a way that makes it confusing, but unlike Victor I don't see this as unfixable. I'd be perfectly happy with Yury's implementation, but with better documentation that focuses on the use of the feature and separates the implementation aspects out clearly. This seems to be a general problem with async - usage-focused documentation that ignores implementation details is very hard to find, and the experts seem to struggle to keep implementation details "behind the scenes". IMO, we need to do more to help push for user-friendly documentation, which is why I'd rather concentrate on helping Yury document context variables in a way that doesn't expose implementation details, rather than trying to understand and critique those implementation details (people like Victor are far better at doing that than I am :-))
Maybe it would help a little to rename get_context() to something like snapshot_context()?
Maybe? But at the moment, the PEP says "the context is immutable" so it shouldn't make a difference (and in fact get_ makes more sense than snapshot_ as long as the context is immutable). I'd prefer we work on clarifying whether (conceptually - not in terms of implementation!) the context should be described as immutable, and once we understand that I suspect answers to questions like this will be obvious. Paul
On Thu, Jan 4, 2018 at 4:11 AM, Paul Moore <p.f.moore@gmail.com> wrote:
On 4 January 2018 at 02:15, Nathaniel Smith <njs@pobox.com> wrote:
On Wed, Jan 3, 2018 at 5:42 PM, Victor Stinner <victor.stinner@gmail.com> wrote:
It seems like many people, including myself, are confused by the lack of concrete current context in the PEP 567 (contextvars). But it isn't difficult to implement the current context (I implemented it, see below).
[Nathaniel]
The problem with such an API is that it doesn't work (or at the very least creates a lot of complications) in a potential future PEP 550 world, where the "current context" becomes something like a ChainMap-of-Contexts instead of just the last Context that had run() called on it. This isn't a big problem for contextvars.get_context(), which returns a snapshot of the current context -- in a PEP 550 world it would return a snapshot of the current "effective" (flattened) context.
[Paul]
But PEP 567 is specifically a restricted version of PEP 550 that doesn't try to solve the case of generators and async generators - precisely because it was proving impossible to gain consensus on how to handle those cases. We can't expect PEP 567 to satisfy an unstated requirement that "it must be possible to extend it to provide full PEP 550 functionality later". Is there a reason within the stated design goals of PEP 567 why Victor's implementation is incorrect? In PEP 567 the *only* point of the Context is to provide a means of implementing the consideration described in the Introduction: "The notion of "current value" deserves special consideration: different asynchronous tasks that exist and execute concurrently may have different values for the same key".
Having said this, I don't really see the need for Victor's re-implementation. The wording of the PEP needs some work, as it mixes implementation details with design in a way that makes it confusing, but unlike Victor I don't see this as unfixable. I'd be perfectly happy with Yury's implementation, but with better documentation that focuses on the use of the feature and separates the implementation aspects out clearly. This seems to be a general problem with async - usage-focused documentation that ignores implementation details is very hard to find, and the experts seem to struggle to keep implementation details "behind the scenes". IMO, we need to do more to help push for user-friendly documentation, which is why I'd rather concentrate on helping Yury document context variables in a way that doesn't expose implementation details, rather than trying to understand and critique those implementation details (people like Victor are far better at doing that than I am :-))
Maybe it would help a little to rename get_context() to something like snapshot_context()?
Maybe? But at the moment, the PEP says "the context is immutable" so it shouldn't make a difference (and in fact get_ makes more sense than snapshot_ as long as the context is immutable). I'd prefer we work on clarifying whether (conceptually - not in terms of implementation!) the context should be described as immutable, and once we understand that I suspect answers to questions like this will be obvious.
In the other thread I think we've clarified what the (im)mutability status of Context is. I don't think we need Victor's implementation either and I agree with Paul that the PEP just needs a better specification / description of the API it offers. Also let's not tweak the name of the function that retrieves a copy of the current context just yet. The PEP has it as copy_context(). Let's keep that. -- --Guido van Rossum (python.org/~guido)
participants (4)
-
Guido van Rossum -
Nathaniel Smith -
Paul Moore -
Victor Stinner