From ba49b0feb2d45471cbc370ae61b347ca03a9c069 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 10 Jul 2021 20:48:15 +1000 Subject: [PATCH 1/4] PEP 558: adjustments after updating implementation * Some fast locals proxy operations will implicitly update the underlying shared mapping on the frame * Anyone explicitly calling `LocalsToFast` is going to want the read/write proxy, not any of the read-only options (plus it's hard to fit an essay in an error message) * Remove lingering reference to the removed PyLocals_RefreshViews() --- pep-0558.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pep-0558.rst b/pep-0558.rst index 525b16086b5..ed12f34c917 100644 --- a/pep-0558.rst +++ b/pep-0558.rst @@ -485,6 +485,10 @@ will be updated only in the following circumstance: * any call to ``PyFrame_GetLocals()``, ``PyFrame_GetLocalsCopy()``, ``_PyFrame_BorrowLocals()``, ``PyFrame_FastToLocals()``, or ``PyFrame_FastToLocalsWithError()`` for the frame +* any operation on a fast locals proxy object that requires the shared + mapping to be up to date on the underlying frame. In the initial reference + implementation, those operations are ``flp.values()``, ``flp.items()``, + ``flp.copy()``, and object comparison. Accessing the frame "view" APIs will *not* implicitly update the shared dynamic snapshot, and the CPython trace hook handling will no longer implicitly update @@ -522,9 +526,9 @@ to avoid having to access the internals of the frame struct from the The ``PyFrame_LocalsToFast()`` function will be changed to always emit ``RuntimeError``, explaining that it is no longer a supported operation, and -affected code should be updated to use ``PyFrame_GetLocals(frame)``, -``PyFrame_GetLocalsCopy(frame)``, ``PyFrame_GetLocalsView(frame)``, or -``PyObject_GetAttrString(frame, "f_locals")`` instead. +affected code should be updated to use +``PyObject_GetAttrString(frame, "f_locals")`` to obtain a read/write proxy +instead. In addition to the above documented interfaces, the draft reference implementation also exposes the following undocumented interfaces:: @@ -734,7 +738,9 @@ simplify the semantics and implementation. Note: if this change proves problematic in practice, it would be reasonably straightforward to amend the implementation to store unknown keys in the C level -``f_locals`` mapping, the same way ``PyEval_GetLocals()`` allows. However, +``f_locals`` mapping, the same way ``PyEval_GetLocals()`` allows (the proxy +already relies on that mapping to provide other features, like mapping +comparison, and iterating over values and items). However, starting the new API with it disallowed offers the best chance of potentially being able to deprecate and remove the behaviour entirely in the future. @@ -804,7 +810,7 @@ into the following cases: be visible to Python code. This is the ``PyLocals_GetCopy()`` API. * always wanting a read-only view of the current locals namespace, without incurring the runtime overhead of making a full copy each time. This is the - ``PyLocals_GetView()`` and ``PyLocals_RefreshViews()`` APIs. + ``PyLocals_GetView()`` API. Historically, these kinds of checks and operations would only have been possible if a Python implementation emulated the full CPython frame API. With From 8751c2a76fe53d3e9cfbae843d393251e0c198e9 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 10 Jul 2021 21:24:35 +1000 Subject: [PATCH 2/4] str() also relies on frame->f_locals --- pep-0558.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pep-0558.rst b/pep-0558.rst index ed12f34c917..056b1b99c42 100644 --- a/pep-0558.rst +++ b/pep-0558.rst @@ -488,7 +488,7 @@ will be updated only in the following circumstance: * any operation on a fast locals proxy object that requires the shared mapping to be up to date on the underlying frame. In the initial reference implementation, those operations are ``flp.values()``, ``flp.items()``, - ``flp.copy()``, and object comparison. + ``flp.copy()``, object comparison, and rendering as a string. Accessing the frame "view" APIs will *not* implicitly update the shared dynamic snapshot, and the CPython trace hook handling will no longer implicitly update From 092f7ef4e7b30fd79b1eafb643278569a41ffc1f Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 10 Jul 2021 23:42:52 +1000 Subject: [PATCH 3/4] Alas, pdb stores extra state on arbitrary frames --- pep-0558.rst | 53 +++++++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/pep-0558.rst b/pep-0558.rst index 056b1b99c42..b398e1eb3d6 100644 --- a/pep-0558.rst +++ b/pep-0558.rst @@ -363,7 +363,10 @@ API: ``__getitem__`` operations on the proxy will populate the ``fast_refs`` mapping (if it is not already populated), and then either return the relevant value -(if the key is found in the ``fast_refs`` mapping), or else raise ``KeyError``. +(if the key is found in either the ``fast_refs`` mapping or the ``f_locals`` +dynamic snapshot stored on the frame), or else raise ``KeyError``. Variables +that are defined, but not yet bound raise ``KeyError`` (just as they're +omitted from the result of ``locals()``). As the frame storage is always accessed directly, the proxy will automatically pick up name binding operations that take place as the function executes. @@ -373,9 +376,11 @@ directly affect the corresponding fast local or cell reference on the underlying frame, ensuring that changes are immediately visible to the running Python code, rather than needing to be written back to the runtime storage at some later time. -Unlike the existing ``f_locals`` implementation on optimised frames, the frame -locals proxy will raise ``KeyError`` for attempts to write to keys that aren't -defined as local or closure variables on the underyling frame. +All modified keys, regardless of whether they're defined as local or closure +variables on the underlying frame, will also be written to the ``f_locals`` +shared dynamic snapshot on optimised frames. This allows utilities like +``pdb`` (which writes ``__return__`` and ``__exception__`` values into the +frame ``f_locals`` mapping) to continue working as they always have. Other ``Mapping`` and ``MutableMapping`` methods will behave as expected for a mapping with these essential method semantics. @@ -487,8 +492,10 @@ will be updated only in the following circumstance: ``PyFrame_FastToLocalsWithError()`` for the frame * any operation on a fast locals proxy object that requires the shared mapping to be up to date on the underlying frame. In the initial reference - implementation, those operations are ``flp.values()``, ``flp.items()``, - ``flp.copy()``, object comparison, and rendering as a string. + implementation, those operations are any that require a full set of mapping + keys and/or values, including ``len(flp)``, ``flp.keys()``, ``flp.values()``, + ``flp.items()``, ``flp.copy()``, iteration, containment checks, object + comparison, and rendering as a string. Accessing the frame "view" APIs will *not* implicitly update the shared dynamic snapshot, and the CPython trace hook handling will no longer implicitly update @@ -555,9 +562,9 @@ this PEP incorporate's Victor Stinner's proposal to no longer implicitly call ``PyFrame_FastToLocalsWithError()`` before calling trace hooks implemented in Python. -Code using the new frame view APIs won't need the dynamic locals snapshot -refreshed, while code using the ``PyEval_GetLocals()`` API will implicitly -refresh it when making that call. +Code using the new frame view APIs will have the dynamic locals snapshot +implicitly refreshed when accessing methods that need it, while code using the +``PyEval_GetLocals()`` API will implicitly refresh it when making that call. The PEP necessarily also drops the implicit call to ``PyFrame_LocalsToFast()`` when returning from a trace hook, as that API now always raises an exception. @@ -725,24 +732,24 @@ emulation of CPython's frame API is already an opt-in flag in some Python implementations). -Dropping support for storing additional data on optimised frames ----------------------------------------------------------------- +Continuing to support storing additional data on optimised frames +----------------------------------------------------------------- -Earlier iterations of this PEP proposed preserving the ability to store +One of the draft iterations of this PEP proposed removing the ability to store additional data on optimised frames by writing to ``frame.f_locals`` keys that didn't correspond to local or closure variable names on the underlying frame. -While that property has been retained for the historical ``PyEval_GetLocals()`` -C API, it has been dropped from the new fast locals proxy proposal in order to -simplify the semantics and implementation. - -Note: if this change proves problematic in practice, it would be reasonably -straightforward to amend the implementation to store unknown keys in the C level -``f_locals`` mapping, the same way ``PyEval_GetLocals()`` allows (the proxy -already relies on that mapping to provide other features, like mapping -comparison, and iterating over values and items). However, -starting the new API with it disallowed offers the best chance of potentially -being able to deprecate and remove the behaviour entirely in the future. +While this idea offered some attractive simplification of the fast locals proxy +implementation, ``pdb`` stores ``__return__`` and ``__exception__`` values on +arbitrary frames, so the standard library test suite fails if that functionality +no longer works. + +Accordingly, the ability to store arbitrary keys was retained, at the expense +of certain operations on proxy objects currently being slower than desired (as +they need to update the dynamic snapshot in order to provide a reliable answer). + +Future implementation improvements should allow that lost performance to be +recovered by only refreshing the snapshot when it is known to be out of date. Historical semantics at function scope From 2c1d8c405e5e9610982a72b70a036f86f0749811 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 10 Jul 2021 23:59:25 +1000 Subject: [PATCH 4/4] Only non-frame keys are currently written directly to the snapshot --- pep-0558.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pep-0558.rst b/pep-0558.rst index b398e1eb3d6..b8c8bc47ad2 100644 --- a/pep-0558.rst +++ b/pep-0558.rst @@ -376,11 +376,11 @@ directly affect the corresponding fast local or cell reference on the underlying frame, ensuring that changes are immediately visible to the running Python code, rather than needing to be written back to the runtime storage at some later time. -All modified keys, regardless of whether they're defined as local or closure -variables on the underlying frame, will also be written to the ``f_locals`` -shared dynamic snapshot on optimised frames. This allows utilities like -``pdb`` (which writes ``__return__`` and ``__exception__`` values into the -frame ``f_locals`` mapping) to continue working as they always have. +Keys that are not defined as local or closure variables on the underlying frame +will instead be written to the ``f_locals`` shared dynamic snapshot on optimised +frames. This allows utilities like ``pdb`` (which writes ``__return__`` and +``__exception__`` values into the frame ``f_locals`` mapping) to continue +working as they always have. Other ``Mapping`` and ``MutableMapping`` methods will behave as expected for a mapping with these essential method semantics.