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
Simplify "with"-related opcodes #77130
Comments
There are some issues with "with"-related opcodes. All other opcodes has constant stack effect for particular control flow. For example FOR_ITER always has the stack effect 1 if not jump (pushes the next item) and -1 if jumps (pops the iterator). The only exceptions are WITH_CLEANUP_START which pushes 1 or 2 values depending on TOS, and WITH_CLEANUP_FINISH which pops 2 or 3 values depending on values pushed by preceding WITH_CLEANUP_START. This breaks consistency and may make debugging harder. WITH_CLEANUP_START duplicates a one of values on the stack without good reasons. Even the comment in the initial commit exposed uncertainty in this. The proposed PR simplifies WITH_CLEANUP_START and WITH_CLEANUP_FINISH. They will be now executed only when the exception is raised. In normal case they will be replaced with calling a function
WITH_CLEANUP_FINISH will be merged with the following END_FINALLY. This PR is inspired by PR 5112 by Mark Shannon, but Mark goes further. In addition to simplifying the implementation and the mental model, the PR adds a tiny bit of performance gain. $ ./python -m perf timeit -s 'class CM:' -s ' def __enter__(s): pass' -s ' def __exit__(*args): pass' -s 'cm = CM()' -- 'with cm: pass' Unpatched: Mean +- std dev: 227 ns +- 6 ns |
This will not solve bpo-29988 but will open a way for solving it at least for synchronous "with" (swap POP_BLOCK and the following LOAD_CONST and disable interrupting after POP_BLOCK). |
Updated PR seems fixes bpo-29988 for synchronous "with". |
Thank you for your PR Mark. The main difference between PR 5883 and PR 5112 is that in PR 5883 the pair of old WITH_CLEANUP_FINISH and END_FINALLY are replaced with a single new WITH_CLEANUP_FINISH, and in PR 5112 it is replaced with a sequence of 7 opcodes including a new opcode RERAISE.
L: This doesn't affect a performance in normal case because this code is executed only when an exception has been raised (an in that case the performance is less important). And seems this doesn't introduce new race conditions. The number of opcodes is the same in both PRs. The implementation of RERAISE in ceval.c in PR 5112 is a tiny bit simpler than the implementation of WITH_CLEANUP_FINISH in PR 5883. But the generated bytecode and the compiler are a tiny bit simpler in PR 5883. If RERAISE be used for other purposes besides implementing a "with" statement, it would be a great advantage. For now both approaches look to me not having significant advantages or disadvantages against the other one. Does anybody have preferences? |
I intend to reuse RERAISE to implement the exceptional case for a finally block. Something like: SETUP_FINALLY final |
There are problems with the f_lineno setter when duplicate a finally body. The duplication works for "with" only because the cleanup code for "with" doesn't correspond any line number. |
It is fiddly to get the frame-setlineno code right for duplicated catch blocks, but it is far from impossible. |
Should this be closed now that PR6641 has been merged? |
Sure. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: