Skip to content

Commit 36beb19

Browse files
committed
op.c: slim down const_av_xsub const_sv_xsub XSUB.h: add GetXSTARG()
-don't use XSRETURN() macros, they use "unit of 1" relative offset "ax" and constantly reread global my_parl->Istack_base and constant rescale ax from unit of 1 to unit of 4/8 -don't use ST() macros for the same reason -in const_sv_xsub() use XSTARG if its available, but just put our long life SV* right on the PL stack if we can't write directly into the caller lval SV*. Don't do "sv_setsv(sv_newmortal(), ssv);" on missing XSTARG branch. Don't use dXSTARG; 50% chance there will be a 2nd!!! secret "sv_setsv(sv_newmortal(),rval)" or "newSVsv(rval)" right after this XSUB anyways, so just pass our SV* on stack instead of TWO "sv_setsv(sv_newmortal(), ssv);" statements executing in a row. const_av_xsub(): -if we will croak, do it ASAP once the minimum amount of data has been read from global memory, AV* is delivered through C function argument CV* cv, its not from the PL stack. So do the check and execute the no return before creating/reading/writing a ton of PL stack related global vars and C auto vars. -GetXSTARG() and GIMME_V() both dig inside PL_op, keep them together w/o any func calls in between such as EXTEND() so the CC can read OP* stored in PL_op only once -break apart Copy()'s overflow bounds checks so we can write a new "length" to global state before copying the large in bytes array, historically Perl has issues with letting PP end users code to keep running for severe overflows/heap corruption/CVE type stuff, eval{}, %SIG, tied, MG, sub END, etc. So do the asserts early before the actual memcpy or PUTBACK. -handle an empty/0 elems long AV* better, don't EXTEND(0), don't call extern libc memcpy with 0 size -don't keep re-reading the AV head and AV body and AvFILLp(av) over and over, func calls EXTEND() and memcpy() won't realloc the AV body ptr or modify the AvFILL member in the body struct XSUB.h: -add a version of dXSTARG where the user handles what to do, if parent frame didn't supply a SV* TARG for lval filling. This gets rid of the inefficient sv_newmortal() that is forced on users when there almost always better faster recipie of how to create a 0, 1, or G_LIST PL stack retval. &PL_sv_undef, newSVpvn_flags(SVs_TEMP), sv_2mortal(newSViv()), return your hash key's value SV* directly, etc. Macro undocumented until further notice, so it can gather some unofficial usage/CORE usage and some opinions regarding is it good or flawed.
1 parent e5ef137 commit 36beb19

File tree

2 files changed

+158
-19
lines changed

2 files changed

+158
-19
lines changed

XSUB.h

+96
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,105 @@ is a lexical C<$_> in scope.
178178
Stack_off_t ax = XS_SETXSUBFN_POPMARK; \
179179
SV **mark = PL_stack_base + ax - 1; dSP; dITEMS
180180

181+
182+
/* The internals of dXSTARG and GetXSTARG are tightly coupled (de jure)
183+
with the optree and P5 lang. In practice, dXSTARG and OPpENTERSUB_HASTARG
184+
haven't been modified since they were created in 5.5.61 in commit 8a7fc0dc30
185+
9/10/1999 3:22:14 PM "s/dXS_TARGET/dXSTARG/ in change#4044" and in
186+
commit d30110745a 8/26/1999 11:33:01 PM "Speeding up XSUB calls up to 66%"
187+
188+
dXSTARG is public API, but the implementation is opaque. OPpENTERSUB_HASTARG
189+
is private API. Future hypothetical enhancements that could change dXSTARG
190+
are GIMME_V == G_BOOL, call_sv_lval(), XS_MULTICALL. */
191+
192+
181193
#define dXSTARG SV * const targ = ((PL_op->op_private & OPpENTERSUB_HASTARG) \
182194
? PAD_SV(PL_op->op_targ) : sv_newmortal())
183195

