gh-134584: Cleanups for refcount elimination #136104 and #135860 #142604
Conversation
markshannon
left a comment
markshannon
left a comment
There was a problem hiding this comment.
Can you make a separate PRs for fixing the leak? I'd like to get that in ASAP.
For the larger changes:
- There is no need for
_POP_TOP_NOT_NULLas it still needs to check for immortal objects andNULLis immortal - The changes to the code generator need more explanation and maybe a new issue.
| assert(oparg == 1); | ||
| STAT_INC(CALL, hit); | ||
| PyObject *res_o = PySequence_Tuple(arg_o); | ||
| DEAD(null); |
There was a problem hiding this comment.
| DEAD(null); |
No need to declare null dead here as it will be handled by INPUTS_DEAD below.
There was a problem hiding this comment.
This is required so that ERROR_IF doesn't emit a useless close for it.
|
When you're done making the requested changes, leave the comment: |
I can't without a partial revert. The problem is that we don't allow live inputs at ERROR_IF. This PR simply changes the ERROR_IF behavior to close all live inputs. The rest of this PR to the cases generator is just moving stuff around if you take a closer look. I mainly had to expose the close_variable function as it was previously a nested one. |
|
Just for personal thought(correct me plz if I'm wrong) The root cause about this refleak is related with the wrong position about So the core point about this PR is the behavior change "raise an exception when here's live variable on TOS -> close all the live variable" So we can mark the stack |
|
@Zheaoli yes you're right |
4 participants
This cleans up PRs #136104 and #135860. Fixing a refleak as well.
Mostly just moving code around.