Issue 929502: Remove ROT_FOUR - Python tracker (original) (raw)
Created on 2004-04-05 02:01 by mcherm, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Messages (10)
Author: Michael Chermside (mcherm) 
Date: 2004-04-05 02:01
I'll apologize in advance for the excessively lengthy description here.
This patch results from an observation that Brett made during the PyCon sprints... that the ROT_FOUR opcode is only used in one incredibly obscure situation... augmented slice assignment (and then only when the slice has exactly 2 arguments). In case it's not obvious what augmented slice assignment IS, here's an example:
x = range(6) x[2:4] += 'abc' x [0, 1, 2, 3, 'a', 'b', 'c', 4, 5]
The feature needs to be supported but is of no practical use whatsoever. Thus, one could ALMOST say that the ROT_FOUR opcode is never used in Python.
Meanwhile, Raymond and others are hard at work trying to squeeze better performance out of Python, and every time they propose adding a new opcode or fiddling with the core interpreter loop, someone somewhere complains about cache effects. If we could drop support for some completely unused opcode, then it might give Raymond and others enough elbow room to do something truly useful.
So this patch re-implements augmented slice assignment WITHOUT using ROT_FOUR (some fancy stack juggling and a temporary tuple do the trick).
I'll leave it to others to decide whether it is worth keeping the ROT_FOUR opcode around "just because we might need it someday" (I'll note that we dropped ROT_FIVE and ROT_SIX some time ago as they were never used). But keeping it around just to support augmented assignments is no longer an issue.
I tried to update everything relevent (ceval and compile obviously, but also docs, the compiler package and a mention in NEWS). I did NOT, however, update a version number for the changed bytecodes, since I'm not sure where such a thing lives (although SOMETHING must be changed whenever a new release has modified the opcodes).
One technical note: I apologize for not using a context diff. If someone can give me pointers on how to do that on Windows (I've been using Tortoise CVS) I'll re-run the diff.
And finally... after hanging around here for a number of years, this is my first half-way decent patch submission! Thanks to everyone at the PyCon sprints who helped get me set up and compiling.
Author: Brett Cannon (brett.cannon) * 
Date: 2004-04-07 02:14
Logged In: YES user_id=357491
I think you can actually use the difflib module from the stdlib. The docs say that Tools/scripts/diff.py is a front-end for doing this on files.
Author: Michael Chermside (mcherm)
Date: 2004-04-07 16:33
Logged In: YES user_id=99874
It's fairly easy to run a context diff between two files, what I'm not sure how to do is to run a context diff over the entire tree, comparing my files against those in CVS and collecting the results into a single diff file. I can do that with a command-line CVS, but I'm not sure how to do it with the tools I have on Windows.
I suppose I could modify diff.py so it was capable of
comparing two trees, but then I'd need to keep an extra up-
to-date copy of the entire python CVS tree on my drive to
compare against. Seems like there's gotta be a better solution.
Author: Brett Cannon (brett.cannon) *
Date: 2004-04-07 18:27
Logged In: YES user_id=357491
There is the cvs diff command if you have a checkout of CVS. That
will diff the specified files (not sure if it will check the whole tree if you
don't give it any args). Don't know how TortoiseCVS handles it, though.
As for making a single diff file, all that takes is concatenating all the individual diffs into a single file. On UNIX it literally is just taking individual diff files and tacking on to the end each other.
As for the better solution, there is always using UNIX. =)
Author: Michael Chermside (mcherm)
Date: 2004-04-07 18:43
Logged In: YES user_id=99874
Jim Jewette writes me separately to say that he handles it by keeping a separate CVS checkout to diff against (as I described before). TortoiseCVS provides "cvs diff" (that's what I used to build this patch) but it defaults to unified diffs, not context diffs.
There oughta be a better diff tool in the world. Perhaps I should write one (in Python)! In the meantime, I guess I'll follow Jim's advice and just keep two separate CVS checkouts.
Author: Raymond Hettinger (rhettinger) *
Date: 2004-04-08 15:54
Logged In: YES user_id=80475
ROT_FOUR is somewhat obscure and I won't miss it. Once it is gone, you can also eliminate the macro definitions for FOURTH and SET_FOURTH which are only used by ROT_FOUR.
The motivation ought to be simplification rather than the vague notion of "cache effects" which gets tossed around too much as if it had more meaning than it does. Size changes to the eval_loop do cause it to hop in and out of local minimums on one compiler or another. While smaller is generally better, do not expect to see a measurable benefits from removing the opcode.
It's up to Brett to decide whether the code base is actually simpler after the patch. The "fancy stack juggling and temporary tuple" are not beautiful and add more complication to the compiler than they remove from the eval loop. When disassembling something that used to contain ROT_FOUR, the new byte code is much less obvious.
Author: Michael Chermside (mcherm)
Date: 2004-04-16 01:42
Logged In: YES user_id=99874
All right, I finally got a chance to re-do the diff as a context diff using Jim's approach. And I removed the FOURTH and SET_FOURTH macros as Raymond suggested. Now we just need to decide whether to apply the patch or reject it. Although I'm loath to "waste" the effort that's gone into this so far, I think in the end it is probably NOT a simplification. So I guess I am (reluctantly) a -0 on my own patch.
Author: Raymond Hettinger (rhettinger) *
Date: 2004-05-12 15:29
Logged In: YES user_id=80475
Jeremy, what do you think about this one?
Author: Raymond Hettinger (rhettinger) *
Date: 2004-05-28 23:03
Logged In: YES user_id=80475
Perhaps this patch can be closed (I'm -0 on it also).
Author: Brett Cannon (brett.cannon) *
Date: 2004-05-29 09:22
Logged In: YES user_id=357491
Well, with both Raymond and Michael -0 on this, I am going to take Raymond's hint and close this patch myself.
It was definitely a learning experience to figure this sucker out. Maybe we should try to eliminate an opcode at every PyCON. =)