196+
/* GetXSTARG() is part of the interp's private API for now. It is intended
197+
that in the future, it will be public API. Timeframe is TBD. It needs
198+
more usage in PERL_CORE, to shake out potential API design flaws or CPAN XS
199+
mousetraps before committing to support it publically. Since it is private,
200+
it can be renamed if needed. */
201+
202+
#ifdef PERL_CORE
203+
204+
/* A faster, more efficient variant of C<dXSTARG>. Similar to the optree's
205+
GETTARGET, but "specialized" for XSUBs written by core or written by CPAN.
206+
The benefit of C<GetXSTARG> over C<dXSTARG> is that C<GetXSTARG> will return
207+
C<NULL> if a targ C<SV *> isn't currently available and lets the user decide
208+
how to go forward. Meanwhile C<dXSTARG> will always internally call
209+
C<sv_newmortal()> if a targ C<SV *> isn't available at that moment.
210+
Do not evaluate this macro multiple times. Save the result of this macro to
211+
a C<SV*> var.
212+
213+
Just like C<dXSTARG>, the C<SV *> returned by C<GetXSTARG> may have stale
214+
prior contents in it. */
215+
/*
216+
You must test the returned value for C<NULL> and procede
217+
accordingly. Do not make any assumptions on why you did or did not get NULL
218+
from this macro. This macro is not a substitute for using L<GIMME_V>.
219+
The non-NULL or NULL result of this macro has no correlation to what
220+
C<@_> context, the caller PP/XS sub, has requested from your XSUB through
221+
C<GIMME_V>.
222+
223+
Assume C<GIMME_V> can return <G_VOID> while at the same time C<GetXSTARG>
224+
returns non-NULL. Also assume C<if (!(sv = GetXSTARG()) && GIMME_V == G_SCALAR)>
225+
can happen and therefore you very likely will need to allocate a new C<SV*>
226+
and mortalize it. It is discouraged and probably a bug, for an XSUB to
227+
bump the C<SvREFCNT(sv)> on C<TARG> and save the C<SV*> for later use.
228+
Do not make assumptions about C<TARG>'s C<SvREFCNT>, or what is the outer
229+
container that really the C<SV*>. Something else inside the interpreter
230+
which is unspecified, owns C<SV* TARG>, and unspecified caller, probably
231+
wants you to write into this C<SV*> as an lval, vs you doing a less
232+
efficient C<sv_newmortal()>/C<sv_2mortal(newSVxxxv())>, and later on the
233+
unspecified caller has to call <sv_setsv()>, and let the mortal stack dispose
234+
of your short lived <SV*>.
235+
236+
Although this is undocumented and private to the interpreter and you may not
237+
write code that is aware of this private implementation detail. Keep in
238+
mind the interpreter uses the C<SV* TARG> concept for both input and/or output
239+
in various places between parts of itself and might be using C<SV* TARG> as
240+
lvalue scratch pad in a loop.
241+
242+
Remember that the C<SV*> from C<dXSTARG> or C<GetXSTARG>, might be C<SvOK()>
243+
and have stale prior contents that you need to wipe or free. C<sv_setxxx()>
244+
functions will always do this for you. There is no guarentee the <SV*>
245+
from C<dXSTARG> or C<GetXSTARG> will be set to C<undef> when you get it.
246+
If you need to return C<undef>, you have 2 choices. Don't fetch and
247+
don't use C<TARG>, and push C<&PL_sv_undef> on the stack. The other choices
248+
you have is to call, sorted most efficient to least efficient:
249+
250+
sv_setsv_undef(my_targ); SvSETMAGIC(my_targ);
251+
sv_setpv_mg(my_targ, NULL);
252+
sv_setsv_mg(my_targ, NULL);
253+
sv_setsv_mg(my_targ, &PL_sv_undef); //more readable
254+
255+
Also consider, there is no clear guidance for this. Do you think you the
256+
PP or XS caller, that called your XSUB, if it is interested in getting a
257+
C<@_> return value in the first place. Is the caller going to write it as
258+
a true/false check, like C<if(do_xsub()) {0;}>, or will it write
259+
C<my $result = do_xsub();> and capture your return value for future use.
260+
The first probably don't set up a TARG for your to use. The 2nd probably
261+
will, but there are no guarentees it will set one up ahead of time.
262+
263+
Returning address C<&PL_sv_undef> is much faster than C<sv_newmortal()> or
264+
C<sv_set_undef()>. C<sv_set_undef()> is faster than the caller later on
265+
doing a C<sv_setsv()>. C<sv_setsv()> has a quick bailout shortcut in it
266+
if src and dest C<SV*>s are the same addr.
267+
268+
There is also no guarentee about what its C<SvTYPE()> is.
269+
Always assume it is of type <SVt_NULL>, and it has no SV body until you
270+
you test its type and possibly call C<SvUPGRADE> or <sv_setiv>/C<sv_setpvn>
271+
on it. There is no guarentee C<SvANY()> is non-NULL or C<SvANY()> contains
272+
a valid address or points to initialized memory. There is no guarentee
273+
C<SvTYPE()> is at minimum C<SVt_IV> and reading C<SvIVX()> won't SEGV. */
274+
275+
#define GetXSTARG() ((PL_op->op_private & OPpENTERSUB_HASTARG) \
276+
? PAD_SV(PL_op->op_targ) : NULL)
277+
278+
#endif
279+
184280
/* Should be used before final PUSHi etc. if not in PPCODE section. */
185281
#define XSprePUSH (sp = PL_stack_base + ax - 1)
186282

op.c

