On Sat, Mar 12, 2011 at 11:05 AM, Raymond Hettinger <raymond.hettin...@gmail.com> wrote: > 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.
Hm. A few comments: - you're dangerously close here to putting your ego ahead of things - further review of the patch may give a better understanding of the risk - a "better solution" that doesn't exist yet shouldn't be too strong an argument against "good enough" > 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. Feel free to go ahead and start it. Write a PEP, or just write a patch and get people to review and understand it. If it's successful, we can delete peephole.c and the effect of Antoine's checkin will be nullified. If it's not successful, Antoine's code has the advantage of possibly being good enough, and will have had those same 18-24 months to have its tires kicked a bit. > 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. A recurring problem I hear about is that there is too much resistance to new ideas coming from new developers. Clearly new developers may not be aware of where the skeletons are hiding. But just as clearly the old guard isn't always right. Antoine is by now not a newbie any more. So please consider Antoine's patch on its own merit and not just from the perspective of a decision made years ago. > 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. peephole.c is now 773 lines; it was 700 lines, so it grew by about 10%. But that's not very large: ceval.c is 4500 lines (many of which are much deeper magic which I personally don't feel I understand at all :-). I am not at all sure that you can create an AST-based optimizer in less than 700 lines. (Nor am I sure that you can't -- nor am I sure it matters. I'm, just saying. :-) All in all I think it would be more productive to try and understand Antoine's patch than to try to get it revert based on "because I [or Tim Peters] said so" arguments. 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. This is an illustration of what is wrong with using IRC for important decisions. I would have been much happier if the discussion about the desirability of the patch could have happened *before* it was committed, and I think maybe we need to have a stricter rule requiring code reviews and possibly cool-off periods. -- --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
