Re: [Python-Dev] Unique loader per module
On 3 January 2018 at 06:35, Barry Warsaw wrote: > Brett doesn’t like this, for several reasons (quoting): > > 1. redundant API in all cases where the loader is unique to the module > 2. the memory savings of sharing a loader is small > 3. it's implementation complexity/overhead for an optimization case. > > The second solution, and the one Brett prefers, is to reimplement zip > importer to not use a shared loader. This may not be that difficult, if for > example we were to use a delegate loader wrapping a shared loader. > > The bigger problem IMHO is two-fold: > > 1. It would be backward incompatible. If there’s any code out there > expecting a shared loader in zipimport, it would break > 2. More problematic is that we’d have to impose an additional requirement on > loaders - that they always be unique per module, contradicting the advice in > PEP 302 We added module.__spec__.loader_state as part of PEP 451 precisely so shared loaders had a place to store per-module state without have to switch to a unique-loader-per-module model. I think the main reason you're seeing a problem here is because ResourceReader has currently been designed to be implemented directly by loaders, rather than being a subcomponent that you can request *from* a loader. If you instead had an indirection API (that could optionally return self in the case of non-shared loaders), you'd keep the current resource reader method signatures, but the way you'd access the itself would be: resources = module.__spec__.loader.get_resource_reader(module) # resources implements the ResourceReader ABC For actual use, the loader protocol could be hidden behind a helper function: resources = importlib_resources.get_resource_reader(module) For a shared loader, get_resource_reader(module) would return a new *non*-shared resource reader (perhaps caching it in __spec__.loader_state). For a non-shared loader, get_resource_reader(module) would just return self. In both cases, we'd recommend that loaders ensure "self is module.__spec__.loader" as part of their get_resource_reader() implementation. Cheers, Nick. -- Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
On Thu, Jan 4, 2018 at 7:58 PM, Nathaniel Smith wrote: > On Thu, Jan 4, 2018 at 6:09 PM, Guido van Rossum wrote: > > However you've not convinced me that it would be better to make Context > > implement the full MutableMapping interface (with `__delitem__` always > > raising). There are use cases for inspecting Context, e.g. a debugger > that > > wants to print the Context for some or all suspended tasks. But I don't > see > > a use case for mutating a Context object that's not the current context, > and > > when it is the current context, ContextVar.set() is more direct. I also > > don't see use cases for other MutableMapping methods like pop() or > update(). > > (And clear() falls under the same prohibition as __delitem__().) > > I was looking at this again, and I realized there's some confusion. > I've been repeating the thing about not wanting to implement > __delitem__ too, but actually, I think __delitem__ is not the problem > :-). > > The problem is that we don't want to provide ContextVar.unset() -- > that's the operation that adds complication in a PEP 550 world. If you > have a stack of Contexts that ContextVar.get() searches, and set/unset > are only allowed to mutate the top entry in the stack, then the only > way to implement unset() is to do something like > context_stack[-1][self] = _MISSING, so it can hide any entries below > it in the stack. This is extra complication for a feature that it's > not clear anyone cares about. (And if it turns out people do care, we > could add it later.) > Ah yes, that's it. A stack of Contexts could have the same semantics as ChainMap, in which __delitem__ deletes the key from the first mapping if present and otherwise raises KeyError even if it is present in a later mapping. That's enough to implement var.reset(), but not enough to implement arbitrary var.unset(). I'm fine with only having var.reset() and not var.unset(). > Deleting entries from individual Context objects shouldn't create > conceptual problems. OTOH I don't see how it's useful either. I don't > think implementing MutableMapping would actually cause problems, but > it's a bunch of extra code to write/test/maintain without any clear > use cases. > The implementation would have to maintain cache consistency, but that's not necessarily a big deal, and it only needs to be done in a few places. But I agree that the a case hasn't been indicated yet. (I like being able to specify ContextVar's behavior using Context as a MutableMapping, but that's not a real use case.) > This does make me think that I should write up a short PEP for > extending PEP 567 to add context lookup, PEP 550 style: it can start > out in Status: deferred and then we can debate it properly before 3.8, > but at least having the roadmap written down now would make it easier > to catch these details. (And it might also help address Paul's > reasonable complaint about "unstated requirements".) > Anything that will help us kill a 550-pound gorilla sounds good to me. :-) It might indeed be pretty short if we follow the lead of ChainMap (even using a different API than MutableMapping to mutate it). Maybe copy_context() would map to new_child()? Using ChainMap as a model we might even avoid the confusion between Lo[gi]calContext and ExecutionContext which was the nail in PEP 550's coffin. The LC associated with a generator in PEP 550 would be akin to a loose dict which can be pushed on top of a ChainMap using cm = cm.new_child(). (Always taking for granted that instead of an actual dict we'd use some specialized mutable object implementing the Mapping protocol and a custom mutation protocol so it can maintain ContextVar cache consistency.) -- --Guido van Rossum (python.org/~guido) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] 'continue'/'break'/'return' inside 'finally' clause
On 2018-01-04, Guido van Rossum wrote: > We should interview you for the paper we may be writing for HOPL. History of Programming Languages? I did some more digging this afternoon, trying to find source code between versions 1.0.1 and 0.9.1. No luck though. It looks like 0.9.1 might have been the last one you uploaded to alt.sources. Later 0.9.X releases were uploaded to ftp.cwi.nl and wuarchive.wustle.edu. No one seems to have an archive of those. I think all my old PCs have been sent to the scrapyard. I might have some old hard disk images somewhere. Maybe on a writable DVD or CDR. Probably unreadable at this point. I don't know exactly which version of Python I first downloaded. No earlier than the fall of 1992 and maybe 1993 but it could have been pre-1.0. I do recall running a DOS port at some point. Here is the announcement of 0.9.4alpha: http://legacy.python.org/search/hypermail/python-1992/0270.html The Misc/HISTORY file has quite a lot of details. It shows that 'continue' was added in 0.9.2. Back on topic, it looks like allowing 'continue' will be trival once Serhiy's unwind stack PR lands. Just a few lines of code and I think everything works. If Mark implements his alternative "wordcode for finally blocks gets copied" thing, it will make things more complicated but not much more so than handling 'break' and 'return'. So, those three should probably be all allowed or all forbidden. Regards, Neil ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
Guido van Rossum wrote: Well, it's not *immutable* (it shouldn't support hash()), but it doesn't follow the MutableMapping protocol -- it only follows the Mapping protocol. That's why I wrote "mapping" with a small "m". If there's a better term for "collection of associations between keys and values that can be changed somehow" then by all means use it. -- Greg ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
On Thu, Jan 4, 2018 at 6:09 PM, Guido van Rossum wrote: > However you've not convinced me that it would be better to make Context > implement the full MutableMapping interface (with `__delitem__` always > raising). There are use cases for inspecting Context, e.g. a debugger that > wants to print the Context for some or all suspended tasks. But I don't see > a use case for mutating a Context object that's not the current context, and > when it is the current context, ContextVar.set() is more direct. I also > don't see use cases for other MutableMapping methods like pop() or update(). > (And clear() falls under the same prohibition as __delitem__().) I was looking at this again, and I realized there's some confusion. I've been repeating the thing about not wanting to implement __delitem__ too, but actually, I think __delitem__ is not the problem :-). The problem is that we don't want to provide ContextVar.unset() -- that's the operation that adds complication in a PEP 550 world. If you have a stack of Contexts that ContextVar.get() searches, and set/unset are only allowed to mutate the top entry in the stack, then the only way to implement unset() is to do something like context_stack[-1][self] = _MISSING, so it can hide any entries below it in the stack. This is extra complication for a feature that it's not clear anyone cares about. (And if it turns out people do care, we could add it later.) Deleting entries from individual Context objects shouldn't create conceptual problems. OTOH I don't see how it's useful either. I don't think implementing MutableMapping would actually cause problems, but it's a bunch of extra code to write/test/maintain without any clear use cases. This does make me think that I should write up a short PEP for extending PEP 567 to add context lookup, PEP 550 style: it can start out in Status: deferred and then we can debate it properly before 3.8, but at least having the roadmap written down now would make it easier to catch these details. (And it might also help address Paul's reasonable complaint about "unstated requirements".) -n -- Nathaniel J. Smith -- https://vorpus.org ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
On Tue, Jan 2, 2018 at 4:30 PM, Victor Stinner wrote: > Hum, it seems like the specification (API) part of the PEP is polluted by > its implementation. The PEP just require a few minor changes to better > describe the behaviour/API instead of insisting on the read only internal > thing which is specific to the proposed implementation which is just one > arbitrary implementation (designed for best performances). > > IMHO the PEP shouldn't state that a context is read only. From my point of > view, it's mutable and it's the mapping holding variable values. There is a > current context which holds the current values. Context.run() switches > temporarily the current context with another context. The fact that there > is no concrete context instance by default doesn't really matter in term of > API. > You've convinced me that Context is neither immutable nor read-only, and the PEP should admit this. Its *Mapping* interface doesn't allow mutations but its run() method does. E.g. here's a function that mutates a Context, effectively doing vtx[var] = value: def mutate_context(ctx, var, value): ctx.run(lambda: var.set(value)) However you've not convinced me that it would be better to make Context implement the full MutableMapping interface (with `__delitem__` always raising). There are use cases for inspecting Context, e.g. a debugger that wants to print the Context for some or all suspended tasks. But I don't see a use case for mutating a Context object that's not the current context, and when it is the current context, ContextVar.set() is more direct. I also don't see use cases for other MutableMapping methods like pop() or update(). (And clear() falls under the same prohibition as __delitem__().) -- --Guido van Rossum (python.org/~guido) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 (contextvars) idea: really implement the current context
On Wed, Jan 3, 2018 at 6:45 PM, Victor Stinner 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) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 (contextvars) idea: really implement the current context
On Thu, Jan 4, 2018 at 4:11 AM, Paul Moore wrote: > On 4 January 2018 at 02:15, Nathaniel Smith wrote: > > On Wed, Jan 3, 2018 at 5:42 PM, Victor Stinner > 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) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] 'continue'/'break'/'return' inside 'finally' clause
We should interview you for the paper we may be writing for HOPL. On Wed, Jan 3, 2018 at 6:05 PM, Neil Schemenauer wrote: > On 2018-01-03, Guido van Rossum wrote: > > I'm sorry, I don't think more research can convince me either way. > > I want all three of return/break/continue to work inside finally > > clauses, despite there being few use cases. > > That's fine. The history of 'continue' inside 'finally' is > interesting. The restriction dates back to at least when Jeremy > committed the AST-based compiler (I have fond memories of hacking on > it with Armin Rigo and others at a Python core sprint). Going > further back, I looked at 1.5.2 and there is the comment in > compile.c: > > TO DO: > ... > XXX Allow 'continue' inside try-finally > > So if we allow 'continue' we will be knocking off a nearly 20 year > old todo item. ;-) > > For giggles, I unpacked a Python 0.9.1 tarball. The source code is > all under 'src' in that version. There doesn't seem to be a > restriction on 'continue' but only because the grammar doesn't > include it! Without doing more research, I think the restriction > could be as old as the 'continue' keyword. > > BTW, the bytecode structure for try/except shown in the compile.c > comments is very simlar to what is currently generated. It is quite > remarkable how well your initial design and implementation have stood > the test of time. Thank you for making it open source. > > Regards, > > Neil > -- --Guido van Rossum (python.org/~guido) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
Your suggestions sound reasonable, but we are now running into a logistical problem -- I don't want to decide this unilaterally but Yury is on vacation until Jan 15. That gives us at most 2 weeks for approval of the PEP and review + commit of the implementation ( https://github.com/python/cpython/pull/5027) before the 3.7.0 feature freeze / beta (Jan 29). Your approach to defaults *may* have another advantage: perhaps we could get rid of reset() and Token. Code to set and restore a context var could just obtain the old value with get() and later restore it with set(). OTOH this would not be entirely transparent, since it would permanently add the var to the keys of the Context if the var was previously not set. [Later] Oh, drat! Writing that paragraph made me realize there was a bug in my own reasoning that led to this question about variables that have no value: they aren't present in the keys of _ContextData, so my concern about keys and values being inconsistent was unfounded. So the only reason to change the situation with defaults would be that it's more convenient. Though I personally kind of like that you can cause var.get() to raise LookupError if the value is not set -- just like all other variables. [Even later] Re: your other suggestion, why couldn't the threadstate contain just the Context? It seems one would just write tstate->current_context->_data everywhere instead of tstate->current_context_data. On Thu, Jan 4, 2018 at 4:18 PM, Nathaniel Smith wrote: > On Thu, Jan 4, 2018 at 8:30 AM, Guido van Rossum wrote: > > On Wed, Jan 3, 2018 at 6:35 PM, Nathaniel Smith wrote: > >> - Context is a mutable object representing a mapping > >> - 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. > >> - OTOH we do implement the (read-only) Mapping interface because > >> there's no harm in it and it's probably useful for debuggers. > > > > > > I think that in essence what Victor saw is a cache consistency issue. > > Yeah, that's a good way to think about it. > > > If you > > look at the implementation section in the PEP, the ContextVar.set() > > operation mutates _ContextData, which is a private (truly) immutable data > > structure that stands in for the HAMT, and the threadstate contains one > of > > these (not a Context). When you call copy_context() you get a fresh > Context > > that wraps the current _ContextData. Because the latter is effectively > > immutable this is a true clone. ctx.run() manipulates the threadstate to > > make the current _ContextData the one from ctx, then calls the function. > If > > the function calls var.set(), this will create a new _ContextData that is > > stored in the threadstate, but it doesn't update the ctx. This is where > the > > current state and ctx go out of sync. Once the function returns or > raises, > > run() takes the _ContextData from the threadstate and stuffs it into ctx, > > resolving the inconsistency. (It then also restores the previous > > _ContextData that it had saved before any of this started.) > > > > So all in all Context is mutable but the only time it is mutated is when > > run() returns. > > > > I think Yury's POV is that you rarely if ever want to introspect a > Context > > object that's not freshly obtained from copy_context(). I'm not sure if > > that's really true; it means that introspecting the context stored in an > > asyncio.Task may give incorrect results if it's the currently running > task. > > > > Should we declare it a bug? The fix would be complex given the current > > implementation (either the PEP's pseudo-code or Yury's actual HAMT-based > > implementation). I think it would involve keeping track of the current > > Context in the threadstate rather than just the _ContextData, and > updating > > the Context object on each var.set() call. And this is something that > Yury > > wants to avoid, so that he can do more caching for var.get() (IIUC). > > I think the fix is a little bit cumbersome, but straightforward, and > actually *simplifies* caching. If we track both the _ContextData and > the Context in the threadstate, then set() becomes something like: > > def set(self, value): > # These two lines are like the current implementation > new_context_data = > tstate->current_context_data->hamt_clone_with_new_item(key=self, > value=value) > tstate->current_context_data = new_context_data > # Update the Context to have the new _ContextData > tstate->current_context->data = new_context_data > # New caching: instead of tracking integer ids, we just need to > track the Context object > # This relies on the assumption that self->last_value
Re: [Python-Dev] PEP 567 v2
On Thu, Jan 4, 2018 at 4:56 PM, Greg Ewing wrote: > Guido van Rossum wrote: > >> It was get_context() in an earlier version of PEP 567. We changed it to >> copy_context() believing that that would clarify that you get a clone that >> is unaffected by subsequent ContextVar.set() operations (which affect the >> *current* context rather than the copy you just got). >> > > In that case it seems clear to me that "the context" is > conceptually a mutable mapping. The fact that it happens > to be built out of immutable components is an implementation > detail that user-level docs should not be talking about. > Well, it's not *immutable* (it shouldn't support hash()), but it doesn't follow the MutableMapping protocol -- it only follows the Mapping protocol. Note that the latter carefully doesn't call itself ImmutableMapping. Context is a mutable object that implements the Mapping protocol. The only way to mutate a Context is to use var.set() when that Context is the current context. (Modulo the caching bug discussed in the subthread with Nathaniel.) -- --Guido van Rossum (python.org/~guido) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
On Thu, Jan 4, 2018 at 9:27 AM, Paul Moore wrote: > On 4 January 2018 at 15:56, Guido van Rossum wrote: > > It was get_context() in an earlier version of PEP 567. We changed it to > > copy_context() believing that that would clarify that you get a clone > that > > is unaffected by subsequent ContextVar.set() operations (which affect the > > *current* context rather than the copy you just got). > > Ah thanks. In which case, simply changing the emphasis to avoid the > implication that Context objects are immutable (while that may be true > in a technical/implementation sense, it's not really true in a design > sense if ContextVar.set modifies the value of a variable in a context) > is probably sufficient. Do you have a specific proposal for a wording change? PEP 567 describes Context as "a read-only mapping, implemented using an immutable dictionary." This sounds all right to me -- "read-only" is weaker than "immutable". Maybe the implementation should not be mentioned here? (The crux here is that a given Context acts as a variable referencing an immutable dict -- but it may reference different immutable dicts at different times.) -- --Guido van Rossum (python.org/~guido) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
Guido van Rossum wrote: It was get_context() in an earlier version of PEP 567. We changed it to copy_context() believing that that would clarify that you get a clone that is unaffected by subsequent ContextVar.set() operations (which affect the *current* context rather than the copy you just got). In that case it seems clear to me that "the context" is conceptually a mutable mapping. The fact that it happens to be built out of immutable components is an implementation detail that user-level docs should not be talking about. -- Greg ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
On Thu, Jan 4, 2018 at 8:30 AM, Guido van Rossum wrote: > On Wed, Jan 3, 2018 at 6:35 PM, Nathaniel Smith wrote: >> - Context is a mutable object representing a mapping >> - 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. >> - OTOH we do implement the (read-only) Mapping interface because >> there's no harm in it and it's probably useful for debuggers. > > > I think that in essence what Victor saw is a cache consistency issue. Yeah, that's a good way to think about it. > If you > look at the implementation section in the PEP, the ContextVar.set() > operation mutates _ContextData, which is a private (truly) immutable data > structure that stands in for the HAMT, and the threadstate contains one of > these (not a Context). When you call copy_context() you get a fresh Context > that wraps the current _ContextData. Because the latter is effectively > immutable this is a true clone. ctx.run() manipulates the threadstate to > make the current _ContextData the one from ctx, then calls the function. If > the function calls var.set(), this will create a new _ContextData that is > stored in the threadstate, but it doesn't update the ctx. This is where the > current state and ctx go out of sync. Once the function returns or raises, > run() takes the _ContextData from the threadstate and stuffs it into ctx, > resolving the inconsistency. (It then also restores the previous > _ContextData that it had saved before any of this started.) > > So all in all Context is mutable but the only time it is mutated is when > run() returns. > > I think Yury's POV is that you rarely if ever want to introspect a Context > object that's not freshly obtained from copy_context(). I'm not sure if > that's really true; it means that introspecting the context stored in an > asyncio.Task may give incorrect results if it's the currently running task. > > Should we declare it a bug? The fix would be complex given the current > implementation (either the PEP's pseudo-code or Yury's actual HAMT-based > implementation). I think it would involve keeping track of the current > Context in the threadstate rather than just the _ContextData, and updating > the Context object on each var.set() call. And this is something that Yury > wants to avoid, so that he can do more caching for var.get() (IIUC). I think the fix is a little bit cumbersome, but straightforward, and actually *simplifies* caching. If we track both the _ContextData and the Context in the threadstate, then set() becomes something like: def set(self, value): # These two lines are like the current implementation new_context_data = tstate->current_context_data->hamt_clone_with_new_item(key=self, value=value) tstate->current_context_data = new_context_data # Update the Context to have the new _ContextData tstate->current_context->data = new_context_data # New caching: instead of tracking integer ids, we just need to track the Context object # This relies on the assumption that self->last_value is updated every time any Context is mutated self->last_value = value self->last_context = tstate->current_context And then the caching in get() becomes: def get(self): if tstate->current_context != self->last_context: # Update cache self->last_value = tstate->current_context_data->hamt_lookup(self) self->last_context = tstate->current_context return self->last_value (I think the current cache invalidation logic is necessary for a PEP 550 implementation, but until/unless we implement that we can get away with something simpler.) So I'd say yeah, let's declare it a bug. If it turns out that I'm wrong and there's some reason this is really difficult, then we could consider making introspection on a currently-in-use Context raise an error, instead of returning stale data. This should be pretty easy, since Contexts already track whether they're currently in use (so they can raise an error if you try to use the same Context in two threads simultaneously). > We could also add extra words to the PEP's spec for run() explaining this > temporary inconsistency. > > I think changing the introspection method from Mapping to something custom > won't fix the basic issue (which is that there's a difference between the > Context and the _ContextData, and ContextVar actually only manipulates the > latter, always accessing it via the threadstate). > > However there's another problem with the Mapping interface, which is: what > should it do with variables that are not set and have no default value? > Should they be considered to have a value equal to _NO_DEFAULT or > Token.MISSING?
Re: [Python-Dev] PEP 567 v2
On Wed, Jan 3, 2018 at 6:35 PM, Nathaniel Smith wrote: > On Wed, Jan 3, 2018 at 3:44 PM, Victor Stinner > wrote: > > Ok, I finally got access to a computer and I was able to test the PEP > > 567 implementation: see my code snippet below. > > > > The behaviour is more tricky than what I expected. While running > > context.run(), the context object is out of sync of the "current > > context". It's only synchronized again at run() exit. So > > ContextVar.set() doesn't immediately modifies the "current context" > > object (set by Context.run()). > > To me this sounds like a oversight (= bug), not intended behavior. At > the conceptual level, I think what we want is: > > - Context is a mutable object representing a mapping > - 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. > - OTOH we do implement the (read-only) Mapping interface because > there's no harm in it and it's probably useful for debuggers. > I think that in essence what Victor saw is a cache consistency issue. If you look at the implementation section in the PEP, the ContextVar.set() operation mutates _ContextData, which is a private (truly) immutable data structure that stands in for the HAMT, and the threadstate contains one of these (not a Context). When you call copy_context() you get a fresh Context that wraps the current _ContextData. Because the latter is effectively immutable this is a true clone. ctx.run() manipulates the threadstate to make the current _ContextData the one from ctx, then calls the function. If the function calls var.set(), this will create a new _ContextData that is stored in the threadstate, but it doesn't update the ctx. This is where the current state and ctx go out of sync. Once the function returns or raises, run() takes the _ContextData from the threadstate and stuffs it into ctx, resolving the inconsistency. (It then also restores the previous _ContextData that it had saved before any of this started.) So all in all Context is mutable but the only time it is mutated is when run() returns. I think Yury's POV is that you rarely if ever want to introspect a Context object that's not freshly obtained from copy_context(). I'm not sure if that's really true; it means that introspecting the context stored in an asyncio.Task may give incorrect results if it's the currently running task. Should we declare it a bug? The fix would be complex given the current implementation (either the PEP's pseudo-code or Yury's actual HAMT-based implementation). I think it would involve keeping track of the current Context in the threadstate rather than just the _ContextData, and updating the Context object on each var.set() call. And this is something that Yury wants to avoid, so that he can do more caching for var.get() (IIUC). We could also add extra words to the PEP's spec for run() explaining this temporary inconsistency. I think changing the introspection method from Mapping to something custom won't fix the basic issue (which is that there's a difference between the Context and the _ContextData, and ContextVar actually only manipulates the latter, always accessing it via the threadstate). However there's another problem with the Mapping interface, which is: what should it do with variables that are not set and have no default value? Should they be considered to have a value equal to _NO_DEFAULT or Token.MISSING? Or should they be left out of the keys altogether? The PEP hand-waves on this issue (we didn't think of missing values when we made the design). Should it be possible to introspect a Context that's not the current context? > (Note that I didn't say anything about HAMTs here, because that's > orthogonal implementation detail. It would make perfect sense to have > Context be an opaque wrapper around a regular dict; it would just give > different performance trade-offs.) > Agreed, that's how the PEP pseudo-code does it. -- --Guido van Rossum (python.org/~guido) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
On 4 January 2018 at 15:56, Guido van Rossum wrote: > It was get_context() in an earlier version of PEP 567. We changed it to > copy_context() believing that that would clarify that you get a clone that > is unaffected by subsequent ContextVar.set() operations (which affect the > *current* context rather than the copy you just got). Ah thanks. In which case, simply changing the emphasis to avoid the implication that Context objects are immutable (while that may be true in a technical/implementation sense, it's not really true in a design sense if ContextVar.set modifies the value of a variable in a context) is probably sufficient. > [The discussion is fragmentary because Yury is on vacation until the 15th > and I am scrambling for time myself. But your long post is saved, and not > forgotten.] OK. I knew you were short on time, but hadn't realised Yury was on vacation too. Sorry if I sounded like I was nagging. Paul ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
On Thu, Jan 4, 2018 at 3:25 AM, Paul Moore wrote: > On 4 January 2018 at 00:17, Greg Ewing > wrote: > > Guido van Rossum wrote: > >> > >> contextvars.copy_context().run(func, ) > > > > If contexts are immutable, why is there something > > called copy_context? > > Agreed. This was something that bothered me, too. I mentioned it in my > review, but that seemed to get lost in the other comments in this > thread... > > I get the impression that the logic is that the context is immutable, > but the ContextVars that it contains aren't, and the copy is deep (at > least 1 level deep) so you copy then change the value of a ContextVar. > But rereading that sentence, it sounds confused even to me, so it's > either not right or the implementation falls foul of "If the > implementation is hard to explain, it's a bad idea." :-) It was get_context() in an earlier version of PEP 567. We changed it to copy_context() believing that that would clarify that you get a clone that is unaffected by subsequent ContextVar.set() operations (which affect the *current* context rather than the copy you just got). [The discussion is fragmentary because Yury is on vacation until the 15th and I am scrambling for time myself. But your long post is saved, and not forgotten.] -- --Guido van Rossum (python.org/~guido) ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 (contextvars) idea: really implement the current context
On 4 January 2018 at 02:15, Nathaniel Smith wrote: > On Wed, Jan 3, 2018 at 5:42 PM, Victor Stinner > 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 ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] PEP 567 v2
On 4 January 2018 at 00:17, Greg Ewing wrote: > Guido van Rossum wrote: >> >> contextvars.copy_context().run(func, ) > > > If contexts are immutable, why is there something > called copy_context? Agreed. This was something that bothered me, too. I mentioned it in my review, but that seemed to get lost in the other comments in this thread... I get the impression that the logic is that the context is immutable, but the ContextVars that it contains aren't, and the copy is deep (at least 1 level deep) so you copy then change the value of a ContextVar. But rereading that sentence, it sounds confused even to me, so it's either not right or the implementation falls foul of "If the implementation is hard to explain, it's a bad idea." :-) Paul ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
