-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
GH-124715: Move trashcan mechanism into Py_Dealloc
#132280
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
Conversation
markshannon
commented
Apr 8, 2025
•
edited by bedevere-app
bot
Loading
edited by bedevere-app
bot
- Issue: Not all types have "trashcan" protection and tp_dealloc can overflow stack #124715
47c2439
to
a7c0082
Compare
This is a good idea, IMHO. I had a similar PR but Mark's is more elegant than mine. |
🤖 New build scheduled with the buildbot fleet by @markshannon for commit 8aa20f2 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F132280%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Initial performance numbers are a bit down. 0.2% - 1.6% slower. I'm testing another branch that adds |
With faster recursion checks, performance is mixed but no worse overall:
Although the 1.5% slowdown on linux x86 is concerning. I also tried passing in the thread state as a parameter to
|
Maybe perf will be better with just one additional if for a decref that is not a container. So you have an implementation of decref for containers, and do one if to decide whether to apply it or stay with the current one. |
Do you mean a test in Adding a branch in Testing for collections is more expensive. |
Sorry, yeah, dealloc. Maybe not worth it. |
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.
The overall design LGTM.
I added a few comments.
Misc/NEWS.d/next/Core_and_Builtins/2025-04-08-17-48-11.gh-issue-124715.xxzQoD.rst
Outdated
Show resolved
Hide resolved
|
|
Replace _PyObject_GC_UNTRACK() with PyObject_GC_UnTrack() to not fail if the method was already untracked.
I wrote #133199 to fix test_functools. |