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

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

Merged
merged 45 commits into from
May 7, 2021

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Apr 29, 2021

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 markshannon changed the title "Zero cost" exception handling bpo-40222: "Zero cost" exception handling Apr 30, 2021
@markshannon markshannon removed the request for review from a team May 6, 2021 11:18
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 6, 2021
@bedevere-bot
Copy link

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

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 6, 2021
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 7, 2021
@bedevere-bot
Copy link

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

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 7, 2021
@markshannon
Copy link
Member Author

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 markshannon merged commit adcd220 into python:main May 7, 2021
@markshannon markshannon deleted the zero-cost-except branch May 7, 2021 14:19
@pablogsal
Copy link
Member

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.

Comment on lines 118 to +126
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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This C-API change seems worth mentioning somewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -308,8 +311,33 @@ def _get_name_info(name_index, name_list):
return argval, argrepr


def parse_varint(iterator):
b = next(iterator)
val = b & 63
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, how does this work? I guess the constant numbers are about 64-bit?

would love to get your explanation, thanks. @markshannon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants