Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-40170: PyType_HasFeature() no longer acccess directly tp_flags #19378

Merged
merged 1 commit into from
Apr 7, 2020
Merged

bpo-40170: PyType_HasFeature() no longer acccess directly tp_flags #19378

merged 1 commit into from
Apr 7, 2020

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Apr 4, 2020

PyType_HasFeature() now always calls PyType_GetFlags() to hide
implementation details. Previously, it accessed directly the
PyTypeObject.tp_flags member when the limited C API was not used.

Add fast inlined version _PyType_HasFeature() and _PyType_IS_GC()
for object.c and typeobject.c.

https://bugs.python.org/issue40170

@vstinner
Copy link
Member Author

vstinner commented Apr 4, 2020

I expect no significant performance difference or a very low overhead for macros and functions using PyType_HasFeature().

PyType_HasFeature() is used by the following code in header files:

  • PyVectorcall_Function()
  • PyType_IS_GC()
  • PyType_FastSubclass()

PyType_FastSubclass() is used by many PyXXX_Check() macros:

  • PyLong_Check()
  • PyBytes_Check(), PyUnicode_Check()
  • PyTuple_Check(op), PyList_Check()
  • PyDict_Check()
  • PyExceptionClass_Check(), PyExceptionInstance_Check()

@vstinner
Copy link
Member Author

vstinner commented Apr 5, 2020

@serhiy-storchaka @methane: Do you expect such change to have an impact on performance? What do you think of the change?

@methane
Copy link
Member

methane commented Apr 5, 2020

Some PyType_HasFeature(cls, Py_TPFLAGS_HAVE_VERSION_TAG) are in Object/typeobject.c.
They are performance critical.
Can we have private inline function?

@vstinner
Copy link
Member Author

vstinner commented Apr 5, 2020

Can we have private inline function?

Yes, that's possible but I would prefer to add new functions to the internal C API for that. For example, add a fast _PyLong_Check() function.


In the past, I tried to reuse the same name for two implementation of the same function: one "slow" using a function call, one "fast" using a macro or static inline function.

I'm thinking at PyThreadState_GET(). The internal C API uses:

/* Redefine PyThreadState_GET() as an alias to _PyThreadState_GET() */
#undef PyThreadState_GET
#define PyThreadState_GET() _PyThreadState_GET()

The problem is that depending if the internal C API is included or not, you may get the slow or the fast function.

I changed my approach: use a different name for the fast function. _PyThreadState_GET() is guaranteed to be fast, but it's only provided by the internal C API. In short, using it ensures that you get the fast implementation, but also that the internal C API is included.

PyType_HasFeature() now always calls PyType_GetFlags() to hide
implementation details. Previously, it accessed directly the
PyTypeObject.tp_flags member when the limited C API was not used.

Add fast inlined version _PyType_HasFeature() and _PyType_IS_GC()
for object.c and typeobject.c.
@vstinner
Copy link
Member Author

vstinner commented Apr 7, 2020

@methane: I added fast inlined version _PyType_HasFeature() and _PyType_IS_GC() for object.c and typeobject.c.

@vstinner vstinner merged commit 45ec5b9 into python:master Apr 7, 2020
@vstinner vstinner deleted the type_has_feature branch April 7, 2020 23:42
vstinner added a commit that referenced this pull request Jul 8, 2020
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 8, 2020
…gs() (pythonGH-19378)" (pythonGH-21390)

This partially reverts commit 45ec5b9.
(cherry picked from commit b26a0db)

Co-authored-by: Victor Stinner <vstinner@python.org>
miss-islington added a commit that referenced this pull request Jul 8, 2020
…gs() (GH-19378)" (GH-21390)

This partially reverts commit 45ec5b9.
(cherry picked from commit b26a0db)

Co-authored-by: Victor Stinner <vstinner@python.org>
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jul 12, 2020
arun-mani-j pushed a commit to arun-mani-j/cpython that referenced this pull request Jul 21, 2020
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants