X Tutup
The Wayback Machine - https://web.archive.org/web/20211024085024/https://github.com/python/cpython/pull/25729
Skip to content
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

Jump to bottom

bpo-40222: "Zero cost" exception handling #25729

Merged
merged 45 commits into from May 7, 2021
Merged

bpo-40222: "Zero cost" exception handling #25729

merged 45 commits into from May 7, 2021

Conversation

@markshannon
Copy link
Contributor

@markshannon markshannon commented Apr 29, 2021 •
edited

For example, this function:

def f():
    try:
        g(0)
    except:
        return "fail"

compiles as follows on main:

  2           0 SETUP_FINALLY            7 (to 16)

  3           2 LOAD_GLOBAL              0 (g)
              4 LOAD_CONST               1 (0)
              6 CALL_FUNCTION            1
              8 POP_TOP
             10 POP_BLOCK
             12 LOAD_CONST               0 (None)
             14 RETURN_VALUE

  4     >>   16 POP_TOP
             18 POP_TOP
             20 POP_TOP

  5          22 POP_EXCEPT
             24 LOAD_CONST               3 ('fail')
             26 RETURN_VALUE

with this PR it compiles as follows:

  2           0 NOP

  3           2 LOAD_GLOBAL              0 (g)
              4 LOAD_CONST               1 (0)
              6 CALL_FUNCTION            1
              8 POP_TOP
             10 LOAD_CONST               0 (None)
             12 RETURN_VALUE
        >>   14 PUSH_EXC_INFO

  4          16 POP_TOP
             18 POP_TOP
             20 POP_TOP

  5          22 POP_EXCEPT
             24 LOAD_CONST               2 ('fail')
             26 RETURN_VALUE
        >>   28 POP_EXCEPT_AND_RERAISE
ExceptionTable:
  2 to 8 -> 14 [0]
  14 to 20 -> 28 [3] lasti

This is not quite zero-cost at the moment, as it leaves a NOPs for each try, and possibly a few other.

Removing NOPs that are on a line by themselves will require changes to the line number table, which has nothing to do with exception handling and I don't want to mix the two PRs.

Although the code is slightly longer overall, there is less work done on the non-exceptional path, one NOP versus a SETUP_FINALLY and POP_BLOCK.

Not only is there a reduction in work done in the bytecode, but in the size of frames is reduced a lot:

def frame_size():
    return sys.getsizeof(sys._getframe())

On master:

>>> frame_size()
400

with this PR:

>>> frame_size()
152

https://bugs.python.org/issue40222
markshannon added 16 commits Apr 19, 2021
@markshannon
Add SETUP_CLEANUP bytecode to track lasti for re-raising exceptions i…
a414fa6 
…n cleanup code.
@markshannon
Move some handling of exceptions to bytecode from unwind code.
902690d
@markshannon
Move setting up except block out of unwind code into bytecode.
21646b1
@markshannon
Move cleanup code for try-finally and try-except in bytecode.
9c9575a
@markshannon
Move cleanup code for with and sync with into bytecode.
3be89db
@markshannon
Simplify exit code for async for loops.
cf664f7
@markshannon
Remove SETUP_EXCEPT opcode.
722b722
@markshannon
Remove EXCEPT_HANDLER as a type of exception handling block.
caa355f
@markshannon
Factor out all block pops and pushes into simple opcodes.
4c21ddd
@markshannon
Generate exception handling table
6a8d90c
@markshannon
Parse and verify exception table when unwinding exceptions.
9115cea
@markshannon
Update test to account for new code object constructor.
350334c
@markshannon
Push lasti to stack when cleaning up exception handlers, and use it t…
d6da9cb 
…o restore when re-raising.
@markshannon
Remove block stack from frames add convert SETUP/POP_BLOCK instructio…
a6b9292 
…ns into NOPs.
@markshannon
Make dis understand the exception table.
9d0a6c1
@markshannon
Merge branch 'master' into zero-cost-except
ca482d0
@markshannon markshannon requested a review from tiran as a code owner Apr 29, 2021
markshannon added 4 commits Apr 30, 2021
@markshannon
Set line numbers at start of exception handlers in try-except and try…
4629a97 
…-fianlly correctly.
@markshannon
Handle exceptions in trace functions correctly.
58b4a5a
@markshannon
Fix syntax error test.
e7a9221
@markshannon
Get frame.setlineno working again.
062ffa4
@markshannon markshannon changed the title "Zero cost" exception handling bpo-40222: "Zero cost" exception handling Apr 30, 2021
markshannon added 4 commits Apr 30, 2021
@markshannon
Merge branch 'master' into zero-cost-except
71d95c1
@markshannon
Remove debugging prints
e5c2c3e
@markshannon
Fix test_dis after merge
7571e90
@markshannon
Layout frameobject in most compact form.
812e78f
@ericsnowcurrently ericsnowcurrently self-requested a review May 6, 2021
ericsnowcurrently
ericsnowcurrently requested changes May 6, 2021
View changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment •
edited

Mostly LGTM, but with a bunch of small comments to sort out first

