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

Fold compare operators on constants (peephole) #70909

Closed
Amper mannequin opened this issue Apr 9, 2016 · 8 comments
Closed

Fold compare operators on constants (peephole) #70909

Amper mannequin opened this issue Apr 9, 2016 · 8 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@Amper
Copy link
Mannequin

Amper mannequin commented Apr 9, 2016

BPO 26722
Nosy @brettcannon, @birkenfeld, @rhettinger, @ncoghlan, @vstinner, @benjaminp, @1st1, @serprex, @Amper
Files
  • peephole_compareops.patch: Fold compare operators on constants (peephole)
  • 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 2016-04-10.02:48:52.277>
    created_at = <Date 2016-04-09.12:52:24.599>
    labels = ['interpreter-core', 'type-feature']
    title = 'Fold compare operators on constants (peephole)'
    updated_at = <Date 2016-04-10.14:16:11.147>
    user = 'https://github.com/Amper'

    bugs.python.org fields:

    activity = <Date 2016-04-10.14:16:11.147>
    actor = 'Demur Rumed'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-04-10.02:48:52.277>
    closer = 'ncoghlan'
    components = ['Interpreter Core']
    creation = <Date 2016-04-09.12:52:24.599>
    creator = 'amper'
    dependencies = []
    files = ['42414']
    hgrepos = []
    issue_num = 26722
    keywords = ['patch']
    message_count = 8.0
    messages = ['263089', '263092', '263093', '263095', '263118', '263122', '263124', '263142']
    nosy_count = 9.0
    nosy_names = ['brett.cannon', 'georg.brandl', 'rhettinger', 'ncoghlan', 'vstinner', 'benjamin.peterson', 'yselivanov', 'Demur Rumed', 'amper']
    pr_nums = []
    priority = 'low'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue26722'
    versions = ['Python 3.6']

    @Amper
    Copy link
    Mannequin Author

    Amper mannequin commented Apr 9, 2016

    Missed peephole optimization:
    1 > 2 -> False
    3 < 4 -> True
    5 == 6 -> False
    6 != 7 -> True
    7 >= 8 -> False
    8 <= 9 -> True
    10 is 11 -> False
    12 is not 13 -> True
    14 in (15, 16, 17) -> False
    18 not in (19, 20, 21) -> True

    @Amper Amper mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Apr 9, 2016
    @serprex
    Copy link
    Mannequin

    serprex mannequin commented Apr 9, 2016

    Do you have any numbers on how common constant comparisons are in real code?

    @vstinner
    Copy link
    Member

    vstinner commented Apr 9, 2016

    Hi, it looks like the author of the peephole optimizer is Raymond Hettinger and he doesn't look to want to handle too many cases, he prefers to keep the code simple.

    FYI I reimplemented recently the peephole optimizer in pure Python as part of the bytecode project:
    https://bytecode.readthedocs.org/en/latest/peephole.html

    I didn't write it to replace the C implementation, it was more a tool to discuss modifying bytecode (when discussing the PEP-511).

    More generally, there is an ongoging discussion of rewriting the peephole optimizer to work on the AST rather than working on the Python code. The FAT Python implements that in pure Python:
    https://faster-cpython.readthedocs.org/fat_python.html

    FAT Python is more than a peephole optimizer, it's more a framework to implement more optimizations. Well, take a look.

    @vstinner
    Copy link
    Member

    vstinner commented Apr 9, 2016

    Do you have any numbers on how common constant comparisons are in real code?

    In my experience, it almost never occur in real application. But it's common when you start with a constant propogation optimization:

    If you extend constant propagation to things like os.name, it's even more common, but it requires to specialize the code, to disable optimization if os.name is modified (that's one feature provided by FAT Python).

    @rhettinger
    Copy link
    Contributor

    AFAICT, the cases the OP listed would be rarely found in real code.

    Victor is correct is saying that we want to limit the scope of the peepholer to the most useful cases.

    He is also correct in saying that we've long desired constant folding to be moved upstream to the AST and don't want to go further down the path of doing more work at the bytecode level. Ideally, the only optimizations at the peephole level would be limited to rejuggling opcodes into cheaper execution sequences without regard to higher level object semantics.

    I recommend rejecting this and suggesting that more effort be focused on the AST optimizations.

    @ncoghlan
    Copy link
    Contributor

    Nice work on getting this running, Alexander. However, as Victor and Raymond noted, we're looking to head in a different direction with our bytecode optimisation efforts, which is to better support pluggable AST level optimisations that can make more assumptions about the runtime environment.

    If this is a topic you'd like to explore further, then in addition to Victor's FAT Python project, you may also be interested in his proposals to add some supporting infrastructure for that to Python 3.6:

    @Amper
    Copy link
    Mannequin Author

    Amper mannequin commented Apr 10, 2016

    Hi all, this is my first patch to Python.
    I'm interested in the performance of python code, I even worked on the development of the static optimizer based on modifications of the AST.
    I had a few ideas for improving peepholer (for example, the expression "x, y = 1, 2" according to my benchmarks is about 7-11% slower than the expression "x = 1; y = 2", this can be fixed by using a peepholer), but I already understood that it is not necessary to do it.
    Thanks for the clarification, I will continue to work towards AST-optimizations.

    @serprex
    Copy link
    Mannequin

    serprex mannequin commented Apr 10, 2016

    I submitted a patch years ago that addressses the ''x,y = 1, 2' case:
    http://bugs.python.org/issue10648 & it was not met with enthusiasm

    2016-04-10 5:08 GMT+00:00 Alexander Marshalov <report@bugs.python.org>:

    Alexander Marshalov added the comment:

    Hi all, this is my first patch to Python.
    I'm interested in the performance of python code, I even worked on the
    development of the static optimizer based on modifications of the AST.
    I had a few ideas for improving peepholer (for example, the expression "x,
    y = 1, 2" according to my benchmarks is about 7-11% slower than the
    expression "x = 1; y = 2", this can be fixed by using a peepholer), but I
    already understood that it is not necessary to do it.
    Thanks for the clarification, I will continue to work towards
    AST-optimizations.

    ----------


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue26722\>


    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants