Skip to content

Commit 36e7ab4

Browse files
committed
pp_ref() builtin_pp_reftype(): strlen()+Newx()+memcpy()->100% pre-made COWs
-ref() PP keyword has extremely high usage. Greping my blead repo shows: Searched "ref(" 4347 hits in 605 files of 5879 searched -The strings keyword ref() returns are part of the Perl 5 BNF grammer. This is not up for debate. Changing their spelling or lowercasing them is not for debate, or i18n-ing them dynamically realtime against glibc.so's current OS process global locale is not up for debate or wiring, or wiring inotify/kqueue into the runloop to monitor /etc or /var so this race condition works as designed in a unit test: $perl -E "dire('hello')" Routine indéfinie &cœur::dire aufgerufen bei -e Zeile 1 -sv_reftype() and sv_ref() have very badly designed prototypes, and the first time a new Perl in C dev reads their source code, they will think these 2 will cause infinite C stack recursion and a SEGV. Probably most automated C code analytic tools will complain these 2 functions do infinite recursion too. -The 2 functions don't return a string length, forcing all callers to execute a libc strlen() call on a string, that could be 8 bytes, or 80 MB. -The 2 functions don't split, parse, cat, or glue multiple strings to create their output. All null term-ed strings that they return, are already sitting in virtual address space. Either const HW RO, or RCed HEK*s from the PL_strtab pool, that were found inside something similar to a GV*/HV*/HE*/CV*/AV*/GP*/OP*/SV* in a OP*(no threads). -COW 255 buffers from Newx() under 9 chars can't COW currently by policy. CODE is 4, SCALAR is 6. HASH is 4. ARRAY is 5. But very short SV HEK* COWs will COW propagate without problems. -PP code "if(ref($self) eq 'HASH') {}" should never involve all 3-4 calls Newx()/Realloc()/strlen()/memcpy(). So this fix all of this, and make pp_ref()/PP KW ref() be closer in speed to C/C++/Asm style object type checking, which is almost always going to be 1 or 2 or 3 ptr equality tests against C constant &sum_vtbl_sum_class, or in Microsoft ecosystem SW, its a equality test of a 16 byte GUID in memory, against a 16 byte SSE literal stored in a SSE opcode (TLDR ver). Just convert backends sv_ref()/sv_reftype() to HEK* retvals, and convert the front end pp_*() ops to fetch HEK*s and return SV*s with POK_on SvPVX()== HEK*. In all likely hood, if right side of PP code is "if (ref($self) eq 'HASH') {}", during the execution of memcpy(pv1, pv2, len) as part of pp_eq, pv1 and pv2 are the same mem addr. But I didn't single step eq operator to verify that yet. -inside PP(pp_reftype) previously the branch sv_setsv(TARG, &PL_sv_undef); did not fire SMG, after this commit it does, IDK why it wasnt firing before, or consequences of SMG firing now on sv_set_undef(rsv); path. -I suspect "sv_setsv(TARG, &PL_sv_undef);" and "sv_set_undef(rsv);" are not perfect behavior copies of each other, in extreme/bizzare/user error and bad CPAN XS code situtations but I haven't found any side effects of the switch from sv_setsv(TARG, &PL_sv_undef); to sv_set_undef(rsv) Untested typothetical cases like sv_setsv(gv_star, &PL_sv_undef); sv_setsv(hv_star, &PL_sv_undef); sv_setsv(svt_regexp_star, &PL_sv_undef); sv_setsv(svt_invlist_star, &PL_sv_undef); sv_setsv(svt_object_star, &PL_sv_undef); sv_setsv(svt_io_star, &PL_sv_undef); -sv_sethek() has a severe pathologic performance problem, if args SV* dsv and HEK* src_hek, test true for if(SvPVX(dsv) == HEK_KEY(src_hek)) {}. But its still better than a strlen()/Newx()/memcpy()/push_save_stack()/ delayed_Safefree(); cycle. Any fix for this would be for the future. -these 2 functions are experimental for now, hence undocumented and not public API, if they are made public, arg "const int ob" should be removed because of its confusing faux-infinite recursion but not real life infinite recursion. The fuctions are exported so P5P hackers and CPAN XS devs (unsanctioned by P5P) can benchmark and research these 2 new functions using Inline::C/EU::PXS. -future improvements not done here, make sv_reftype() and sv_ref() wrappers around their HEK* counterparts. Note the HEK* must be RC++ed and stuffed in a new SV*, or a PAD TARG SV*, before the rpp_replace_1_1_NN(TARG); call because in artificial situations/fuzzing, strange things can happen during a SvREFCNT_dec_NN(); call, and the HEK* sitting in a C auto might get freed during the SvREFCNT_dec_NN(); -another improvement, sv_sethek(rsv, hek); is somewhat heavy, and doesn't have a shortcut, to RC-- an existing SVPV HEK* COW itself, instead it uses SV_THINKFIRST_***() and sv_force_normal***() to RC-- an existing SVPV HEK* COW. If the SV* PAD TARG, is being used over and over by ref() opcode, its always going to have a stale HEK* SVPVX() that needs to be RC--ed. -another improvement, check if(sv_reftypehek() == SvPVX(targ)) before calling sv_sethek(rsv, hek); -another improvement, beyond scope for me, make into 1 OP*/opcode: if(ref($self) eq 'HASH') and if(ref($self) eq 'ARRAY') -another improvement, dont deref my_perl->Iop/PL_ptr many times in a row. I didn't do any CPU opcode/instruction stripping in this commit. Thats for a future commit. -another improvement, investigate if most of large switch() inside Perl_sv_reftypehek() can be turned into a const I8 arr_of_PL_sv_consts_idxs[]; with a couple tiny special cases. -todo invert "if (!rsv) {" branch, so hot path (yes cached in PL_sv_consts). comes first in machine code/asm order.
1 parent 8b524d3 commit 36e7ab4

