Repository navigation
SET_LINENO killer #36946
Description
| BPO | 587993 |
|---|---|
| Nosy | @mwhudson, @gvanrossum, @tim-one, @loewis, @arigo, @nascheme |
| Files |
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 = 'https://github.com/mwhudson'
closed_at = <Date 2002-11-27.09:44:01.000>
created_at = <Date 2002-07-29.11:27:43.000>
labels = ['interpreter-core']
title = 'SET_LINENO killer'
updated_at = <Date 2002-11-27.09:44:01.000>
user = 'https://github.com/mwhudson'bugs.python.org fields:
activity = <Date 2002-11-27.09:44:01.000>
actor = 'mwh'
assignee = 'mwh'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2002-07-29.11:27:43.000>
creator = 'mwh'
dependencies = []
files = ['4453', '4454', '4455', '4456']
hgrepos = []
issue_num = 587993
keywords = ['patch']
message_count = 47.0
messages = ['40679', '40680', '40681', '40682', '40683', '40684', '40685', '40686', '40687', '40688', '40689', '40690', '40691', '40692', '40693', '40694', '40695', '40696', '40697', '40698', '40699', '40700', '40701', '40702', '40703', '40704', '40705', '40706', '40707', '40708', '40709', '40710', '40711', '40712', '40713', '40714', '40715', '40716', '40717', '40718', '40719', '40720', '40721', '40722', '40723', '40724', '40725']
nosy_count = 8.0
nosy_names = ['mwh', 'gvanrossum', 'tim.peters', 'loewis', 'nnorwitz', 'arigo', 'nascheme', 'niemeyer']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue587993'
versions = ['Python 2.3']
Activity
mwhudson commentedon Jul 29, 2002
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!
nascheme commentedon Jul 29, 2002
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.
mwhudson commentedon Jul 29, 2002
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...
tim-one commentedon Jul 29, 2002
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).
mwhudson commentedon Jul 30, 2002
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.
mwhudson commentedon Jul 30, 2002
Logged In: YES
user_id=6656
Guido should see this, assuming he still isn't subscribed to
patches@python.org.
gvanrossum commentedon Jul 30, 2002
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.)
gvanrossum commentedon Jul 30, 2002
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. :-(
mwhudson commentedon Aug 1, 2002
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.
24 remaining items
mwhudson commentedon Aug 27, 2002
Logged In: YES
user_id=6656
Attached a patch. This removes the need for RETURN_NONE,
implementing a scheme somewhat like Armin suggests. Needs
more expository comments, and probably some refactoring.
Also adds a test case.
Armin, why do you say size >= 0 should be size > 0? Things
stop working if I make that change.
nnorwitz commentedon Aug 27, 2002
Logged In: YES
user_id=33168
Michael, did you forget to upload the patch? The last patch
is from the 15th (#8).
arigo commentedon Aug 28, 2002
Logged In: YES
user_id=4771
mwh: "spam.py" was the 5 lines of code above in the message.
The weird thing about the while(size>=0) loop is that it may
be executed 'size+1' times, as stated. There is an
off-by-one bug somewhere. If it works like this, it must
obscurely rely on something like Python string arrays being
always terminated by an extra NULL character.
mwhudson commentedon Aug 28, 2002
Logged In: YES
user_id=6656
guess I must have forgotten to check the box...
arigo commentedon Aug 28, 2002
Logged In: YES
user_id=4771
Michael, I meant the other while loop! The *first* one. I have
no objection about the second loop.
A minor comment about your patch: your extra
variable "trace_called" looks like an indirect way to say that
we must only invoke call_trace() if after the loop we have
(frame->f_lasti==*instr_lb).
mwhudson commentedon Aug 29, 2002
Logged In: YES
user_id=6656
Thanks for the comments!
I was being dumb.
Patch attached, though I don't really know why; I'm
confident this is right.
Now, do we rip RETURN_NONE out again? That's one for Guido,
I think.
mwhudson commentedon Aug 29, 2002
Logged In: YES
user_id=6656
Sigh, *here's* the patch.
nnorwitz commentedon Aug 29, 2002
Logged In: YES
user_id=33168
The test doesn't need to import pprint or os.
I'd like to see RETURN_NONE removed, it seemed like a hack,
and if it's not necessary...
Unrelated to the changes in the patch, could/should the
PyStrring_AsString and _Size be changed to the macro
versions? The return values aren't checked and it should be
faster. Although this code is only executed when tracing.
gvanrossum commentedon Aug 29, 2002
Logged In: YES
user_id=6380
Little time to ponder here, but if we could get rid of
RETURN_NONE, that would be nice.
mwhudson commentedon Aug 30, 2002
Logged In: YES
user_id=6656
Right, RETURN_NONE is gone again.
Fixes checked in as
Doc/lib/libdis.tex revision 1.40
Doc/whatsnew/whatsnew23.tex revision 1.49
Include/opcode.h revision 2.41
Lib/dis.py revision 1.44
Lib/test/test_trace.py revision 1.1
Python/ceval.c revision 2.333
Python/compile.c revision 2.263
Thanks for the comments! Closing again...
gvanrossum commentedon Aug 30, 2002
Logged In: YES
user_id=6380
Thanks, Michael!
Do you think the import.c MAGIC needs to be changed again???
mwhudson commentedon Aug 30, 2002
Logged In: YES
user_id=6656
In a word, no. Just run
find $(srcdir) -name '*.pyc" | xargs rm
and be done with it.
You can change it if you care.
niemeyer commentedon Nov 26, 2002
Logged In: YES
user_id=7887
Michael, could you please have a look at the compiler
package? It's currently broken because it still using
SET_LINENO.
mwhudson commentedon Nov 27, 2002
Logged In: YES
user_id=6656
I have! See bpo-597919 and go pester Jeremy :)