
On Sat, 30 Jan 2021, 10:18 pm Mark Shannon, <mark@hotpy.org> wrote:
Hi Nick,
On Sat, 30 Jan 2021 at 10:30, Nick Coghlan <ncoghlan@gmail.com> wrote:
On Sat, 30 Jan 2021, 12:13 am Mark Shannon, <mark@hotpy.org> wrote:
With a direct proxy coherency is not an issue.
For things in the frame, it *is* a direct proxy - reads pull from the
On 30/01/2021 4:44 am, Nick Coghlan wrote: frame object, and writes go to both the frame object and the mapping: https://github.com/python/cpython/pull/3640/files#diff-7b8cef249e5cca077d30d...
Reviewing the code again, I'd misremembered how the draft implementation works - right now, reads are relying on the snapshot being up to date.
Given the resulting C API compatibility break and the loss of code
sharing, I don't think it's a net win to drop the extra storage.
I'm not sure what you mean by "loss of code sharing", but I don't see how a mapping that is both a cache and a proxy can be easier to implement than a mapping that is just a proxy.
The read side is currently completely handled by the existing MappingProxyType, so the frame locals proxy subclass only implements the extra MutableMapping bits. "reads are relying on the snapshot being up to date", doesn't that just
mean that reads can be wrong?
There is no way to keep a snapshot reliably up to date in the presence of threads.
We don't support running the same *frame* from multiple threads, so only one thread can be updating the frame directly, and the proxy writes to both the frame and the snapshot. But avoiding the need to update the entire snapshot to read specific variables from the frame would definitely be a compelling technical benefit regardless.
For now, I have a PR adding this as an open question: https://github.com/python/peps/pull/1787/files
Given the performance benefit of being able to more reasonably drop the implicit call to `PyFrame_LocalsToFast()`, I'm mostly convinced that switching reads to pull from the frame if possible is the right thing to do, even if it reduces the amount of code that can be inherited without modification from MappingProxyType.
The API compatibility concerns would mean the extra mapping store still needed to stick around, but it would only be used for: > * providing backwards compatibility for the `PyEval_GetLocals()` and C-level `f_locals` interfaces * reading and writing names that don't have entries in the `fast_refs`
mapping
* writing shadow updates for names in the `fast_refs` mapping
Given that f_locals is broken, why is keeping compatibility for this obscure, and probably unused case worthwhile?
f_locals *isn't* broken a lot of the time. The PEP aims to keep that code working unchanged rather than forcing people to modify their code to handle problems they may not have.
The break in compatibility with locals() seems much more intrusive, yet you are OK with that (as am I).
PyEval_GetLocals() is part of the stable ABI and returns a borrowed reference. That means there are a lot of implementation restrictions around keeping that API working. A follow-up PEP could propose deprecating and removing the API as intrinsically broken, but I don't want to go that far in PEP 558. By contrast, for the Python level API, Nathaniel was able to make a compelling case that most locals() usage would at worst suffer a performance degradation with the semantic change, and at least some use cases would see latent defects transparently fixed. Cheers, Nick.
Cheers, Mark.