File tree

8 files changed

+215
-12
lines changed

8 files changed

+215
-12
lines changed

builtin.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -563,17 +563,37 @@ PP(pp_refaddr)
563563

564564
PP(pp_reftype)
565565
{
566-
dXSTARG;
567-
SV *arg = *PL_stack_sp;
566+
HEK *hek;
567+
SV *rsv;
568+
SV ** svp = PL_stack_sp;
569+
SV *arg = *svp;
568570

569571
SvGETMAGIC(arg);
570572

571573
if(SvROK(arg))
572-
sv_setpv_mg(TARG, sv_reftype(SvRV(arg), FALSE));
574+
hek = sv_reftypehek(SvRV(arg), FALSE);
573575
else
574-
sv_setsv(TARG, &PL_sv_undef);
576+
hek = NULL;
577+
578+
/* unrolled dXSTARG; avoid slower sv_setxv_mg(sv_newmortal(), ); */
579+
if (PL_op->op_private & OPpENTERSUB_HASTARG) {
580+
rsv = PAD_SV(PL_op->op_targ);
581+
if (hek)
582+
sv_sethek(rsv, hek);
583+
else /* if a PAD TARG exists, returning &PL_sv_undef will force a */
584+
sv_set_undef(rsv); /* slow trip through sv_setsv() in next OP */
585+
SvSETMAGIC(rsv);
586+
rpp_replace_1_1_NN(rsv); /* no RC_STK =, RC_STK RC++ = */
587+
}
588+
else {
589+
if (!hek)
590+
rpp_replace_1_IMM_NN(&PL_sv_undef);
591+
else {
592+
rsv = newSVhek(hek);
593+
rpp_replace_at_norc(svp, rsv); /* no RC_STK mortal =, RC_STK RC++ = */
594+
}
595+
}
575596

576-
rpp_replace_1_1_NN(TARG);
577597
return NORMAL;
578598
}
579599

