Skip to content

Remove ROT_FOUR #40121

Closed
Closed
Remove ROT_FOUR#40121
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)

Description

@mcherm
mcherm
mannequin
opened on Apr 5, 2004
BPO 929502
Nosy @brettcannon, @rhettinger
Files
  • remove_ROT_FOUR.patch: Patch to remove the ROT_FOUR opcode.
  • remove_ROT_FOUR.patch: Patch as context diff and dropping macros.
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2004-05-29.09:22:01.000>
created_at = <Date 2004-04-05.02:01:15.000>
labels = ['interpreter-core']
title = 'Remove ROT_FOUR'
updated_at = <Date 2004-05-29.09:22:01.000>
user = 'https://bugs.python.org/mcherm'

    bugs.python.org fields:

    activity = <Date 2004-05-29.09:22:01.000>
actor = 'brett.cannon'
assignee = 'jhylton'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2004-04-05.02:01:15.000>
creator = 'mcherm'
dependencies = []
files = ['5905', '5906']
hgrepos = []
issue_num = 929502
keywords = ['patch']
message_count = 10.0
messages = ['45703', '45704', '45705', '45706', '45707', '45708', '45709', '45710', '45711', '45712']
nosy_count = 4.0
nosy_names = ['jhylton', 'brett.cannon', 'mcherm', 'rhettinger']
pr_nums = []
priority = 'low'
resolution = 'rejected'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue929502'
versions = ['Python 2.4']

    Activity

    1. mcherm commented on Apr 5, 2004

      @mcherm
      mchermmannequin
      on Apr 5, 2004
      MannequinAuthor

      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.

    2. mcherm
      closed this as completedon Apr 5, 2004
    3. mcherm
      assigned
      jhylton
      on Apr 5, 2004
    4. mcherm
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      on Apr 5, 2004
    5. mcherm
      closed this as completedon Apr 5, 2004
    6. mcherm
      assigned
      jhylton
      on Apr 5, 2004
    7. mcherm
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      on Apr 5, 2004

    8. brettcannon commented on Apr 7, 2004

      @brettcannon
      brettcannon
      on Apr 7, 2004
      Member

      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.

    9. mcherm commented on Apr 7, 2004

      @mcherm
      mchermmannequin
      on Apr 7, 2004
      MannequinAuthor

      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.

    10. brettcannon commented on Apr 7, 2004

      @brettcannon
      brettcannon
      on Apr 7, 2004
      Member

      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. =)

    11. mcherm commented on Apr 7, 2004

      @mcherm
      mchermmannequin
      on Apr 7, 2004
      MannequinAuthor

      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.

    12. rhettinger commented on Apr 8, 2004

      @rhettinger
      rhettinger
      on Apr 8, 2004
      Contributor

      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.

    13. mcherm commented on Apr 16, 2004

      @mcherm
      mchermmannequin
      on Apr 16, 2004
      MannequinAuthor

      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.

    14. rhettinger commented on May 12, 2004

      @rhettinger
      rhettinger
      on May 12, 2004
      Contributor

      Logged In: YES
      user_id=80475

      Jeremy, what do you think about this one?

    15. rhettinger commented on May 28, 2004

      @rhettinger
      rhettinger
      on May 28, 2004
      Contributor

      Logged In: YES
      user_id=80475

      Perhaps this patch can be closed (I'm -0 on it also).

    16. brettcannon commented on May 29, 2004

      @brettcannon
      brettcannon
      on May 29, 2004
      Member

      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. =)

    17. ezio-melotti
      transferred this issue fromon Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      interpreter-core(Objects, Python, Grammar, and Parser dirs)

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions