-
Notifications
You must be signed in to change notification settings - Fork 601
Improve -Dx, -Dt etc output, especially in threaded builds #23322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@iabyn, I'd like to move forward with the discussion of this pull request ... but I have little idea of what practical difference it would make. Could you perhaps give some "before" and "after" examples of Thanks. |
|
On Wed, Jul 16, 2025 at 05:51:26PM -0700, James E Keenan wrote:
Could you perhaps give some "before" and "after" examples of `-Dx` and
`-Dt` in unthreaded and threaded builds so that readers can assess the
impact of the revisions?
These two commits within the PR show examples of perl -Dx and perl -Dt
before and after output; are these not sufficient?
op_dump(): display SVs on threaded builds
Perl_debop() / -Dt: display some OP args better
…--
Overhead, without any fuss, the stars were going out.
-- Arthur C Clarke
|
Currently SVOPs and METHOPs are dumped using the same case branch within op_dump(). This works at the moment because the op_sv field and the op_meth_sv field happen to occupy the same offset within their respective structs. This commit separates out the handling of those two OP classes, which will also allow another commit shortly to handle METHOD ops more specifically. OP_COREARGS is also added as another dumpable SVOP type (i.e. an op which may have an SV hanging off op_sv). This commit also makes it so that it now always displays the value of the op_sv field of SVOPs, which on threaded builds starts off holding an SV, but later gets set to 0 to indicate that the SV has been moved to the pad.
Some types of method call have a redirect class in addition to the method name, e.g. $obj->BAR::foo(). This value 'BAR' wasn't being displayed by op_dump(). So this commit makes it do so. I also took the opportunity to add comments to the various OP_METHOD_FOO cases to identify what sort of method calls it handled, and added a stub OP_METHOD case rather than it just being handled by the default branch. This is to make it clearer that OP_METHOD *does* exist, but it doesn't have any values (like 'foo' or 'BAR') which need dumping.
Some OPs, such as OP_CONST, OP_GVSV and OP_METHOD_NAMED, point to an SV
or GV. In threaded builds, these SVs are moved to the pad and an index is
stored in the OP instead (typically op_targ or op_padix).
When op_dump() is called upon to display an OP (typically during
debugging or via perl -Dx), then currently, information about the linked
SV (e.g. the glob's name) is displayed only on non-threaded builds,
since op_dump() can't assume that PL_curpad[] is associated with this
particular op. Thus you get things like an OP_CONST being dumped that
doesn't display the const's value. This is annoying during debugging.
This commit makes it so that when dumping common OPs which have an SV in
the pad, it tries to find the CV, if any, associated with that op, and
if so, uses that CV's pad to lookup the value. If unsuccessful, it
falls back to not displaying the SV. This commit uses two main
techniques to find the CV. Both rely on first following the op_parent
chain from the current op to find the root op of the optree which this
op is embedded in. Then, if compiling, it compares this with the roots
of the optrees currently on the parse stack, and so, uses the associated
CV which is is pointed to from that slot on the parse stack. Or, if
runtime, looks for a SUB or EVAL context on the context stack and sees
if that sub or eval's CvROOT() / PL_eval_root matches the root of the
op's tree.
The next two commits will extend this to handle 'perl -Dx' too.
This commit also tries to show the state of the fields on CONST and
METHOD_FOO ops which can hold an SV or index, in addition to showing the SV
that is retrieved from them.
Here are examples of some op dumps on threaded builds before and after
this commit:
--------------------------------------------------------------
const SVOP(0x2a051578) ===> 6 [gvsv 0x2a0515e8]
TARG = 2
FLAGS = (SCALAR,SLABBED,MORESIB)
gvsv PADOP(0x2a0515e8) ===> 5 [sassign 0x2a051538]
FLAGS = (SCALAR,SLABBED)
PADIX = 1
method_redir METHOP(0x13dd4318) ===> 5 [entersub 0x13dd4358]
TARG = 4
FLAGS = (UNKNOWN,SLABBED)
--------------------------------------------------------------
const SVOP(0x22f655b8) ===> 6 [gvsv 0x22f65628]
TARG = 2
FLAGS = (SCALAR,SLABBED,MORESIB)
OP_SV = 0x0
SV = PV("abc"\0) (0x22f65768)
gvsv PADOP(0x22f65628) ===> 5 [sassign 0x22f65578]
FLAGS = (SCALAR,SLABBED)
PADIX = 1
GV = main::x (0x22f58f20)
method_redir METHOP(0x1d83f318) ===> 5 [entersub 0x1d83f358]
TARG = 4
FLAGS = (UNKNOWN,SLABBED)
OP_METH_SV = 0x0
METH = PV("foo") (0x1d833010)
RCLASS_TARG = 2
RCLASS = PV("BAR") (0x1d83f638)
--------------------------------------------------------------
A minor refactor in preparation for the next commit: make Perl_dump_sub_perl() invoke S_do_op_dump_bar() directly, rather than going via op_dump() which indirectly calls the former. Should make no functional difference.
A couple of commits ago I added a mechanism to display the values of the SV for ops (such as OP_CONST and OP_GVSV) on threaded builds when possible, where the SV has been moved into the pad. This commit extends that mechanism to work when a sub's optree is being dumped via the '-Dx' perl command-line switch. That previous commit tried to find the CV (and thus pad) associated with the op being dumped by rummaging around on the context and parse stacks. But the -Dx mechanism is neither of those things. It dumps all the subs it can find in packages after compilation, but before execution. This commit adds an extra parameter to S_do_op_dump_bar() which optionally indicates what CV is having its optree dumped. The -Dx mechanism can use this parameter to pass a hint to the SV-in-pad finding code. If the parameter is null, it falls back to the mechanisms added in the previous commits.
The OP_RCATLINE op has a GV attached. So When dumping OPs, display its value, similarly to what is already done for other GV-holding ops like OP_GVSV. Similarly, OP_ANONCODE has a CV attached.
Perl_debop() displays an op in a compact one-line form, typically used
by 'perl -Dt' to show the next op to be executed.
This commit improves the display of some ops slightly: in particular,
where the name of a GV argument to the op, or the name of the associated
lexical var is displayed, sometimes this wasn't being done, for example
for the new op OP_PADSV_STORE, which probably just got missed when being
added.
It also now displays:
* the name of the lexical var for ops which have the OPpTARGET_MY
optimisation;
* the name of the method and redirect class for method ops;
* the index of the aelemfast and aelemfast_lex op
For example, with the following code:
my ($sum);
sub Bar::foo {}
my $obj = bless {}, 'Foo';
my @lexary;
$^D='t';
$sum = 1;
$sum = $ary[-2] + $lexary[3];
$obj->Bar::foo();
$x .= <>;
then before, the -Dt output for certain lines was:
padsv_store
aelemfast
aelemfast_lex
add
method_redir
rcatline
and is now:
padsv_store($sum)
aelemfast(main::ary)[-2]
aelemfast_lex(@lexary)[3]
add($sum)
method_redir(PV("foo"))(PV("Bar"))
rcatline(main::ARGV)
This series of commits improves the way various OPs are displayed by Perl_op_dump() (as used by perl -Dx) and by Perl_debop() (as used by perl -Dt).
In particular it tries, where possible, to display information about an SV or GV attached to the OP when that SV has been moved to the pad on threaded builds.