On Sat, 30 Jan 2021, 10:18 pm Mark Shannon, <m...@hotpy.org> wrote:

> Hi Nick,
>
> On 30/01/2021 4:44 am, Nick Coghlan wrote:
> > On Sat, 30 Jan 2021 at 10:30, Nick Coghlan <ncogh...@gmail.com> wrote:
> >> On Sat, 30 Jan 2021, 12:13 am Mark Shannon, <m...@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
> frame object, and writes go to both the frame object and the mapping:
> https://github.com/python/cpython/pull/3640/files#diff-7b8cef249e5cca077d30de4e428a6bde6b9b803464e790e9cffa7e052e19efddR1315
> >
> > 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.
>
_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/HLWVTPUF5GPSUDU322AHYRPCEPFZEYKM/
Code of Conduct: http://python.org/psf/codeofconduct/

Reply via email to