My only big request would be a bit more explanation somewhere of how the zero-cost exception handling works. I didn't have much context when reading through frameobject.c, as you may note by a number of "why did this change" comments. 🙂

Also, have you been able to benchmark this? My guess is that it will be faster and leaner generally, but a little slower when handling exceptions.

Include/cpython/frameobject.h Show resolved Hide resolved
Lib/importlib/_bootstrap_external.py Show resolved Hide resolved
Misc/NEWS.d/next/Core and Builtins/2021-04-30-15-48-36.bpo-40222.j3VxeX.rst Outdated Show resolved Hide resolved
Misc/NEWS.d/next/Core and Builtins/2021-04-30-15-48-36.bpo-40222.j3VxeX.rst Outdated Show resolved Hide resolved
Include/cpython/frameobject.h
PyAPI_FUNC(void) PyFrame_BlockSetup(PyFrameObject *, int, int, int);
PyAPI_FUNC(PyTryBlock *) PyFrame_BlockPop(PyFrameObject *);
Copy link
Member

@ericsnowcurrently ericsnowcurrently May 6, 2021

Given that these were part of the public C-API, it would probably be worth adding a note to the What's New doc about their removal. I suppose the same may be true of f_blockstack, though it's less likely that matters.

Python/compile.c Show resolved Hide resolved
Python/compile.c Outdated Show resolved Hide resolved
Python/compile.c
if (!handler->v.ExceptHandler.type && i < n-1) {
return compiler_error(c, "default 'except:' must be last");
}
Copy link
Member

@ericsnowcurrently ericsnowcurrently May 6, 2021

Is this another case where you happened to notice it was incorrect, or is this change due to this PR?

Copy link
Contributor Author

@markshannon markshannon May 6, 2021

I think this was incorrect before. I'll see if it makes sense to apply it to 3.10b.

Python/compile.c Outdated Show resolved Hide resolved
Python/compile.c Show resolved Hide resolved
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented May 6, 2021

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be put in the comfy chair!

markshannon added 3 commits May 6, 2021
@markshannon
Assorted clarifications.
43c199d
@markshannon
Rename frame.setlineno assistant code to clarify that we now model th…
630e755 
…e evaluation stack, not the block stack.
@markshannon
Revert mistaken change.
6333f6c
@markshannon markshannon removed the request for review from May 6, 2021
@markshannon
Update Include/opcode.h
e1d6e1e
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented May 6, 2021

🤖 New build scheduled with the buildbot fleet by @markshannon for commit e1d6e1e 🤖

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

markshannon added 5 commits May 6, 2021
@markshannon
Extend notes on exception handling.
73b600a
@markshannon
Fix refleaks
f263ee2
@markshannon
Merge branch 'main' into zero-cost-except
2f31c84
@markshannon
Document removal frame push/pop C-API functions.
0c3fd95
@markshannon
Update importlib
b92ada2
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented May 7, 2021

🤖 New build scheduled with the buildbot fleet by @markshannon for commit b92ada2 🤖

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

@markshannon
Copy link
Contributor Author

@markshannon markshannon commented May 7, 2021

The failure on buildbot/AMD64 Arch Linux Asan PR is a pre-existing failure.
The failure on buildbot/PPC64LE Fedora Stable seems to be some sort of race condition in concurrent_futures

@markshannon
Clarify exception handling notes
1878117
@markshannon markshannon merged commit adcd220 into python:main May 7, 2021
12 checks passed
@markshannon markshannon deleted the zero-cost-except branch May 7, 2021
@pablogsal
Copy link
Member

@pablogsal pablogsal commented May 10, 2021

The failure on buildbot/AMD64 Arch Linux Asan PR is a pre-existing failure.
The failure on buildbot/PPC64LE Fedora Stable seems to be some sort of race condition in concurrent_futures

@markshannon Unfortunately the ASAN failure is legit. Check the BPO issue for more details.

scoder
scoder reviewed May 11, 2021
View changes
Include/cpython/code.h
PyAPI_FUNC(PyCodeObject *) PyCode_New(
int, int, int, int, int, PyObject *, PyObject *,
PyObject *, PyObject *, PyObject *, PyObject *,
PyObject *, PyObject *, int, PyObject *);
PyObject *, PyObject *, int, PyObject *, PyObject *);

PyAPI_FUNC(PyCodeObject *) PyCode_NewWithPosOnlyArgs(
int, int, int, int, int, int, PyObject *, PyObject *,
PyObject *, PyObject *, PyObject *, PyObject *,
PyObject *, PyObject *, int, PyObject *);
PyObject *, PyObject *, int, PyObject *, PyObject *);
Copy link
Contributor

@scoder scoder May 11, 2021

This C-API change seems worth mentioning somewhere.

Copy link
Member

@pablogsal pablogsal May 11, 2021

Check the discussion in https://bugs.python.org/issue40222

@tacaswell tacaswell mentioned this pull request May 14, 2021
Open
@markshannon markshannon mentioned this pull request May 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scoder scoder

@ericsnowcurrently ericsnowcurrently

@pablogsal pablogsal

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
6 participants
@markshannon @bedevere-bot @pablogsal @scoder @ericsnowcurrently @the-knights-who-say-ni
X Tutup