-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
gh-134584: Eliminate redundant refcounting from _CALL_TUPLE_1
#135860
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-134584: Eliminate redundant refcounting from _CALL_TUPLE_1
#135860
Conversation
f99ffdd to
c7c4873
Compare
|
Nice! That was fast. This PR LGTM, but we need to wait will Mark merges his register allocation PR to get the benefits of this work. See my other comment here #135818 (comment) |
|
Cool. I see an ~5% improvement in this code: Considering only one reference count was eliminated, this is good news for everything else that will remove more! |
|
@markshannon sorry can you please check if this is correct? I think the ERROR_IF might be misplaced, sorry! |
|
the generated code looks correct but I wonder if the C analyzer treats it any differently |
So this seems to be the case. If you look at the error case, it leaks a reference on exit, as it doesn't decref/close it. Sorry I have to revert this PR, or provide a fix. So sorry again! |
Yes, this leaks |
|
I confirm that this change introduced a memory leak in the following tests: Example AMD64 Fedora Stable Refleaks 3.x: https://buildbot.python.org/#/builders/320/builds/3425 |
|
@Fidget-Spinner wrote #142604 to fix the leak. |
|
Sorry about that! Thanks @Fidget-Spinner 🙏 |
He wrote another change to fix the leak: #142620. |
Uh oh!
There was an error while loading. Please reload this page.