embed.fnc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3366,9 +3366,12 @@ AMTdip |SV * |SvREFCNT_inc |NULLOK SV *sv
33663366
AMTdip |SV * |SvREFCNT_inc_NN|NN SV *sv
33673367
AMTdip |void |SvREFCNT_inc_void \
33683368
|NULLOK SV *sv
3369+
EXp |HEK * |sv_refhek |NN const SV * const sv \
3370+
|const int ob
33693371
ARdp |const char *|sv_reftype|NN const SV * const sv \
33703372
|const int ob
3371-
3373+
ERXp |HEK * |sv_reftypehek |NN const SV * const sv \
3374+
|const int ob
33723375
Adp |void |sv_regex_global_pos_clear \
33733376
|NN SV *sv
33743377
ARdp |bool |sv_regex_global_pos_get \

embed.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,6 +1810,8 @@
18101810
# define skipspace_flags(a,b) Perl_skipspace_flags(aTHX_ a,b)
18111811
# define sv_magicext_mglob(a) Perl_sv_magicext_mglob(aTHX_ a)
18121812
# define sv_only_taint_gmagic Perl_sv_only_taint_gmagic
1813+
# define sv_refhek(a,b) Perl_sv_refhek(aTHX_ a,b)
1814+
# define sv_reftypehek(a,b) Perl_sv_reftypehek(aTHX_ a,b)
18131815
# define utf16_to_utf8_base(a,b,c,d,e,f) Perl_utf16_to_utf8_base(aTHX_ a,b,c,d,e,f)
18141816
# define utf8_to_utf16_base(a,b,c,d,e,f) Perl_utf8_to_utf16_base(aTHX_ a,b,c,d,e,f)
18151817
# define validate_proto(a,b,c,d) Perl_validate_proto(aTHX_ a,b,c,d)

pp.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,13 +573,13 @@ PP(pp_ref)
573573

574574
do_sv_ref:
575575
{
576+
HEK* hek = sv_refhek(SvRV(sv), TRUE);
576577
dTARGET;
577-
sv_ref(TARG, SvRV(sv), TRUE);
578+
sv_sethek(TARG, hek);
578579
rpp_replace_1_1_NN(TARG);
579580
SvSETMAGIC(TARG);
580581
return NORMAL;
581582
}
582-
583583
}
584584

585585

pp_hot.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,8 @@ Perl_rpp_free_2_(pTHX_ SV *const sv1, SV *const sv2,
224224

225225
PP(pp_const)
226226
{
227-
rpp_xpush_1(cSVOP_sv);
227+
SV* sv = cSVOP_sv;
228+
rpp_xpush_1(sv);
228229
return NORMAL;
229230
}
230231

proto.h

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sv.c

Lines changed: 152 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3000,8 +3000,9 @@ Perl_sv_2pv_flags(pTHX_ SV *const sv, STRLEN *const lp, const U32 flags)
30003000

30013001
return RX_WRAPPED(re);
30023002
} else {
3003-
const char *const typestring = sv_reftype(referent, 0);
3004-
const STRLEN typelen = strlen(typestring);
3003+
HEK * const typestring_hek = sv_reftypehek(referent, 0);
3004+
const char *const typestring = HEK_KEY(typestring_hek);
3005+
const I32 typelen = HEK_LEN(typestring_hek);
30053006
UV addr = PTR2UV(referent);
30063007
const char *stashname = NULL;
30073008
STRLEN stashnamelen = 0; /* hush, gcc */
@@ -10634,6 +10635,135 @@ Perl_sv_reftype(pTHX_ const SV *const sv, const int ob)
1063410635
}
1063510636
}
1063610637

