[Patches] [ python-Patches-587993 ] alternative SET_LINENO killer
noreply@sourceforge.net
noreply@sourceforge.net
Mon, 05 Aug 2002 05:20:57 -0700
Patches item #587993, was opened at 2002-07-29 11:27
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=587993&group_id=5470
Category: Core (C code)
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Michael Hudson (mwh)
Assigned to: Guido van Rossum (gvanrossum)
Summary: alternative SET_LINENO killer
Initial Comment:
This patch is a proof-of-concept of another way to
remove the SET_LINENO patch (as opposed to Vladimir's
ancient one).
Instead of rewriting bytecode (ick!) we poke into the
c_lnotab to see if we've moved onto a different line.
The c_lnotab is not the most transparent of data
structures, it has to be said.
I'm not sure this patch is 100% correct -- but I think
the idea can definitely fly. There will be some more
overhead to tracing than before, but I hope not too
much. I haven't tested these aspects.
Comments welcome!
----------------------------------------------------------------------
>Comment By: Michael Hudson (mwh)
Date: 2002-08-05 12:20
Message:
Logged In: YES
user_id=6656
Here's another update. I've also deleted the old patches.
Changes this time:
+ doesn't include pystone LOOPS boost
+ finish RETURN_NONE off:
- add to dis.py
- add to libdis.tex
+ removed now-unecessary co_lnotab grovelling in inspect
and traceback. left the functions for compatibility.
+ removed the WE_WANT_THE_HACK hack
+ incudes Neal's test_hotshot patch
+ more trace.py tweaks.
Lib/compiler is still broken.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-05 11:56
Message:
Logged In: YES
user_id=33168
Re-assigning to Guido. I think what happened was I was
reviewing the patch before it was assigned to Guido. Sorry
about that.
You're right about the set_lineno, I thought the opcode was
also generated in there. I agree about the code reuse--it's
probably too hard. If I had a better idea, I would have
shared it. :-(
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-08-05 09:09
Message:
Logged In: YES
user_id=6656
Thanks for the comments, Neal!
I'm not sure it's possible to separate the changes in the
way you describe. For instance, com_addoparg ->
com_set_lineno stops SET_LINENO being generated, so breaks
tracing without the VM changes, but the VM changes make
SET_LINENO into an unknown opcode...
I didn't intend to upload the pystone change.
About the comments:
- the last little RETURN_NONE changes are easy. Thanks for
the pointer.
- agree there are too many bits of code grovelling
co_lnotab. however, it's not clear that they can easily be
refactored. maybe a generator would be useful... hmm. let
me think about this one.
- did I take out the initialisation of frame.f_lineno
entirely? oops. probably set it to co_firstlineno.
- fixed trace.py docstring. reusing code not all that
easy, because different uses have subtly different requirements.
hmm, inspect.getlineno is now pointless...
was there any particular reason you unassigned the patch?
Just curious as the main reason it was assigned to Guido was
to make sure he saw it.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-04 19:50
Message:
Logged In: YES
user_id=33168
Attached is a patch to test_hotshot which is necessary
because the duplicate line #s at the beginning of the
function no longer exist. WIth this patch test_hotshot passes.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-03 20:06
Message:
Logged In: YES
user_id=33168
Overall, the patch looks very good.
It would be nice to check in some of the changes separate
from this patch:
* com_addoparg(SET_LINENO) -> com_set_lineno()
* the updated comments and whitespace
* pystone
* dis.py, disassemble(), perhaps
* RETURN_NONE, perhaps
Here's other minor comments:
* Need to add RETURN_NONE opcode into dis.py
* Need to add doc for RETURN_NONE in libdis.tex
* It would be nice if the code from inspect.getlineno could
be reused in disassemble()
(not sure this can be done, though--i saw at least 4
variations of this code, 2 in python and 2 in C)
* Should frame.f_lineno be initialized to 0, -1 or
something invalid?
* scripts/trace.py reimplements getting the lineno, why not
reuse code?
* docstring in scripts/trace._find_LINENO_from_code needs
to remove ref to SET_LINENO
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-08-03 15:52
Message:
Logged In: YES
user_id=6656
Update. New this time:
- adds RETURN_NONE opcode for the epilgoue.
- don't call line trace events on it.
- bumps MAGIC
this is the output of "cvs diff" at the top level, so it's
all in there.
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-08-02 13:34
Message:
Logged In: YES
user_id=6656
Here's an update (just for ceval.c):
- moves tracing code out of line
- more expository comments
- don't cause line trace events in the function epilogue
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-08-01 15:43
Message:
Logged In: YES
user_id=6656
Hang on, lets get all the doc changes:
Doc/lib/libdis.tex
Doc/tut/tut.tex
Doc/whatsnew/whatsnew23.tex
Misc/NEWS
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-08-01 15:41
Message:
Logged In: YES
user_id=6656
And finally, docs.
This touches:
Doc/lib/libdis.tex
Doc/tut/tut.tex
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-08-01 15:40
Message:
Logged In: YES
user_id=6656
Here's the "boring" patch, more mechanical stuff.
It touches:
Include/opcode.h
Lib/traceback.py
Modules/_hotshot.c
Python/frozen.c
Python/traceback.c
This should still be reviewed, of course, but I really
shouldn't have messed any of this up.
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-08-01 15:38
Message:
Logged In: YES
user_id=6656
Yeah, the obvious response to that was "delete your .pycs"...
Right, I'm going to upload my latest, and hopefully final
patch in three bits.
First, I'm attaching the "interesting" patch. This touches:
Objects/frameobject.c -- adds f_lineno descr
Python/ceval.c -- the obvious
Python/compile.c -- don't emit SET_LINENO
Lib/dis.py -- use c_lnotab for line numbers
Tools/scripts/trace.py -- ditto
This is the most subtle patch, and the one that I'd most
like review on.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-07-30 22:09
Message:
Logged In: YES
user_id=6380
Never mind the errors, I hadn't done a cvs update in weeks
on the machine where I tested it. :-(
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-07-30 21:54
Message:
Logged In: YES
user_id=6380
Cool idea, but I get "unknown opcode" errors... Keep me
posted though! (I wil ll now see any changes to this patch
item.)
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-07-30 09:59
Message:
Logged In: YES
user_id=6656
Guido should see this, assuming he still isn't subscribed to
patches@python.org.
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-07-30 09:58
Message:
Logged In: YES
user_id=6656
I worked out why some of the code in ceval.c wasn't making
sense to me -- it didn't make sense, period.
I've also fixed a number of silly and not so silly bugs in
my patch. I'm now 99% certain this idea can fly. The patch
isn't *finished* but the hard bit is done, IMHO.
There are some other points to make, but I think I'll raise
them on python-dev.
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2002-07-29 20:18
Message:
Logged In: YES
user_id=31435
Dropping out of "vacation mode" long enough to say "mondo
cool!" and encourage this. Guido may not agree, but I also
encourage you to redefine c_lnotab if it can make life easier
and quicker here. That subtle compression scheme has
been the source of several nasty bugs, both in the core C
code and in Jeremy's compiler pkg (cut 'n paste bugs
abound here, because few people understand what's really
needed, so flawed code gets copied with little thought).
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-07-29 15:34
Message:
Logged In: YES
user_id=6656
Uhh, the instr_[lu]b variables need to keep their values
around the loop; otherwise we might just as well call
PyCode_Addr2Line each time around.
I have another version of the patch that does that, but I
assumed the overhead of doing so was deemed too high, or
someone else would have done this by now. It's certainly
easier.
Glad to hear it doesn't affect python -O too much. I was
doing this away from the internet and forgot to keep a clean
copy of the source around for doing comparisons with...
----------------------------------------------------------------------
Comment By: Neil Schemenauer (nascheme)
Date: 2002-07-29 15:18
Message:
Logged In: YES
user_id=35752
Moving the "int io, instr_ub = -1, instr_lb = 0;"
declaration and the
"io = INSTR_OFFSET();"| statement below the "if
(tstate-c_tracefunc ...)"
test gives a small speedup on my machine and is a little
neater, IMHO.
I was worried that this would slow down "python -O". That
doesn't seem to
be the case (at least I can't measure it). Well done.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=587993&group_id=5470