GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — [WIP, DO NOT MERGE] bpo-41188: Prepare CPython for opague PyObject structure. by WildCard65 · Pull Request #21262 · python/cpython Conversation
|
| */ | ||
|
|
||
| /* Set if the type object's tp_basicsize is set for opague object */ | ||
| #define Py_TPFLAGS_OPAQUE_OBJECT (1UL << 8) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I would prefer a more explicit name, lilke: Py_TPFLAGS_OMIT_PYOBJECT_SIZE.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I would prefer to open a separated issue on bugs.python.org for this work. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@vstinner The static library will contain '.c' files that reference internal implementations FOR the runtime it's designed for. As it currently stands, the current Py_TYPE still requires direct access to internals for end users while this static library remove that requirement. example would be: This allows for implementation independent access while still allowing the C compiler to aggressively inline out the small functions. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…thon's internals and external users. Note: This static library is incomplete.
|
@vstinner Before I proceed any further, I want you to review "slib_pythoncore" first. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
This is the purpose of the limited C API which provides a stable ABI. It's already implemented since Python 3.2. I don't see the point of adding a second flavor of the "libpython" library. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
As it stands, functions like _Py_TYPE, _Py_SIZE, _Py_INCREF, _Py_DECREF rely on "PyObject" being a complete type. Opaque objects on the other hand are classified as incomplete types. Incomplete types can't be used for anything but pointers (*) and references (&), these functions will fail when PyObject goes opaque in the limited API, infact, sizeof() also fails on incomplete types. The purpose of the static library is not to be a different flavour of Python, but to act as a bridge into Python's internals while respecting the limitations of incomplete types, while also eliminating function call overhead. As an added bonus, this static library can be "reskinned" for runtimes that take a different approach to PyObject (if any) and descendant built-in types while end users don't have to worry about what the runtime's schematic is. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Added a basic implementation of the flag, but I do not know where all usages of "tp_basicsize" is, I'm also debating on whether or not I need to make GET_TYPE_TOTALSIZE into an inlinable function. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
IMHO you gone too far in the implementation before discussing the design. You should first propose the design on python-ideas or python-dev list.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| vectorcallfunc tp_vectorcall; | ||
|
|
||
| /* INTERNAL USE ONLY! MODIFYING THIS CAN CRASH PYTHON! */ | ||
| const Py_ssize_t tp_obj_offset; /* Offset from "PyObject *" pointer */ |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't think that this is needed. When I get a "PyObject *ob", the offset to the PyObject structure is 0: "PyObject copy = *ob;" in Python internals is valid.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It's not required if everything inherited ONLY PyObject, that will never be true.
The offset is calculated from the immediate base type of the type, which can be for example: list, tuple, dict, type, custom types.
Same thing applies to "tp_obj_size", without these members, heap corruption can be very likely.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
||
| /* INTERNAL USE ONLY! MODIFYING THIS CAN CRASH PYTHON! */ | ||
| const Py_ssize_t tp_obj_offset; /* Offset from "PyObject *" pointer */ | ||
| const Py_ssize_t tp_obj_size; /* Total memory allocation size */ |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't see the point of adding a new member. You can just check for Py_TPFLAGS_USES_OPAQUE_OBJECT flag in functions like PyType_GenericAlloc(), _PyObject_SIZE() and _PyObject_VAR_SIZE(). If the flag is set, add sizeof(PyObject).
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
See previous comment.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| Py_SLIB_LOCAL(PyTypeObject *) PyObject_GetType(const PyObject *ob); | ||
|
|
||
| Py_SLIB_LOCAL(void) PyObject_SetRefCount(PyObject *ob, const Py_ssize_t refcnt); | ||
| Py_SLIB_LOCAL(void) PyObject_SetType(PyObject *ob, const PyTypeObject *type); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Python 3.9 already have Py_SET_REFCNT() and Py_SET_TYPE() functions. What is the advantage of adding new functions?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
These aren't essentially "new functions", they are bridges into CPython's internals WITHOUT leaking implementation details.
As mentioned before, the existing functions WILL FAIL when PyObject goes full opaque.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Py_TPFLAGS_USES_OPAQUE_OBJECT looks useful, but other changes like "slib" looks useless to me. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't see the point of tp_obj_offset, tp_obj_size and slib.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Consider the following: struct myobject; void use_myobject(struct myobect *obj); // Valid, incomplete types can be used as pointers. size_t get_object_value(struct my object *obj) { return obj->m_value; // ERROR: my_object is an incomplete type! } Above demonstrates why functions like _Py_Type will fail in limited api. 'Py_TPFLAGS_OMIT_OBJECT_SIZE' was intended as the stepping stone for making types like PyTypeObject opaque as well, this makes knowing the true offset off the base time a runtime dependant scenario, one with minimal impact to performance, probably less than having _Py_TYPE, etc, exported from python binary. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@vstinner One way or another, PY_TYPE and co need to have their implementations PURGED from the Python header files for a truly opaque PyObject. Another thing that needs to be purged is "ob_base" usage of objects like PyTupleObject, PyTypeObject, PyListObject to name a few. I'm willing to replace slib with a precompiled header trio, but that's a can of worms on its own (at least on MSVC). I'm not budging in regards to "tp_obj_offset" and "tp_obj_size", see above mentioned purging. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.