classification
Title: Fold compare operators on constants (peephole)
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: Demur Rumed, amper, benjamin.peterson, brett.cannon, georg.brandl, ncoghlan, rhettinger, vstinner, yselivanov
Priority: low Keywords: patch

Created on 2016-04-09 12:52 by amper, last changed 2016-04-10 14:16 by Demur Rumed. This issue is now closed.

Files
File name Uploaded Description Edit
peephole_compareops.patch amper, 2016-04-09 12:52 Fold compare operators on constants (peephole) review
Messages (8)
msg263089 - (view) Author: Alexander Marshalov (amper) * Date: 2016-04-09 12:52
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
msg263092 - (view) Author: Demur Rumed (Demur Rumed) * Date: 2016-04-09 13:15
Do you have any numbers on how common constant comparisons are in real code?
msg263093 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-09 13:17
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.
msg263095 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-09 13:19
> 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:

* https://faster-cpython.readthedocs.org/optimizations.html#constant-propagation
* https://fatoptimizer.readthedocs.org/en/latest/optimizations.html#constant-propagation

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).
msg263118 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-04-10 00:19
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.
msg263122 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-04-10 02:48
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:

* Dict versioning: https://www.python.org/dev/peps/pep-0509/
* Function specialisation: https://www.python.org/dev/peps/pep-0510/
* Code transformers: https://www.python.org/dev/peps/pep-0511/
msg263124 - (view) Author: Alexander Marshalov (amper) * Date: 2016-04-10 05:08
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.
msg263142 - (view) Author: Demur Rumed (Demur Rumed) * Date: 2016-04-10 14:16
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>
> _______________________________________
>
History
Date User Action Args
2016-04-10 14:16:11Demur Rumedsetmessages: + msg263142
2016-04-10 05:08:29ampersetmessages: + msg263124
2016-04-10 02:48:52ncoghlansetstatus: open -> closed
resolution: rejected
messages: + msg263122

stage: patch review -> resolved
2016-04-10 00:19:43rhettingersetpriority: normal -> low
nosy: + rhettinger
messages: + msg263118

2016-04-09 13:19:14vstinnersetmessages: + msg263095
2016-04-09 13:17:00vstinnersetmessages: + msg263093
2016-04-09 13:15:16Demur Rumedsetnosy: + Demur Rumed
messages: + msg263092
2016-04-09 12:56:10SilentGhostsetnosy: + brett.cannon, georg.brandl, ncoghlan, vstinner, benjamin.peterson, yselivanov

stage: patch review
2016-04-09 12:52:24ampercreate