-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
gh-124218: Use per-thread refcounts for code objects #125216
Conversation
Use per-thread refcounting for the reference from function objects to their corresponding code object. This can be a source of contention when frequently creating nested functions. Deferred refcounting alone isn't a great fit here because these references are on the heap and may be modified by other libraries.
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 018b2a1 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
This seems like we are adding more code, rather than reusing the existing code for per-thread refcounts already in existence for classes. |
It is factored out and reusing existing code. That's why What concretely would you like changed? |
Quite a lot of code is being added to pycore_object.h, but I don't see code being removed from typeobject.c or wherever the code for per-thread reference counts for classes is. |
What would you like to see changed?
|
static inline void | ||
_Py_DECREF_CODE(PyCodeObject *co) | ||
{ | ||
_Py_THREAD_DECREF_OBJECT((PyObject *)co, co->_co_unique_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert that co is a code object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
co
is statically a PyCodeObject *
. We don't typically dynamically check the type when we have the static type (e.g., in PyCode_Addr2Line
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant it to be protect against incorrect casts, anyways it's minor.
static inline void | ||
_Py_INCREF_CODE(PyCodeObject *co) | ||
{ | ||
_Py_THREAD_INCREF_OBJECT((PyObject *)co, co->_co_unique_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same add assert to check it is codeobject
Co-authored-by: Kumar Aditya <[email protected]>
!buildbot AMD64 Fedora Stable Refleaks |
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 7ad129b 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - just some comment fixups inline.
Co-authored-by: mpage <[email protected]>
Use per-thread refcounting for the reference from function objects to their corresponding code object. This can be a source of contention when frequently creating nested functions. Deferred refcounting alone isn't a great fit here because these references are on the heap and may be modified by other libraries.
We will still need to address the globals and builtins dictionaries in a similar way.