Comments on PEP 573 – Module state access from C extension modules
Hi all, and thank you to the authors of this well thought-out PEP.
https://www.python.org/dev/peps/pep-0573/
I read through the latest version and found it in a very good and convincing shape. It has been resting for more than a year now, and some things happened along the way, most notably PEP 590, to which I think it should be adapted. Here are my comments.
I find it a bit sad that state lookups in slot methods are disadvantaged compared to normal methods, given that their entire point is to provide a performance _benefit_. However, for the time being, we do not know how relevant and visible this will become in practice, so it is good that the PEP does not propose a new/real/complex solution to it for now. Instead, it resorts to the MRO lookup (which can always be done), and leaves any larger change open for the future, when we know better if there really is a problem to solve. Check from my side, this is the right trade-off.
I like the idea of immutable exception types.
I'm not entirely happy with the word "Prepare" vs. "New" in the function name "PyErr_PrepareImmutableException", which is described to be similar to "PyErr_NewExceptionWithDoc". I understand that it does not create a new type everytime you call it, but then, it also only prepares something the first time you call it. Can't say if there is a good way to express this behaviour, but "prepare" seems worse to me than "new" or "create" IMHO, not better.
Section [1] is the one on extension type methods, which IMHO should be adapted to PEP 590. I think it should be based on the vectorcall/fastcall calling conventions and not take tuple and dict but C arrays as arguments. Alternatively, we could have two new signatures (old-style + new-style), but that feels like overkill. the fastcall signatures are easy enough to handle in C code, and in most cases easier than tuple+dict. The signature of PyCMethod should become
(PyObject *obj, PyTypeObject *defining_class,
PyObject *const *args, size_t nargs,
PyObject *keywords)The descriptor special casing [2] in PEP 590 is also worth mentioning in the context of this PEP. As far as I can see it, the C method type should implement the vectorcall protocol, and then pass the call through to the underlying C function, adding the "defining_class" parameter along the way. I've written up a PR [3] to change the PEP in that direction.
The "_PyMethodDef_RawFastCall*" functions may no longer need to be modified after this, I think.
BTW, if there is "a new type PyCMethodObject", then there is probably also a PyTypeObject for it, right? That should be mentioned somewhere.
The section on static/immutable exceptions [4] says "On deinitialization of the exception type, *exc will be set to NULL". What sets them to NULL?
Under "Helpers", it says "PyType_GetModuleState … returns pointer to state of the module". I assume that this refers to the PEP-3121 module state, but an explicit reference would make it clearer.
It also says in the same section that "When a type without a module is passed in, SystemError is set and NULL returned". Is that necessarily an exception case? Why not simply return NULL and let the user deal with it? I think the question is: is there a use case for getting the module state of an arbitrary type? If so, passing a type that does not have a module reference is fine and should not raise an exception. If that's not a use case, then it _might_ be worth informing the user about their misuse, or not. I don't know. What was the intention here?
Finally, the reference link to the patch set of the implementation does not work for me. Maybe it should be this?
https://github.com/Dormouse759/cpython/compare/master...Dormouse759:pep-c
As you can see, I do not have any objections or major concerns about this PEP. Next, I'll look through the implementation, and would also like to see at least an attempt to implement it in Cython, to get an idea about how its usage would look in the general case. But apart from that, I can see that a lot of thinking has already gone into this PEP.
Stefan
[1] https://www.python.org/dev/peps/pep-0573/#passing-the-defining-class-to-exte...
[2] https://www.python.org/dev/peps/pep-0590/#descriptor-behavior
[3] https://github.com/python/peps/pull/1159
[4] https://www.python.org/dev/peps/pep-0573/#static-exceptions
On 9/10/19 2:15 PM, Stefan Behnel wrote:
I do to, but I realized it is orthogonal to the rest of the PEP, and might conflict with other efforts like Eric's conversion to heap types. I would rather leave this to a follow-up PEP. I proposed removal of immutable exceptions in the PR [3].
[...]
One nitpick I realized now: vectorcall uses a "number of arguments + offset flag" argument, so we now tend to use "nargsf" instead of just "nargs" for the argument name. I added that to the PR.
That's just a C type (struct) for more storage, it shouldn't be visible from Python.
[...]
The previous paragraph says "per-module state", to the terminology section. In the PR, I linked to the terminology section.
I personally hate calling things "PEP-XXX Y", because I have trouble remembering numbers. So prefer to not call this "PEP-3121 module state".
I don't know, so I default to not let errors pass silently.
It's now https://github.com/Dormouse759/cpython/tree/pep-c-rebase_newer And it's a work in progress.
Also changed in the PR.
Thank you for the review!
On 2019-09-10, Stefan Behnel wrote:
While PEP 573 accomplishes the goal of providing access to the module state, I don't find it too elegant. I would like a design that reduces the differences between native Python code functions compared to ones implemented in C code.
For native Python code (i.e. implemented by bytecode), functions keep a reference to their module namespace. I.e. the __globals__ attribute of a PyFunctionObject. I think we should have a PyFunctionObject analog for functions implemented by C code. They would bind a reference to their namespace (i.e. the module/module state) when they are defined.
That would remove the need for PyType_DefiningTypeFromSlotFunc(). Once you have the function object, you have the reference to the correct module. No need to crawl around in the MRO to find it.
On 9/12/19 3:03 PM, Neil Schemenauer wrote:
With PEP 573, you don't need PyType_DefiningTypeFromSlotFunc() for
normal methods. It's there for slots. For example, addition (nb_add)
calls the C function:
typedef PyObject * (*binaryfunc)(PyObject *left, PyObject *right);There's no space to pass a PyFunction object, or any other object, to this directly. PEP 573 *gives up* on solving the problem, but documents the least bad solution that's available now, and gives you the PyType_DefiningTypeFromSlotFunc helper since it's not trivial to write.
For normal methods, your idea is almost what the PEP proposes -- with one extra indirection through the defining class (which you can't reliably get from the module state).
The context that a method code could want is:
- (sub)Interpreter state
- Module (module object or its state)
- Defining class
- The (unbound) method object
These form a tree (an interpreter has many modules; a module has many types), and it's easy to get an item's parent (there just needs to be a pointer available) but it's not easy to get a child (Python code uses name-based lookups to get stuff from __globals__, but that's unsafe for C code, since anyone replace a __globals__ entry with an unexpected object). So, with PEP 590 (vectorcall) we pass the leaf of that hierarchy to the implementation code, and PEP 573 proposes adding the missing "parent" pointers.
Petr Viktorin schrieb am 12.09.19 um 16:28:
I'm wondering, would it hurt to also reference and pass the module directly from the normal methods? It would create a double reference cycle, ok, which doesn't seem worse than a single one. But it seems to me that methods (just like functions) are more likely to need a reference to their module (and its globals) than to their defining class. To other functions, types, loggers, global configuration, all that. Why require the methods to go through their defining class and then to their module and then to its state or globals, when we can just give the module to them directly?
I concur with Neil that making C functions/methods as much like Python functions/methods as possible is a worthy goal all by itself.
Stefan
On 9/12/19 4:01 PM, Stefan Behnel wrote:
I'm probably missing some context here. Reference from where, and pass to where?
They might need *any* of these: method object, defining class for things like clone() methods, module, or the interpreter state. You should be able to get to any of them from the method object.
The proposed PyType_GetModuleState() helper will give you the module state from the defining class. It does two checks and pointer dereferences (if it's optimized). I don't think it needs further optimization now.
I concur with Neil that making C functions/methods as much like Python functions/methods as possible is a worthy goal all by itself.
I disagree. I want to make it *possible* to make Python-like functions/methods. C functions/methods have their own advantages and restrictions. Making them the same is a goal for Cython; the goal of CPython is to make it easier for Cython.
On 2019-09-12, Petr Viktorin wrote:
Slots like np_add could lookup the PyFunction analog and call it. That object has the reference to the module namespace, just like a PyFunction object does. When the module is first created, you allocate heap objects for each of the PyFunction object thingies and bind them to the module. I believe this could (eventually) greatly simplify typeobject.c.
Most of the CPython runtime should not have to care if a function is implemented using bytecode or is implemented in C. The way things are organized now, there a lot of differences. Over time, I would prefer we make those two things more similar rather than more different. I see the PEP 573 an expedient solution but not an elegant one.
Performance of my proposed approach is a potential issue. Having nb_add point to a C function that directly implements the add operation is more efficient than looking up a PyFunction like thing (maybe from tp_dict, not sure) and then calling that. However, by making these things look the same, I believe you would remove a slew of special cases and branches within the CPython runtime and maybe preformance is as good or better. For types with an immutable nb_add slot, you could JIT the machine code for the nb_add slot and avoid the dynamic lookup of the PyFunction thingy.
On 9/12/19 9:05 PM, Neil Schemenauer wrote:
Where would they look it up?
Why to the module, and not to the type object (which is bound to the module)?
That, I agree with. Most of the runtime should just call a callable object.
Sorry, but I don't see what you mean by "making things look the same". Do you want to get source/tracebacks from C code? Have C code be as dynamic (and full of slow checks) as Python? Do you want to write a PEP for that?
Again, PEP 573 doesn't really try to solve the slots case.
On 2019-09-12, Petr Viktorin wrote:
I think either in tp_dict or you can have a table of object pointers indexed by slot ID.
To match the way PyFunction works. It has a reference to the module global namespace (i.e. __globals__). Likewise, functions defined in an extension module should have a reference to their global namespace.
The binding of a method to the type object should be done by the descriptor. That will make methods implemented in native Python code work similar to methods implemented by C code.
No, I'm talking about looking up functions and methods and calling them. I want the fact that they are implemented in Python or in C to be encapulated more than it currently is.
Do you want to write a PEP for that?
I suppose I could try. While I think the idea should work, there many details to work out. Maybe there is some fatal flaw.
On 9/12/19 10:56 PM, Neil Schemenauer wrote:
tp_dict of what? Py_TYPE(self) doesn't work for subclasses.
For normal methods, it's almost what PEP 573 does: I don't see much difference between this and storing the *type* with the PyFunction, and passing the type to the C code, which can then access the module state from the type.
For slots, this sounds like a way forward. There's quite a lot practical complications, though, so PEP 573 does not really address slots -- aside from the PyType_DefiningTypeFromSlotFunc crutch, which doesn't pretend to be a very good solution, but it's a very simple one that's available now. (If *that* is where PEP 573 strikes you as inelegant but expedient, then we agree. My educated guess is that an elegant alternative to this will take years to design and implement, and will be largely orthogonal to the rest of PEP 573.)
PEP 573 doesn't change how methods are looked up. They're looked up on the instance, which normally delegates to the type and its superclasses. The changes in PEP 573 are:
- the type object is passed to the C code
- the type object has a reference to its defining class, which has a reference to the module (All of that is opt-in for backwards compatibility)
PEP 573 has a fairly complete proof of concept implementation, so I'm fairly sure it doesn't have fatal flaws. (It does have deliberate omissions, though.)
On Fri, 13 Sep 2019 at 08:22, Petr Viktorin <encukou@gmail.com> wrote:
I also think we may end up deciding that this is something better left to context variables, as those are explicitly designed to be fast enough for the Decimal module to use them instead of a thread local variable, and it will allow C extension methods to cache the reference they actually care about, with whatever form of cache invalidation they want to support (with the separation of the cache by coroutine/thread/interpreter handled by the context variable machinery).
That way even if the slow paths continue to differ between binary extensions (MRO walking and METH_METHOD) and Python source modules (__class__ closure cell and function __globals__), the fast paths will converge (context variable lookup).
Cheers, Nick.
-- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
Petr Viktorin schrieb am 13.09.19 um 11:43:
Ah, that's not what I meant. I think we should have that function, as it seems to fill a need in the current setup. I just meant that we shouldn't add more mechanisms than that.
I would even argue that it would be nice to have yet another function that returns the module object directly, instead of requiring extension code to a) know that that's referenced by the type and b) get it from there by calling the module-for-class function/macro. It would be nice if extension authors could express explicitly what they want, so that the API becomes more independent of how things work exactly.
However, I do accept that this function is essentially a kludge for as long as we have no better way to do it. So adding even more to the API right now isn't something I'd fight for.
Stefan
I've sent another PR to the PEP. Aside from clean-ups, it removes the slot for dict and weaklist offsets. Dino Viehland is working on a better solution for those.
On 9/10/19 2:15 PM, Stefan Behnel wrote:
I do to, but I realized it is orthogonal to the rest of the PEP, and might conflict with other efforts like Eric's conversion to heap types. I would rather leave this to a follow-up PEP. I proposed removal of immutable exceptions in the PR [3].
[...]
One nitpick I realized now: vectorcall uses a "number of arguments + offset flag" argument, so we now tend to use "nargsf" instead of just "nargs" for the argument name. I added that to the PR.
That's just a C type (struct) for more storage, it shouldn't be visible from Python.
[...]
The previous paragraph says "per-module state", to the terminology section. In the PR, I linked to the terminology section.
I personally hate calling things "PEP-XXX Y", because I have trouble remembering numbers. So prefer to not call this "PEP-3121 module state".
I don't know, so I default to not let errors pass silently.
It's now https://github.com/Dormouse759/cpython/tree/pep-c-rebase_newer And it's a work in progress.
Also changed in the PR.
Thank you for the review!
On 2019-09-10, Stefan Behnel wrote:
While PEP 573 accomplishes the goal of providing access to the module state, I don't find it too elegant. I would like a design that reduces the differences between native Python code functions compared to ones implemented in C code.
For native Python code (i.e. implemented by bytecode), functions keep a reference to their module namespace. I.e. the __globals__ attribute of a PyFunctionObject. I think we should have a PyFunctionObject analog for functions implemented by C code. They would bind a reference to their namespace (i.e. the module/module state) when they are defined.
That would remove the need for PyType_DefiningTypeFromSlotFunc(). Once you have the function object, you have the reference to the correct module. No need to crawl around in the MRO to find it.
On 9/12/19 3:03 PM, Neil Schemenauer wrote:
With PEP 573, you don't need PyType_DefiningTypeFromSlotFunc() for
normal methods. It's there for slots. For example, addition (nb_add)
calls the C function:
typedef PyObject * (*binaryfunc)(PyObject *left, PyObject *right);There's no space to pass a PyFunction object, or any other object, to this directly. PEP 573 *gives up* on solving the problem, but documents the least bad solution that's available now, and gives you the PyType_DefiningTypeFromSlotFunc helper since it's not trivial to write.
For normal methods, your idea is almost what the PEP proposes -- with one extra indirection through the defining class (which you can't reliably get from the module state).
The context that a method code could want is:
- (sub)Interpreter state
- Module (module object or its state)
- Defining class
- The (unbound) method object
These form a tree (an interpreter has many modules; a module has many types), and it's easy to get an item's parent (there just needs to be a pointer available) but it's not easy to get a child (Python code uses name-based lookups to get stuff from __globals__, but that's unsafe for C code, since anyone replace a __globals__ entry with an unexpected object). So, with PEP 590 (vectorcall) we pass the leaf of that hierarchy to the implementation code, and PEP 573 proposes adding the missing "parent" pointers.
Petr Viktorin schrieb am 12.09.19 um 16:28:
I'm wondering, would it hurt to also reference and pass the module directly from the normal methods? It would create a double reference cycle, ok, which doesn't seem worse than a single one. But it seems to me that methods (just like functions) are more likely to need a reference to their module (and its globals) than to their defining class. To other functions, types, loggers, global configuration, all that. Why require the methods to go through their defining class and then to their module and then to its state or globals, when we can just give the module to them directly?
I concur with Neil that making C functions/methods as much like Python functions/methods as possible is a worthy goal all by itself.
Stefan
On 9/12/19 4:01 PM, Stefan Behnel wrote:
I'm probably missing some context here. Reference from where, and pass to where?
They might need *any* of these: method object, defining class for things like clone() methods, module, or the interpreter state. You should be able to get to any of them from the method object.
The proposed PyType_GetModuleState() helper will give you the module state from the defining class. It does two checks and pointer dereferences (if it's optimized). I don't think it needs further optimization now.
I concur with Neil that making C functions/methods as much like Python functions/methods as possible is a worthy goal all by itself.
I disagree. I want to make it *possible* to make Python-like functions/methods. C functions/methods have their own advantages and restrictions. Making them the same is a goal for Cython; the goal of CPython is to make it easier for Cython.
On 2019-09-12, Petr Viktorin wrote:
Slots like np_add could lookup the PyFunction analog and call it. That object has the reference to the module namespace, just like a PyFunction object does. When the module is first created, you allocate heap objects for each of the PyFunction object thingies and bind them to the module. I believe this could (eventually) greatly simplify typeobject.c.
Most of the CPython runtime should not have to care if a function is implemented using bytecode or is implemented in C. The way things are organized now, there a lot of differences. Over time, I would prefer we make those two things more similar rather than more different. I see the PEP 573 an expedient solution but not an elegant one.
Performance of my proposed approach is a potential issue. Having nb_add point to a C function that directly implements the add operation is more efficient than looking up a PyFunction like thing (maybe from tp_dict, not sure) and then calling that. However, by making these things look the same, I believe you would remove a slew of special cases and branches within the CPython runtime and maybe preformance is as good or better. For types with an immutable nb_add slot, you could JIT the machine code for the nb_add slot and avoid the dynamic lookup of the PyFunction thingy.
On 9/12/19 9:05 PM, Neil Schemenauer wrote:
Where would they look it up?
Why to the module, and not to the type object (which is bound to the module)?
That, I agree with. Most of the runtime should just call a callable object.
Sorry, but I don't see what you mean by "making things look the same". Do you want to get source/tracebacks from C code? Have C code be as dynamic (and full of slow checks) as Python? Do you want to write a PEP for that?
Again, PEP 573 doesn't really try to solve the slots case.
On 2019-09-12, Petr Viktorin wrote:
I think either in tp_dict or you can have a table of object pointers indexed by slot ID.
To match the way PyFunction works. It has a reference to the module global namespace (i.e. __globals__). Likewise, functions defined in an extension module should have a reference to their global namespace.
The binding of a method to the type object should be done by the descriptor. That will make methods implemented in native Python code work similar to methods implemented by C code.
No, I'm talking about looking up functions and methods and calling them. I want the fact that they are implemented in Python or in C to be encapulated more than it currently is.
Do you want to write a PEP for that?
I suppose I could try. While I think the idea should work, there many details to work out. Maybe there is some fatal flaw.
On 9/12/19 10:56 PM, Neil Schemenauer wrote:
tp_dict of what? Py_TYPE(self) doesn't work for subclasses.
For normal methods, it's almost what PEP 573 does: I don't see much difference between this and storing the *type* with the PyFunction, and passing the type to the C code, which can then access the module state from the type.
For slots, this sounds like a way forward. There's quite a lot practical complications, though, so PEP 573 does not really address slots -- aside from the PyType_DefiningTypeFromSlotFunc crutch, which doesn't pretend to be a very good solution, but it's a very simple one that's available now. (If *that* is where PEP 573 strikes you as inelegant but expedient, then we agree. My educated guess is that an elegant alternative to this will take years to design and implement, and will be largely orthogonal to the rest of PEP 573.)
PEP 573 doesn't change how methods are looked up. They're looked up on the instance, which normally delegates to the type and its superclasses. The changes in PEP 573 are:
- the type object is passed to the C code
- the type object has a reference to its defining class, which has a reference to the module (All of that is opt-in for backwards compatibility)
PEP 573 has a fairly complete proof of concept implementation, so I'm fairly sure it doesn't have fatal flaws. (It does have deliberate omissions, though.)
On Fri, 13 Sep 2019 at 08:22, Petr Viktorin <encukou@gmail.com> wrote:
I also think we may end up deciding that this is something better left to context variables, as those are explicitly designed to be fast enough for the Decimal module to use them instead of a thread local variable, and it will allow C extension methods to cache the reference they actually care about, with whatever form of cache invalidation they want to support (with the separation of the cache by coroutine/thread/interpreter handled by the context variable machinery).
That way even if the slow paths continue to differ between binary extensions (MRO walking and METH_METHOD) and Python source modules (__class__ closure cell and function __globals__), the fast paths will converge (context variable lookup).
Cheers, Nick.
-- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
Petr Viktorin schrieb am 13.09.19 um 11:43:
Ah, that's not what I meant. I think we should have that function, as it seems to fill a need in the current setup. I just meant that we shouldn't add more mechanisms than that.
I would even argue that it would be nice to have yet another function that returns the module object directly, instead of requiring extension code to a) know that that's referenced by the type and b) get it from there by calling the module-for-class function/macro. It would be nice if extension authors could express explicitly what they want, so that the API becomes more independent of how things work exactly.
However, I do accept that this function is essentially a kludge for as long as we have no better way to do it. So adding even more to the API right now isn't something I'd fight for.
Stefan
I've sent another PR to the PEP. Aside from clean-ups, it removes the slot for dict and weaklist offsets. Dino Viehland is working on a better solution for those.
participants (4)
-
Neil Schemenauer -
Nick Coghlan -
Petr Viktorin -
Stefan Behnel