+62-19
Original file line numberDiff line numberDiff line change
@@ -16266,39 +16266,82 @@ Perl_wrap_op_checker(pTHX_ Optype opcode,
1626616266
static void
1626716267
const_sv_xsub(pTHX_ CV* cv)
1626816268
{
16269+
SV * sv = MUTABLE_SV(XSANY.any_ptr);
1626916270
dXSARGS;
16270-
SV *const sv = MUTABLE_SV(XSANY.any_ptr);
16271-
PERL_UNUSED_ARG(items);
16272-
if (!sv) {
16273-
XSRETURN(0);
16271+
SP -= items; /* wipe incoming, this is a ... vararg on PP level */
16272+
/* Don't optimize/chk for G_VOID, very unlikely or caller bug
16273+
to reach this XSUB and discard its @_ retval. */
16274+
if (sv) {
16275+
EXTEND(SP, 1);
16276+
SV *const targ = GetXSTARG();
16277+
/* If we have it, write into it, to prevent and shortcut
16278+
the inevitable sv_setsv() the caller will do. */
16279+
if (targ) {
16280+
SV *const ssv = sv;
16281+
sv = targ;
16282+
sv_setsv_mg(targ, ssv);
16283+
}
16284+
PUSHs(sv);
1627416285
}
16275-
EXTEND(sp, 1);
16276-
ST(0) = sv;
16277-
XSRETURN(1);
16286+
PUTBACK; /* ret 0 or 1 SV*s */
1627816287
}
1627916288

1628016289
static void
1628116290
const_av_xsub(pTHX_ CV* cv)
1628216291
{
16283-
dXSARGS;
1628416292
AV * const av = MUTABLE_AV(XSANY.any_ptr);
16285-
SP -= items;
16293+
16294+
if (av && SvRMAGICAL(av))
16295+
Perl_croak_nocontext("Magical list constants are not supported");
1628616296
assert(av);
16297+
16298+
dXSARGS;
16299+
SP = MARK; /* wipe all */
1628716300
#ifndef DEBUGGING
1628816301
if (!av) {
16289-
XSRETURN(0);
16302+
PUTBACK;
16303+
return;
1629016304
}
1629116305
#endif
16292-
if (SvRMAGICAL(av))
16293-
croak("Magical list constants are not supported");
16294-
if (GIMME_V != G_LIST) {
16295-
EXTEND(SP, 1);
16296-
ST(0) = sv_2mortal(newSViv((IV)AvFILLp(av)+1));
16297-
XSRETURN(1);
16306+
SSize_t av_cur = AvFILLp(av)+1;
16307+
/* protect PUTBACK before Copy(), in case perl's Copy()/memcpy()
16308+
returns execution control to PP code with longjmp(). */
16309+
MEM_WRAP_CHECK(av_cur, SV *);
16310+
16311+
SV * retsv;
16312+
U8 gm = GIMME_V;
16313+
if (gm != G_LIST) { /* group GIMME_V GetXSTARG so they share PL_op derefs */
16314+
retsv = GetXSTARG();
16315+
if (retsv)
16316+
sv_setiv_mg(retsv, (IV)av_cur);
16317+
else
16318+
retsv = sv_2mortal(newSViv((IV)av_cur));
16319+
}
16320+
else if(av_cur == 0) { /* empty array */
16321+
PUTBACK;
16322+
return;
16323+
}
16324+
else
16325+
retsv = NULL;
16326+
EXTEND(SP, retsv ? 1 : av_cur);
16327+
SP++; /* move to ST(0), returning atleast 1 elem */
16328+
if (retsv) {
16329+
SETs(retsv);
16330+
PUTBACK;
16331+
}
16332+
else {
16333+
SV ** avarr = AvARRAY(av);
16334+
SV ** sp_start = SP;
16335+
perl_assert_ptr(sp_start);
16336+
perl_assert_ptr(avarr);
16337+
SP += (av_cur-1); /* leave SP on top of last valid element, not 1 after */
16338+
PUTBACK;
16339+
/* Ideally Copy() will tailcall to libc or do a theoretical unrealistic
16340+
croak() which resumes normal PP control flow. So do all of Copy()'s
16341+
croak()s and checks earlier. Now the PUTBACK to global state can be
16342+
done safely before Copy/memcpy executes, and tailcail out of here. */
16343+
memcpy((char*)(sp_start),(const char*)(avarr), (av_cur) * sizeof(SV *));
1629816344
}
16299-
EXTEND(SP, AvFILLp(av)+1);
16300-
Copy(AvARRAY(av), &ST(0), AvFILLp(av)+1, SV *);
16301-
XSRETURN(AvFILLp(av)+1);
1630216345
}
1630316346

1630416347
/* Copy an existing cop->cop_warnings field.

0 commit comments

Comments
 (0)