10638+
HEK*
10639+
Perl_sv_reftypehek(pTHX_ const SV *const sv, const int ob)
10640+
{
10641+
PERL_ARGS_ASSERT_SV_REFTYPEHEK;
10642+
10643+
if (ob && SvOBJECT(sv))
10644+
return sv_refhek(sv, ob);
10645+
else {
10646+
SV* rsv;
10647+
U32 idx;
10648+
/* WARNING - There is code, for instance in mg.c, that assumes that
10649+
* the only reason that sv_reftype(sv,0) would return a string starting
10650+
* with 'L' or 'S' is that it is a LVALUE or a SCALAR.
10651+
* Yes this a dodgy way to do type checking, but it saves practically reimplementing
10652+
* this routine inside other subs, and it saves time.
10653+
* Do not change this assumption without searching for "dodgy type check" in
10654+
* the code.
10655+
* - Yves */
10656+
switch (SvTYPE(sv)) {
10657+
case SVt_NULL:
10658+
case SVt_IV:
10659+
case SVt_NV:
10660+
case SVt_PV:
10661+
case SVt_PVIV:
10662+
case SVt_PVNV:
10663+
case SVt_PVMG:
10664+
if (SvVOK(sv)) {
10665+
idx = SV_CONST_VSTRING;
10666+
break;
10667+
}
10668+
else if (SvROK(sv)) {
10669+
idx = SV_CONST_REF;
10670+
break;
10671+
}
10672+
else {
10673+
idx = SV_CONST_SCALAR;
10674+
break;
10675+
}
10676+
case SVt_PVLV: idx = (SvROK(sv) ? SV_CONST_REF
10677+
/* tied lvalues should appear to be
10678+
* scalars for backwards compatibility */
10679+
: (isALPHA_FOLD_EQ(LvTYPE(sv), 't'))
10680+
? SV_CONST_SCALAR : SV_CONST_LVALUE); break;
10681+
case SVt_PVAV: { idx = SV_CONST_ARRAY; break; }
10682+
case SVt_PVHV: { idx = SV_CONST_HASH; break; }
10683+
case SVt_PVCV: { idx = SV_CONST_CODE; break; }
10684+
case SVt_PVGV: {
10685+
idx = isGV_with_GP(sv) ? SV_CONST_GLOB : SV_CONST_SCALAR;
10686+
break;
10687+
}
10688+
case SVt_PVFM: { idx = SV_CONST_FORMAT; break; }
10689+
case SVt_PVIO: { idx = SV_CONST_IO; break; }
10690+
case SVt_INVLIST: { idx = SV_CONST_INVLIST; break; }
10691+
case SVt_REGEXP: { idx = SV_CONST_REGEXP; break; }
10692+
case SVt_PVOBJ: { idx = SV_CONST_OBJECT; break; }
10693+
default: { idx = SV_CONST_UNKNOWN; break; }
10694+
}
10695+
10696+
rsv = PL_sv_consts[idx];
10697+
if (!rsv) {
10698+
U8 len;
10699+
const char * pv;
10700+
switch(idx) {
10701+
case SV_CONST_ARRAY:
10702+
pv = "ARRAY";
10703+
len = STRLENs("ARRAY");
10704+
break;
10705+
case SV_CONST_CODE:
10706+
pv = "CODE";
10707+
len = STRLENs("CODE");
10708+
break;
10709+
case SV_CONST_FORMAT:
10710+
pv = "FORMAT";
10711+
len = STRLENs("FORMAT");
10712+
break;
10713+
case SV_CONST_GLOB:
10714+
pv = "GLOB";
10715+
len = STRLENs("GLOB");
10716+
break;
10717+
case SV_CONST_HASH:
10718+
pv = "HASH";
10719+
len = STRLENs("HASH");
10720+
break;
10721+
case SV_CONST_INVLIST:
10722+
pv = "INVLIST";
10723+
len = STRLENs("INVLIST");
10724+
break;
10725+
case SV_CONST_IO:
10726+
pv = "IO";
10727+
len = STRLENs("IO");
10728+
break;
10729+
case SV_CONST_LVALUE:
10730+
pv = "LVALUE";
10731+
len = STRLENs("LVALUE");
10732+
break;
10733+
case SV_CONST_OBJECT:
10734+
pv = "OBJECT";
10735+
len = STRLENs("OBJECT");
10736+
break;
10737+
case SV_CONST_REF:
10738+
pv = "REF";
10739+
len = STRLENs("REF");
10740+
break;
10741+
case SV_CONST_REGEXP:
10742+
pv = "REGEXP";
10743+
len = STRLENs("REGEXP");
10744+
break;
10745+
case SV_CONST_SCALAR:
10746+
pv = "SCALAR";
10747+
len = STRLENs("SCALAR");
10748+
break;
10749+
case SV_CONST_UNKNOWN:
10750+
pv = "UNKNOWN";
10751+
len = STRLENs("UNKNOWN");
10752+
break;
10753+
case SV_CONST_VSTRING:
10754+
pv = "VSTRING";
10755+
len = STRLENs("VSTRING");
10756+
break;
10757+
default: /* unreachable, don't make a verbose long string */
10758+
croak_no_mem_ext(STR_WITH_LEN("sv_reftypehek"));
10759+
}
10760+
rsv = newSVpvn_share(pv, len, 0);
10761+
PL_sv_consts[idx] = rsv;
10762+
}
10763+
return SvSHARED_HEK_FROM_PV(SvPVX_const(rsv));
10764+
}
10765+
}
10766+
1063710767
/*
1063810768
=for apidoc sv_ref
1063910769

@@ -10669,6 +10799,26 @@ Perl_sv_ref(pTHX_ SV *dst, const SV *const sv, const int ob)
1066910799
return dst;
1067010800
}
1067110801

10802+
HEK *
10803+
Perl_sv_refhek(pTHX_ const SV *const sv, const int ob)
10804+
{
10805+
HEK * hek;
10806+
PERL_ARGS_ASSERT_SV_REFHEK;
10807+
10808+
if (ob && SvOBJECT(sv)) {
10809+
HV* stash = SvSTASH(sv);
10810+
if (HvHasNAME(stash))
10811+
hek = HvNAME_HEK(stash);
10812+
else {
10813+
SV * rsv = SV_CONST(__ANON__);
10814+
hek = SvSHARED_HEK_FROM_PV(SvPVX_const(rsv));
10815+
}
10816+
}
10817+
else
10818+
hek = sv_reftypehek(sv, 0);
10819+
return hek;
10820+
}
10821+
1067210822
/*
1067310823
=for apidoc sv_isobject
1067410824

sv.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2695,9 +2695,25 @@ Create a new IO, setting the reference count to 1.
26952695
# define SV_CONST_CLEAR 32
26962696
# define SV_CONST_UNTIE 33
26972697
# define SV_CONST_DESTROY 34
2698+
2699+
# define SV_CONST_ARRAY 35
2700+
# define SV_CONST_CODE 36
2701+
# define SV_CONST_FORMAT 37
2702+
# define SV_CONST_GLOB 38
2703+
# define SV_CONST_HASH 39
2704+
# define SV_CONST_INVLIST 40
2705+
# define SV_CONST_IO 41
2706+
# define SV_CONST_LVALUE 42
2707+
# define SV_CONST_OBJECT 43
2708+
# define SV_CONST_REF 44
2709+
# define SV_CONST_REGEXP 45
2710+
/* "SCALAR" is above */
2711+
# define SV_CONST_UNKNOWN 46
2712+
# define SV_CONST_VSTRING 47
2713+
# define SV_CONST___ANON__ 48
26982714
#endif
26992715

2700-
#define SV_CONSTS_COUNT 35
2716+
#define SV_CONSTS_COUNT 49
27012717

27022718
/*
27032719
* Bodyless IVs and NVs!

0 commit comments

Comments
 (0)