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

Move jumps optimization from the peepholer to the compiler #76658

Closed
serhiy-storchaka opened this issue Jan 1, 2018 · 11 comments
Closed

Move jumps optimization from the peepholer to the compiler #76658

serhiy-storchaka opened this issue Jan 1, 2018 · 11 comments
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@serhiy-storchaka
Copy link
Member

BPO 32477
Nosy @brettcannon, @rhettinger, @ncoghlan, @pitrou, @benjaminp, @methane, @markshannon, @serhiy-storchaka, @1st1
PRs
  • bpo-32477: Move jumps optimization from the peepholer to the compiler. #5077
  • bpo-1875, bpo-32477: Raise SyntaxError in invalid blocks that will be optimized away. #14116
  • 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:

    assignee = None
    closed_at = <Date 2021-03-02.10:41:02.793>
    created_at = <Date 2018-01-01.22:03:27.686>
    labels = ['interpreter-core', '3.9', 'performance']
    title = 'Move jumps optimization from the peepholer to the compiler'
    updated_at = <Date 2021-03-02.10:41:02.789>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2021-03-02.10:41:02.789>
    actor = 'Mark.Shannon'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-03-02.10:41:02.793>
    closer = 'Mark.Shannon'
    components = ['Interpreter Core']
    creation = <Date 2018-01-01.22:03:27.686>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32477
    keywords = ['patch']
    message_count = 11.0
    messages = ['309351', '309432', '309439', '309440', '309445', '309446', '330929', '337397', '345109', '345696', '387906']
    nosy_count = 9.0
    nosy_names = ['brett.cannon', 'rhettinger', 'ncoghlan', 'pitrou', 'benjamin.peterson', 'methane', 'Mark.Shannon', 'serhiy.storchaka', 'yselivanov']
    pr_nums = ['5077', '14116']
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue32477'
    versions = ['Python 3.9']

    @serhiy-storchaka
    Copy link
    Member Author

    The proposed patch moves jumps optimization from the peepholer to the compiler. The optimization is performed for lists of instructions before generating concrete bytecode and is not restricted by the size of bytecode instructions. It allows to optimize more long chains of jumps.

    This is a step to getting rid of the peepholer.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Jan 1, 2018
    @rhettinger
    Copy link
    Contributor

    This is a step to getting rid of the peepholer.

    There is no goal to get rid of the peepholer optimizer. Please don't invent this as a requirement.

    If some optimization are more easily performed upstream, then go ahead and move them upstream. The goal is to have a net reduction in complexity, not just the elimination of peephole.c just because you decided it should disappear.

    @serhiy-storchaka
    Copy link
    Member Author

    Actually moving any of remaining optimizations (including two optimizations that are moved by this issue and two other that are left) slightly increases the complexity. But instead the optimizations become more general. At the end, after moving all optimizations, we could drop the auxiliary code in the peepholer (building the table of basic blocks, removing NOPs, fixing up jump targets) and the net complexity will be reduced. This can also reduce the time and memory consumption of compilation.

    Thus getting rid of the peepholer optimizer working with the bytecode is my goal. But some peephole optimizations will work with the intermediate fixed-size representations used by the compiler, before generating the concrete bytecode.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 3, 2018

    But some peephole optimizations will work with the intermediate fixed-size representations used by the compiler, before generating the concrete bytecode.

    Then perhaps the goal should be to make the peephole operate on that intermediate representation?

    I think it's valuable to have a separate C module for optimizations, instead of cramming them in compiler.c.

    @serhiy-storchaka
    Copy link
    Member Author

    I think it's valuable to have a separate C module for optimizations, instead of cramming them in compiler.c.

    This would require major rewriting. PR 5077 adds just two functions in compile.c. But they use 4 structures defined locally in this file. If move these two functions in a separate file we will need to move these 4 structures and related definitions to the common header file. And maybe it will be in vain if once all these optimizations will be moved to other parts of the compiler. Some of jumps optimization already are performed on instructions emitting stage. Dead code elimination perhaps could be performed at the stage of assembling the bytecode from basic blocks. Constant tuples folding could be implemented by at least three ways besides the current implementation, I'm testing what of them is the simplest.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 3, 2018

    Well, on the other hand, the change you're proposing is hardly necessary, unless it actually demonstrates a noticeable improvement on some workload. So perhaps we should leave the peepholer alone until someone has a better idea.

    @serhiy-storchaka
    Copy link
    Member Author

    Simplified PR 5077. It uses now NEXT_BLOCK() to guarantee that a new block is always used after jumps. NEXT_BLOCK() was defined, but not used before.

    bpo-35193 and bpo-9566 demonstrate that the code of peephole.c is complex and error-prone. Moving it to the upper level makes it more error-proof and general.

    @serhiy-storchaka serhiy-storchaka added 3.8 only security fixes and removed 3.7 (EOL) end of life labels Dec 3, 2018
    @serhiy-storchaka
    Copy link
    Member Author

    After analyzing the difference between bytecodes generated by the current peepholer and the new optimizer I have found that he peepholer do not optimizes jumps in case of some multiline expressions. For example:

    [x
    for x in a if x]

    1 0 BUILD_LIST 0
    2 LOAD_FAST 0 (.0)
    >> 4 FOR_ITER 12 (to 18)

    2 6 STORE_FAST 1 (x)
    8 LOAD_FAST 1 (x)
    10 POP_JUMP_IF_FALSE 16

    1 12 LOAD_FAST 1 (x)
    14 LIST_APPEND 2
    >> 16 JUMP_ABSOLUTE 4
    >> 18 RETURN_VALUE

    if x:
        if (y and
            z):
            foo()
    else:
        bar()

    1 0 LOAD_NAME 0 (x)
    2 POP_JUMP_IF_FALSE 20

    2 4 LOAD_NAME 1 (y)
    6 POP_JUMP_IF_FALSE 18

    3 8 LOAD_NAME 2 (z)

    2 10 POP_JUMP_IF_FALSE 18

    4 12 LOAD_NAME 3 (foo)
    14 CALL_FUNCTION 0
    16 POP_TOP
    >> 18 JUMP_FORWARD 6 (to 26)

    6 >> 20 LOAD_NAME 4 (bar)
    22 CALL_FUNCTION 0
    24 POP_TOP
    >> 26 LOAD_CONST 0 (None)
    28 RETURN_VALUE

    It preserves jumps to jump instructions. I do not know the cause. All works with single-line expressions.

    The new optimizer does not have this bug.

    @serhiy-storchaka
    Copy link
    Member Author

    Opened bpo-37213 for the regression in the peepholer.

    @serhiy-storchaka
    Copy link
    Member Author

    PR 14116 is based on the part of PR 5077. It serves three functions:

    • Finally fixes bpo-1875.

    • Simplifies the code. Removes special cases for "if 0" and "while 1". 31 insertions, 80 deletions in Python/compile.c + Python/peephole.c.

    • However such optimization is now applied in more general cases, not just for "if" and "while".

    @serhiy-storchaka serhiy-storchaka added 3.9 only security fixes and removed 3.8 only security fixes labels Jun 15, 2019
    @markshannon
    Copy link
    Member

    This has rendered obsolete by removal of the peephole optimizer in https://bugs.python.org/issue41323

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants