Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
On Fri, Mar 11, 2011 at 9:53 PM, Alexander Belopolsky alexander.belopol...@gmail.com wrote: On Fri, Mar 11, 2011 at 9:28 PM, Raymond Hettinger raymond.hettin...@gmail.com wrote: Today, there was a significant check-in to the peephole optimizer that I think should be reverted: http://hg.python.org/cpython/rev/14205d0fee45/ +1 I was going to comment on the corresponding issue #11244 more or less supporting Raymond's arguments. When I wrote this, I was actually looking at the issue 11462. I now realize that #11462 was in fact closed as rejected. I am still confused, however, about [14205d0fee45]. The commit message refers to Issue #11244, but the comment does not match the subject of the tracker issue: Issue #11244: The peephole optimizer is now able to constant-fold arbitrarily complex expressions. This also fixes a 3.2 regression where operations involving negative numbers were not constant-folded. I think issue #11244 is just the second part: a 3.2 regression where operations involving negative numbers were not constant-folded. If this is the case, what is left to do for #11244? The bug reported in that issue seem to have been fixed: dis(lambda: (1,-2,3)) 1 0 LOAD_CONST 5 ((1, -2, 3)) 3 RETURN_VALUE ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
On Sat, Mar 12, 2011 at 1:08 PM, Raymond Hettinger
raymond.hettin...@gmail.com wrote:
I would like to withdraw my suggestion for the recursive constant folding
patch to be reverted.
So what is the status of peephole optimization now? Is it back under
active development? Let me quote a tracker comment that I posted two
years ago and go no response to (''-quote are from Raymond's
message):
Constant folding promotes more readable code: 24*60*60 is more obvious
than 86400, prefixing positive numbers with + leads to better visual
alignment, etc. Users should not be required to think twice about
which constant expressions are folded and which are not.
Here is another surprise with the current peepholer:
dis(lambda:1+2*3)
1 0 LOAD_CONST 0 (1)
3 LOAD_CONST 3 (6)
6 BINARY_ADD
7 RETURN_VALUE
dis(lambda:2*3+1)
1 0 LOAD_CONST 4 (7)
3 RETURN_VALUE
I have a fix in the works, but I will wait for your further comments
before submitting it.
More importantly, we decided that the peepholer is the wrong place to
do much of this work. Most of the peepholer is going to be migrated
up the chain, after the AST is generated, but before the opcodes are
generated. That is a faster, more reliable, and more general
approach.
I agree. Constant folding, is an interesting case because peepholer
has to duplicate a subset of eval logic. I wonder if the new approach
could eliminate that.
BTW, what is the rationale for leading +/- not being part of the
number literal? Unary +/- optimization seems mostly useful for the
simple +/-x case which could be handled by the tokenizer.
What is the timeline for that project? Maybe a comment should be
placed in peephole.c explaining that there is a plan to move
uptimization logic up the compilation chain and announcing a
moratorium on further peephole.c work. I am not the only one
submitting peephole optimizer patches recently.
http://bugs.python.org/issue2499#msg64638
I understand that the constant folding pass for AST is the subject of
http://bugs.python.org/issue1346238. The first patch in that issue is
more than four years old. Is that an indication that optimizing AST
is actually harder than optimizing bytecode?
___
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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
On 3/12/2011 12:39 AM, Eugene Toder wrote: You've got wishful thinking if you think a handful of tests can catch errors in code that sophisticated. Why limit yourself with a handful of tests? Python is widespread, there's*a lot* of code in Python. Unlike with libraries, any code you run tests the optimizer, so just run a lot of code. And, as I've said, write a test generator. As we're thinking about what the optimizer of the future should be, it would be great to have a way to turn it off completely. This would allow the tests to run test code both with and without the optimizer, and to verify that the two results were the same. Then we could automatically verify that the optimizer isn't changing semantics. BTW: I also believe it would be very useful to make the turn-off-the-optimizer switch available for users so they can run their code unoptimized for times when they are more interested in program analysis than in execution speed. See http://bugs.python.org/issue2506 (closed 3 years ago) that I filed with this request. --Ned. ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
2011/3/14 Alexander Belopolsky alexander.belopol...@gmail.com
On Sat, Mar 12, 2011 at 1:08 PM, Raymond Hettinger
raymond.hettin...@gmail.com wrote:
I would like to withdraw my suggestion for the recursive constant folding
patch to be reverted.
So what is the status of peephole optimization now? Is it back under
active development? Let me quote a tracker comment that I posted two
years ago and go no response to (''-quote are from Raymond's
message):
Constant folding promotes more readable code: 24*60*60 is more obvious
than 86400, prefixing positive numbers with + leads to better visual
alignment, etc. Users should not be required to think twice about
which constant expressions are folded and which are not.
Here is another surprise with the current peepholer:
dis(lambda:1+2*3)
1 0 LOAD_CONST 0 (1)
3 LOAD_CONST 3 (6)
6 BINARY_ADD
7 RETURN_VALUE
dis(lambda:2*3+1)
1 0 LOAD_CONST 4 (7)
3 RETURN_VALUE
I have a fix in the works, but I will wait for your further comments
before submitting it.
More importantly, we decided that the peepholer is the wrong place to
do much of this work. Most of the peepholer is going to be migrated
up the chain, after the AST is generated, but before the opcodes are
generated. That is a faster, more reliable, and more general
approach.
I agree. Constant folding, is an interesting case because peepholer
has to duplicate a subset of eval logic. I wonder if the new approach
could eliminate that.
I followed a different approach. Constant folding in WPython is made between
ASDL evalutation and AST building.
The idea is to intercept constant values and apply the operations
generating a new value instead of generating the classic AST node (a BinOp
for a binary operation, for example).
This way there's no need to parse the AST tree seeking for cases where to
apply the constant folding logic.
It's faster, because you don't need an additional pass through the AST:
you'll do it while building the AST...
It consumes less memory too, since you don't need to generate complex AST
nodes that must be discarded after applying the folding (which generates new
nodes). Think about a tuple of constant values, for example: you have to
generate a Tuple AST structure from the ASDL, then an AST constant folder
will generate the tuple. In WPython the tuple is generated immediately,
directly from the ADSL seq structure.
It's also efficient, since expressions such as 1 + 2 * 3 can be completely
folded generating 7, instead of 1 + 6 of the (classic) peepholer. That's
because, when parsing the ASDL structures, nodes are evaluated in respect of
operator precedence, so first we evaluate 2 * 3, which produces 6 applying
the folding, and then 1 + 6, which produces 7 in the end.
Cesare
___
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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
To give this a positive spin, here's a patch that implements constant folding on AST (it does few other optimizations on compiler data structures, so it replaces peephole over bytecode completely). http://bugs.python.org/issue11549 It passes make test, but of course more testing is needed. Comments are welcome. Eugene ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Hello, 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. Well, the regression stems from a commit in the ast code, not in the peepholer code (and it's only a regression as in misses an optimization opportunity, not produces bogus code). The peepholer is actually not very subtle, and that's why it missed the obvious optimization here. 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. I didn't get any comments against the patch before committing it, and I trusted Eugene's comment that the second patch was fine (Eugene proposed other, decent patches for the peepholer and thus seems acquainted with the code). Also, as Eugene pointed out, running the whole test suite in itself stresses the peepholer quite a bit if you take care of removing pyc files beforehand (which the buildbots do). Any further review is obviously welcome, as usual. 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. Well, the much simpler fix fixes a separate issue with -0; it is orthogonal to the original issue, and doesn't fix it (Eugene should probably have filed it it in a separate issue). The patch I committed, OTOH, *is* a fix to the original issue (there's even a test for that ;). The fact that the fix implies a more general improvement to the peepholer might be seen as an annoyance, or a benefit, depending on your point of view (although I'm not sure why a general improvement would be an annoyance). (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.) As Eugene answered, there is actually an unittest for that. 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. But then, as others pointed out, why don't we rip out the peepholer? If it's fragile and doesn't serve a purpose, it doesn't deserve staying. There is, by the way, recent history for adding optimizations to the peepholer, which was even approved by Raymond: http://bugs.python.org/issue6690 ... so I'm not sure why there is opposition to fixing a regression, when apparently new optimizations are uncontroversial. 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 original issue was about constant-folding of tuples containing negative numbers, which doesn't sound like an exotic situation. The obvious solution (enabling propagation of constant-folding) also entails constant-folding of arbitrarily complex expressions, which is why I have added tests for it, and suitably described it in the Misc/NEWS. Having more tests is always worthwhile IMO (even if we later choose to change them for whatever reason). 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. I really don't think the patch is complicated. It adds a stack to manage the current run of constants. This replaces the previous code where that run of constants was implicitly managed through the lastlc and cumlc variables and hand-computed indices into the bytecode; the new approach seems less clumsy to me. (Eugene apparently understood the patch, so hopefully I'm not alone :-)) Now, if a majority is in favour of rolling it back (backout in hg terminology), let's do it. That regression obviously isn't killing anyone. Should we ask for a vote? Regards Antoine. ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
FWIW, I'm -1 on backing out Antoine's patch. In addition to fixing the minor optimization regression, it makes the peepholer significantly more consistent in what it can and can't fold. One of the first times that I delved into the peepholer code was to try to understand why expressions like: 2 * 3 * 4 and 3**2 * 7 were constant folded, when 2 * (3 * 4) and 7 * 3**2 were not; with Antoine's patch, they're all folded. That both Antoine and Eugene are happy with the code gives me confidence in its correctness. I can also see the case for ripping out the peepholer entirely. But reverting Antoine's patch seems like a step backwards. Mark ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
On Sat, Mar 12, 2011 at 5:07 AM, Mark Dickinson dicki...@gmail.com wrote: I can also see the case for ripping out the peepholer entirely. But reverting Antoine's patch seems like a step backwards. +1 to what Mark says here. If the day comes when the peepholer can be ripped out in favour of AST based optimisation, then yay. In the meantime, having it work as consistently as possible in picking up and optimising literal expressions makes for potentially valuable micro-optimisations that people don't have to worry about doing by hand. Cheers, Nick. -- Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 ... 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. Just as a side comment here, I often do this sort of thing when dealing with time based comments, or large constants. It is a bit more obvious that: 10*1024*1024 vs 10485760 is 10MiB especially since you can't put commas in your constants. 10,485,760 would at least make it immediately obvious it was 10M not 104M or something else. Similarly is 10,800s 3 hours? 3*60*60 certainly is. I don't think I've done that sort of thing in anything performance critical. But I did want to point out that writing X*Y*Z as a constant isn't always pretty silly. John =:- -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk17c0sACgkQJdeBCYSNAAMUrQCdEhissWuvTElIc6Wy/2qotzaU xz4AnRO+ND/3NkKWC7Bbu78ACjs2X920 =QR/2 -END PGP SIGNATURE- ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
There are separate social, strategic, and tactical questions. The social question: if the person who designed, implemented, and maintained the optimizer recommends against a patch and another committer just checks it in anyway, do we care? I've taken responsibility for this code and have treated it will a good deal of care. I don't believe the patch should go in because the risk/reward ratio is too low and because a better solution exists. The strategic question: constant folding in the peephole optimizer pre-dates the AST branch. When AST landed, there was general agreement among myself and those involved that any future optimizations which could be done with the AST, should be done there. It is the correct technological solution. At one point, we had strategic agreement to stop building-out the peepholer in any significant way. In fairness to Antoine, the conversations took place several years before he became a committer. The strategic question is do we want to come to that agreement again? Can we agree to not have significant changes to the peepholer, to treat it with great care and restraint? This patch doesn't have to go in. Py3.3 won't be out for 18 to 24 months anyway. There is a lot of time to do the job right and not add weight to a house of cards. The tactical question: is this particular patch correct? Maybe it is. I don't know, I didn't get past all the new macros. I do know that I could have whipped up something similar years ago but chose not to based on advice from Thomas, Andrew, Tim, and Guido. The only change since then is that there is a newer committer doesn't buy into that reasoning. Constant folding is the most complex part of the optimizer. Why would we add to it, when a better and more reliable approach exists? The patch hasn't even been demonstrated to be necessary in any real-world code. Raymond ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
I would like to withdraw my suggestion for the recursive constant folding patch to be reverted. I value social harmony much more than a decision about whether a particular patch is a good idea. I apologize to anyone who is upset over the discussion. Raymond ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Hello, Finally: There appears to be some disagreement on who said what, in particular Raymond claims that he told Antoine not to commit whereas Antoine claims he did not hear this feedback. I'm guessing it happened in IRC (#python-dev), which is intentionally not logged anywhere. Raymond did tell me he was objecting on a patch, but unless I misunderstood him he was talking about http://bugs.python.org/issue11462 (which he indeed closed later), not the present issue. I agree IRC is a poor platform for making decisions (and I've probably been guilty of this too). Regards Antoine. ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
On Sat, 12 Mar 2011 13:08:26 -0500 Raymond Hettinger raymond.hettin...@gmail.com wrote: I would like to withdraw my suggestion for the recursive constant folding patch to be reverted. I value social harmony much more than a decision about whether a particular patch is a good idea. I apologize to anyone who is upset over the discussion. Thank you Raymond. Of course, if there turns out to be a technical issue that can't be fixed, the patch should still be backed out. Regards Antoine. ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Raymond The social question: if the person who designed, implemented, Raymond and maintained the optimizer recommends against a patch and Raymond another committer just checks it in anyway, do we care? Guido - you're dangerously close here to putting your ego ahead of Guido things Maybe, but we have historically tended to give some extra weight to the primary author of at least modules and packages. If someone wanted to make a significant change to xml.etree, I think we would give reasonably large weight to Fredrik Lundh's opinion on the change. If the peephole optimizer is largely Raymond's work, why should that be treated any differently? Is it just because it can't practically be distributed outside of Python proper the way ElementTree can? Skip ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Maybe, but we have historically tended to give some extra weight to the primary author of at least modules and packages. If someone wanted to make a significant change to xml.etree, I think we would give reasonably large weight to Fredrik Lundh's opinion on the change. If the peephole optimizer is largely Raymond's work, why should that be treated any differently? Is it just because it can't practically be distributed outside of Python proper the way ElementTree can? These cases are certainly different. For Elementtree, /F has explicitly asked that it be included into Python only under the condition that he has the last say in all changes, and that the only exception to this would be urgent security fixes, or systematic changes that apply to all modules. Whether such a privilege should have been granted in the first place is a different question, but I personally feel obliged to honor it, until some of the involved parties change their minds. I don't think any of the regular core committers got any such explicit veto powers on any code, and I don't think it should be granted to anybody. Regards, Martin ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Martin I don't think any of the regular core committers got any such Martin explicit veto powers on any code... Veto powers is your term, not mine. I suggested that Raymond's opinion should be accorded extra weight. This isn't the UN Security Council. Skip ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Am 12.03.11 22:36, schrieb s...@pobox.com: Martin I don't think any of the regular core committers got any such Martin explicit veto powers on any code... Veto powers is your term, not mine. I suggested that Raymond's opinion should be accorded extra weight. This isn't the UN Security Council. Yes, but you were comparing it with ElementTree, where Fredrik *does* have veto powers. I was pointing out that the case at hand is different. Regards, Martin ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Martin == Martin v Löwis mar...@v.loewis.de writes: Martin Am 12.03.11 22:36, schrieb s...@pobox.com: Martin I don't think any of the regular core committers got any such Martin explicit veto powers on any code... Veto powers is your term, not mine. I suggested that Raymond's opinion should be accorded extra weight. This isn't the UN Security Council. Martin Yes, but you were comparing it with ElementTree, where Fredrik Martin *does* have veto powers. I was pointing out that the case at Martin hand is different. I didn't know that, nor did I infer that Fredrik had veto power. I wrote: I think we would give reasonably large weight to Fredrik Lundh's opinion on the change. Skip ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
On Fri, Mar 11, 2011 at 9:28 PM, Raymond Hettinger raymond.hettin...@gmail.com wrote: Today, there was a significant check-in to the peephole optimizer that I think should be reverted: http://hg.python.org/cpython/rev/14205d0fee45/ +1 I was going to comment on the corresponding issue #11244 more or less supporting Raymond's arguments. There is no end of optimization ideas that can be implemented in peephole optimizer. I know this first hand having implemented several of those that have been ultimately rejected. At the end of the day, peephole optimizer is a hack that predates proper AST design. There is no need to continue squeezing out last drops of juice from 10-year old technology when much better approach is available with the modern design. ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
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/archive%40mail-archive.com
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Experience shows that optimizations are always error prone, no matter what framework or internal representation you use. I don't think we should assume that simply rewriting all optimizations to work on AST will make them bug free once and for all. On the contrary, I think such a rewrite will introduce some bugs of it's own. The remedy against this is well known. Instead of being afraid to touch the code we should add more tests and verifiers. If had written tests are not enough, test generator that produces python programs with different structures can be written. Such generators are used by many compiler writers. For verifiers, a function that checks that bytecode is sane (doesn't reference invalid names or consts, doesn't jump between instructions, all joins have same stack depth? etc) that runs after optimizer in debug builds can save a lot of time. Eugene ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
On Mar 11, 2011, at 11:11 PM, Eugene Toder wrote: Experience shows that optimizations are always error prone, no matter what framework or internal representation you use. On that basis, I believe that we ought to declare peephole.c as being somewhat off-limits for further development (except for small adaptations if the underlying opcodes change meaning). The code is stable and most of the low-hanging fruit has already been taken. The bright ideas for other enhancements aren't actually new ideas. They were considered at the outset and not included for a reason. We were intentionally very conservative with the optimizer. I don't think we should assume that simply rewriting all optimizations to work on AST will make them bug free once and for all. On the contrary, I think such a rewrite will introduce some bugs of it's own. You make an automatic truth when say, I don't think we should assume X. It would be more correct to say that peepholing is the wrong technique for constant folding because: 1) much of the semantic information available the AST has been lost by the time the bytecode is generated 2) peephole.c is having to disassemble the bytecode, reconstruct semantic relationships as well as it can, recognize transformation patterns and then regenerate bytecode. Moving constant folding to the AST means that the disassembly and reassembly don't have to be done and that more semantic information is available. In other words, the two techniques are equal just because it is always possible to make a mistake using any technique. The remedy against this is well known. Instead of being afraid to touch the code we should add more tests and verifiers. Not really. It is darned difficult to design tests to catch all the corner cases. You've got wishful thinking if you think a handful of tests can catch errors in code that sophisticated. Look at GCC's bug history to get a sense for what can go wrong with optimization. I admire your and Antoine's enthusiasm for wanting to optimize, but think your efforts are misdirected and that you should be taking some advice from the guy who wrote thing in the first place. Raymond ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Experience shows that optimizations are always error prone, no matter what framework or internal representation you use. On that basis, I believe that we ought to declare peephole.c as being somewhat off-limits for further development (except for small adaptations if the underlying opcodes change meaning). With this attitude, it would be more logical to remove the whole thing completely. Even if you don't touch the optimizer, any changes in the compiler can start producing code that exposes a previously hidden bug. The bright ideas for other enhancements aren't actually new ideas. They were considered at the outset and not included for a reason. It would help if these were documented. It would be more correct How can you be more correct than tautological truth? :) 1) much of the semantic information available the AST has been lost by the time the bytecode is generated Not really. Many systems use stack based bytecode as an intermediate form and reconstruct different IRs from it. Examples are JVM, CLR and PyPy. The real problem with bytecode is that it's a representation optimized for storage and interpretation. Transforming it is harder, less efficient and more error-prone than transforming a tree structure. But transformations of trees are in no way automatically bug free. 2) peephole.c is having to disassemble the bytecode, reconstruct semantic relationships as well as it can, recognize transformation patterns and then regenerate bytecode. v8 does peephole directly on x86 machine code. No kittens were harmed. Now, I do agree with you that moving optimizations to AST is a step forward. Moving them to a specialized IR in SSA form would be even better. If there was a framework for AST-based optimizations in the current code, I'd write my code to use it. However, I couldn't find it. What I don't understand is why doing 90% of the work and refuse to do the last 10%. Looking at the actual patches, I do not see significant increase in complexity or code size -- all the complex cases are already handled by existing code. In some cases the code became cleaner and simpler. Not really. It is darned difficult to design tests to catch all the corner cases. You either do that or do not write optimizations. No one said it should be easy. You've got wishful thinking if you think a handful of tests can catch errors in code that sophisticated. Why limit yourself with a handful of tests? Python is widespread, there's *a lot* of code in Python. Unlike with libraries, any code you run tests the optimizer, so just run a lot of code. And, as I've said, write a test generator. Cheers, Eugene ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
2011/3/12 Benjamin Peterson benja...@python.org 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. -- Regards, Benjamin It's not true. I already moved almost all peephole optimizations (introducing others, as well) from peephole.c to ast.chttp://code.google.com/p/wpython2/source/browse/Python/ast.c?repo=wpython11and compiler.chttp://code.google.com/p/wpython2/source/browse/Python/compile.c?repo=wpython11in WPython 1.1. Take a look at pages 21-23 of thishttp://wpython2.googlecode.com/files/Cleanup%20and%20new%20optimizations%20in%20WPython%201.1.pdf . Also, optimizations can be done not only for numbers, but even for tuples, lists, dictionaries, and... slices (pag. 22). See pages 21-24 of thishttp://wpython2.googlecode.com/files/Beyond%20Bytecode%20-%20A%20Wordcode-based%20Python.pdf . Cesare ___ 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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
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.
Isn't this doing it?
1.20 +# Long tuples should be folded too.
1.21 +asm = dis_single(repr(tuple(range(1
1.22 +# One LOAD_CONST for the tuple, one for the None return value
1.23 +self.assertEqual(asm.count('LOAD_CONST'), 2)
1.24 +self.assertNotIn('BUILD_TUPLE', asm)
Eugene
___
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
Re: [Python-Dev] Suggest reverting today's checkin (recursive constant folding in the peephole optimizer)
Cesare Di Mauro wrote: Also, optimizations can be done not only for numbers, but even for tuples, lists, dictionaries, and... slices (pag. 22). See pages 21-24 of thishttp://wpython2.googlecode.com/files/Beyond%20Bytecode%20-%20A%20Wordcode-based%20Python.pdf For the record, constant-folding, and the lack of it, does seem to come up in users' requests. Whether they *need* constant-folding or not, it seems that some people have come to expect it. E.g. see this recent thread on python-list: http://code.activestate.com/lists/python-list/596506/ -- Steven ___ 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
