I recall several occasions where the peephole optimizer was subtly buggy -- on one occasion the bug remained undetected for at least a whole release cycle. (Sorry, I can't recall the details.) In fact, the bug reported in http://bugs.python.org/issue11244 is another example of how subtle the peephole optimizer is. Adding more complexity to it just increases the chances that it will be broken by seemingly unrelated changes.
As far as Antoine's commit goes, it looks like it went in hastily, without review, and before any discussion. Clearly it *is* controversial, and controversial fixes always need to be held back until at least rough consensus is reached or until a BDFL decision. Also it is not a fix to the bug reported in the issue, so the Misc/NEWS text is misleading. A much simpler fix was proposed in the bug and even approved by Antoine. (One note on the patch: it allocates an extra stack which is dynamically grown; but there is no unittest to exercise the stack-growing code. This note does constitute a full review.) I have always felt uncomfortable with *any* kind of optimization -- whether AST-based or bytecode-based. I feel the cost in code complexity is pretty high and in most cases the optimization is not worth the effort. Also I don't see the point in optimizing expressions like "3 * 4 * 5" in Python. In C, this type of thing occurs frequently due to macro expansion and the like, but in Python your code usually looks pretty silly if you write that. So this is basically a solution to a non-problem. The only two places where I personally see a need for compile-time optimization are unary minus (since there is no way to write negative constants otherwise) and string literal concatenation (which is a common way to break long string literals). Now, should Antoine's checkin be rolled back? I think the case is in Antoine's court to convince us that his patch is correct *and* to help at least one or two others feel comfortable they understand how it works -- if only Antoine understands it, it is too complex. If Antoine does not succeed with this task he should roll it back. Or he can choose to roll it back immediately, and then we can continue the discussion under a lot less pressure, and come to a conclusion amiably. --Guido On Fri, Mar 11, 2011 at 10:01 PM, Benjamin Peterson <benja...@python.org> wrote: > 2011/3/11 Raymond Hettinger <raymond.hettin...@gmail.com>: >> Today, there was a significant check-in to the peephole optimizer that I >> think should be reverted: >> http://hg.python.org/cpython/rev/14205d0fee45/ >> The peephole optimizer pre-dated the introduction of the abstract syntax >> tree. Now that we have an AST, the preferred way to implement additional >> optimizations is with AST manipulation, upstream from code generation. This >> approach is faster and much more reliable than the more brittle approach >> of disassembling, analyzing, and rewriting the bytecode created by the >> compiler. > > The problem is no such AST optimizer exists. It's one thing avoid > changing old code because an improved version is in the works or > available (say argparse in lieu of getopt) and quite another when no > replacement code exists. At the moment, there is little reason not to > accept progressive improvements (with sights on overall design as > usual) to the code. > > IMO, Ast or not ast statically optimizing python in any meaningful way > is a impossible task anyway. So, a better solution would be to just > rip the thing out. > >> There should no longer be any significant changes to the existing >> optimizer. It took a good deal of care to get it right in the first place. >> The code is stable and has been proven successful by many years of >> deployment. The nature of the peephole optimizer is that changes to it are >> high risk, that the procedure is brittle, and that it is easily mucked-up in >> ways that are hard to detect. Accordingly, we've kept to simple conservative >> transformations and have assiduously avoided more complex (and interesting) >> optimizations. > > Can you point to examples where changes introduced long-term > instability? ISTM, that the peepholer is one of the better tested > parts of the interpreter if not directly from its own tests, from > running the entire stdlib tests. > >> FWIW, I've been the maintainer (as well as the original designer and >> implementer) of the peephole optimizer from the beginning; however, today's >> committer did not accept my advice to be very conservative with changes to >> it and to strongly prefer AST transformations instead. Please consider >> reverting this change. > > > > -- > Regards, > Benjamin > _______________________________________________ > Python-Dev mailing list > Python-Dev@python.org > http://mail.python.org/mailman/listinfo/python-dev > Unsubscribe: > http://mail.python.org/mailman/options/python-dev/guido%40python.org > -- --Guido van Rossum (python.org/~guido) _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
