[issue11462] Peephole creates duplicate and unused constants

[Thomas Wouters]

Thomas Wouters <thomas at python.org> added the comment:

What is the actual performance impact of this change? As far as I can see the difference is almost entirely in .pyc (on-disk) size, as when loading most of these constants are interned anyway. Even the on-disk change is minimal - and I say that as a person who actually cares a lot about the size of .pyc files. Considering the extra work that needs to be done, I expect planA to be a measurable performance drop, and planB to be indistinguishable. Perhaps I'm wrong, so feel free to prove it so by providing some (unladen-swallow) benchmarks :)

I'm not sure this change is worth it unless there's a clear performance benefit. The peephole optimizer is very limited, fragile, and an uncomfortable place to edit (and I say *that* as a person who has introduced way too many subtle, stupid bugs in the past.) As Raymond says, the right thing to do with the peepholer is to rip it out and replace it with an AST-based optimizer, which wouldn't bypass or duplicate all manner of sanity- and safety-checks. Perhaps this should be a GSoC project.

About the patches themselves: the plan-A patch is a bad idea, I wouldn't consider it; duplicating the compiler checks seems pointless. In the plan-B patch, PyCode_Optimize is an exported symbol so it should not change; you need to change the name and provide the old interface in a macro. The extern declaration of _PyCode_AddObj in peephole.c is now unnecessary. The name of that function is rather non-obvious, since it doesn't do anything with a code object at all -- but I don't have a better suggestion.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue11462>
_______________________________________