◐ Shell
reader mode source ↗
Skip to content

bpo-32949: Better bytecodes for "with" statement.#5112

Closed
markshannon wants to merge 6 commits into
python:masterfrom
markshannon:better-with-bytecodes
Closed

bpo-32949: Better bytecodes for "with" statement.#5112
markshannon wants to merge 6 commits into
python:masterfrom
markshannon:better-with-bytecodes

Conversation

@markshannon

@markshannon markshannon commented Jan 6, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

This PR cleans up the interpreter by generating two different code paths for exiting a with statement. One for normal exits and another for the exceptional case.

For this function:

def foo():
     with a:
        b

The bytecode generated changes from:

 2           0 LOAD_GLOBAL              0 (a)
             2 SETUP_WITH              10 (to 14)
             4 POP_TOP

 3           6 LOAD_GLOBAL              1 (b)
             8 POP_TOP
             10 POP_BLOCK
             12 BEGIN_FINALLY
        >>   14 WITH_CLEANUP_START
             16 WITH_CLEANUP_FINISH
             18 END_FINALLY
             20 LOAD_CONST               0 (None)
             22 RETURN_VALUE

to

 2           0 LOAD_GLOBAL              0 (a)
             2 SETUP_WITH    20 (to 24)
             4 POP_TOP
 3           6 LOAD_GLOBAL              1 (b)
              8 POP_TOP
             10 POP_BLOCK
             12 LOAD_CONST               0 (None)
             14 DUP_TOP
             16 DUP_TOP
             18 CALL_FUNCTION            3
             20 POP_TOP
             22 JUMP_FORWARD            16 (to 40)
        >>   24 WITH_EXCEPT_START
             26 POP_JUMP_IF_TRUE        30
             28 RERAISE
        >>   30 POP_TOP
             32 POP_TOP
             34 POP_TOP
             36 POP_EXCEPT
             38 POP_TOP
        >>   40 LOAD_CONST               0 (None)
             42 RETURN_VALUE

Although this superficially appears worse, consider the non-exceptional path, after the POP_BLOCK bytecode.
before:

             12 BEGIN_FINALLY
        >>   14 WITH_CLEANUP_START
             16 WITH_CLEANUP_FINISH
             18 END_FINALLY

after:

             12 LOAD_CONST               0 (None)
             14 DUP_TOP
             16 DUP_TOP
             18 CALL_FUNCTION            3
             20 POP_TOP
             22 JUMP_FORWARD            16 (to 40)

we have replaced several complex and inefficient bytecodes with a few more simpler and faster bytecodes.

https://bugs.python.org/issue32949

@markshannon markshannon requested a review from 1st1 as a code owner January 6, 2018 15:53
@markshannon markshannon mentioned this pull request Jan 6, 2018
Closed
@serhiy-storchaka

serhiy-storchaka commented Jan 7, 2018

Copy link
Copy Markdown
Member

Please open an issue for the discussion. I was going to make different changes in with-related opcodes after merging #5006, but your approach may be better. Needed some microbenchmarks for comparing implementations.

You can simplify and speed up the bytecode (get rid of LOAD_CONST and 2 DUP_TOPs) if make WITH_SETUP pushing 3 Nones after pushing a block.

@serhiy-storchaka serhiy-storchaka mentioned this pull request Feb 13, 2018
Merged
@markshannon markshannon force-pushed the better-with-bytecodes branch from 445f25b to e9f7689 Compare March 11, 2018 19:24
@markshannon markshannon changed the title Better bytecodes for "with" statement. Mar 13, 2018
@serhiy-storchaka

serhiy-storchaka commented Apr 4, 2018

Copy link
Copy Markdown
Member

@markshannon, are you aware that tests are failed with your current code? And merging with master will add failures in new tests.

@markshannon

markshannon commented Apr 29, 2018

Copy link
Copy Markdown
Member Author

I've decided that generating new bytecodes for the with statement is best done at the same time as for try-finally. There is a lot of similarity between with and try-finally at the bytecode level.
See #6641 which replaces this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@1st1 1st1 Awaiting requested review from 1st1

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@markshannon @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot