Great, I am happy to see this submitted. I have one clarification with regards to the decision tree lines
Py_TPFLAGS_ITEMS_AT_END used: itemsize is inherited.
Py_TPFLAGS_ITEMS_AT_END not used: Fail. (Possible conflict.)
This flag could be specified in two different places (spec and base). Does it need another sub-decision tree, or is it okay if either class has the flag?
It’s okay if either class has the flag. It’s in the previous section, so I hope the simple “used” is OK for the big-picture decision tree.
Here’s my belated critique of PEP 697.
Overall, I agree that this is a problem that needs solving, and the idea
of decoupling the structs needed for extension classes from the object layout
is important.
However, the details are too tied to the current VM implementation and will hurt
our ability to improve the VM in the future.
The overhead of using this API is likely to be too large for tools like Cython and
MyPyC, as well as extensions like NumPy. If the API isn’t good enough to implement tuple or list using it, then it is unreasonable to expect third-parties to use it as well.
The section of variable-size layouts is too tied to the existing implementation, which we are trying to move away from.
We already moved the __dict__ to a fixed offset in Python objects, which allows much faster attribute access.
The only reason we can’t do that for variable sized objects like subclasses of tuple, is the constraints imposed by the C API. Let’s not make the situation worse.
The decision tree for layout seems too complicated. This is likely to lead to errors in understanding, implementation, or both.
PyObject_GetTypeData and PyType_GetTypeDataSize should be defined by what they do, not how they are implemented.
In other words, instead of defining PyObject_GetTypeData(obj, cls) as return (char *)obj + _align(cls->tp_base->tp_basicsize);
it should be defined as “returns a pointer to the struct for class cls within the object obj”.
The Py_RELATIVE_OFFSET is good, as it decouples field access from the absolute layout, which is what we want.
Putting the offset calculations behind an API is also a step forward.
Something like this:
Grand Unified Python Object Layout · Issue #553 · faster-cpython/ideas · GitHub
Right, extensions that mess with the internals for speed aren’t the intended use case. But there are plenty of extensions that can trade a few nanoseconds for safety & consistency.
I think the API could be good enough for most uses of list/tuple. But doing that would mean changing their layout, which would be a much bigger change.
That’s mostly because the PEP explains how the API ties into the current implementation. If you only look at the proposed API, I hope you’ll find it a reasonable abstraction. For example, if your Under Grand Unified Python Object Layout comes to be, it shouldn’t be hard to adapt the proposed API to it.
Similar situation here. The status quo is complex.
The leaves of the tree tell you how the proposal deals with each case, but the structure of the tree is mostly status quo – what to think of when you try to extend arbitrary classes today.
Yes, they should be, and I tried to make that clear with this disclaimer:
In the code blocks below, only function headers are part of the specification. Other code (the size/offset calculations) are details of the initial CPython implementation, and subject to change.
Saying what they do with the current class layout makes the proposal much more understandable for readers who are already familiar with the current layout. That’s the only reason the function bodies are in the PEP.
For example, if your Under Grand Unified Python Object Layout comes to be, it shouldn’t be hard to adapt the proposed API to it.
Almost.
Py_TPFLAGS_ITEMS_AT_END flags specifies the layout (at least the name implies it does).
The PEP says:
Py_TPFLAGS_ITEMS_AT_END, which will indicate thePyHeapTypeObject-like layout.
What does “PyHeapTypeObject-like” mean here?
Is defining the placement necessary, or is it just saying that the struct data is variable sized?
Would Py_TPFLAGS_VARIABLE_SIZED_STRUCT be OK?
One other thing I overlooked is the reference to the “base class” in the PEP. The problem with that is it assumes single-inheritance. By defining the size of the object, rather than the size of the struct, multiple inheritance becomes impossible (or somewhat fragile, at least):
Suppose we have a class C1 that needs a struct S1 and a class C2 that needs a struct S2, both defined to inherit from object.
Now define two classes,
class D(C1, C2): pass
class E(C2, C1): pass
To compute the offsets of S1 and S2 in D() and E() we need to reverse the computation used to get basicsize in the first place.
If the extension defines the size, itemsize and alignment of the struct, not the class, and leave placement to the VM, then things are a lot more robust.
Yes, the flag names a specific pre-existing layout, so the interpreter can know that a class uses it. If a better layout is added in the future, this one can be phased out (along with the name that the PEP adds for it).
Items at end, rather than at a fixed offset (the second set of diagrams).
No, that’s would imply just tp_basicsize > 0.
That’s, again, part the status quo that the PEP does not try to change. When C structs are involved, you’re currently limited to single inheritance.
However, the new API should be compatible with leaving placement to the VM. Again, don’t look at the proposed function bodies, those are for the current CPython and subject to change.
The Python Steering Council has reviewed PEP 697 and is accepting it pending a couple things to clarify in the text:
(1) We’re accepting the negative spec->basesize. The PEP text still lists that as an Open Issue to be resolved. Please mark it as such and just mention the flag alternative under “Rejected Ideas”.
Rationale: The meaning of this internal field is changed by this PEP regardless so if there were code accessing these internals without knowing about the special negative or about an equivalent flag, the runtime consequences would be similar: memory or data corruption. We think the negative is a practical approach. The implementation detail will remain hidden within CPython internals.
One caveat: We suspect some debugger related code that does directly use at internal values might need to be updated. Take a look at Tools/gdb/libpython.py when making your PR; that might need updating? If users have debugging tools that do the same, they’ll need to do similar but as these are internals this is a normal kind of change to expect to adapt to within a release.
(2) While the current PEP-697 text ultimately covers the details of its specific scope. It isn’t immediately obvious to the reader in the Abstract the scope limitations that non-heap-allocated data variable size types are intentionally excluded.
While re-reading a few times so I could write this up, I noticed some typos and realize that I should just propose edits to take care of both suggestions above along with those anyways so… Cleanup PEP-697 before PSC acceptance. by gpshead · Pull Request #3041 · python/peps · GitHub
Thanks for your work on this and writing the PEP!
Thank you!
To clarify, does this mean the PEP is accepted now that I merged your PR?
It’ll remain hidden in the internals of whatever code creates types, not just CPython.
PyType_Spec is used to create types, but can’t be retrieved from a PyTypeObject. Usually it’s defined as a static (immortal and local to one source file), or on the heap (so the spec doesn’t outlive the type created from it).
If some extension decides to make its type specs public for some reason – well, it doesn’t need to switch to negative basicsize if that would break its users.
Negative values will only appear in spec->basicsize. When the actual type is created, type->tp_basicsize is set to the computed positive value, so introspection tools (including Tools/gdb/libpython.py) won’t be affected.
To clarify, does this mean the PEP is accepted now that I merged your PR?
Yep! Accepted!