Skip to content

Commit aa5e4ce

Browse files
committed
add sv_refhek, sv_reftypehek functions for better ref() performance
-ref() PP keyword has extremely high usage. Greping my blead repo shows: Searched "ref(" 4347 hits in 605 files of 5879 searched -High level PP keyword ref(), aka C function Perl_pp_ref(), uses slow, inefficient, badly designed, backend public XS/C API called functions called Perl_sv_ref()/Perl_sv_reftype(). -This commit fixes all design problems with Perl_sv_ref()/Perl_sv_reftype(), and will speed up the very high usage PP keyword ref(), along with a very similar but very new and very little used PP keyword called "use builtin qw( reftype );" which is near identical to Perl_pp_ref(). -a crude benchmark, with the array ref in $aref holding 43000 SV*s, split 1/3rd SV* IOK, 1/3rd RV* to SV* IOK, and 1/3rd RV* to CV*, showed a %6 speed increase for this code sub benchme { foreach my $el (@{$aref}) { $cnt++ if ref($el) eq 'SCALAR';} } -The all UPPERCASE strings keyword ref() returns are part of the Perl 5 BNF grammer. Changing their spelling or lowercasing them is not for debate, or i18n-ing them dynamically realtime against glibc.so's current "OS global locale" with inotify()/kqueue() in the runloop to monitor a text file /etc or /var so this race condition works as designed in a unit test will never happen: $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 maybe will complain these 2 functions do infinite recursion. -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. -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. ref() is also used to retrieve "Local::My::Class" strings, which have an extremely high chance to wind up getting passed to hv_common() through some high level PP keyword like bless or @isa, and hv_common() extracts precalculated U32 hash values from SV* with HEK* buffers, speeding up hv_common(). So SV* POKs with COW 255 and COW SVs_STATIC buffers are bad choices compared to using SV* POK HEK* buffers for a new faster version of sv_reftype()/sv_ref(). -PP code "if(ref($self) eq 'HASH') {}" should never involve all 3-5 calls Newx()/Realloc()/strlen()/memcpy()/Safefree(), on each execution of the line. To improve the src code dev-friendlyness of the prototypes of, and speed inside of, and the speed of in all libperl callers of Perl_sv_ref()/Perl_sv_reftype(). Make HEK* variants of them. Initially the HEK* variants are private to libperl. Maybe after 1-3 years into the future, they can be made official public C API for CPAN XS authors. These 2 new functions are undocumented/private API until further notice. Using SV* holding RC-ed HEK* SvPVX() buffers removes all these libc C lang logical and/or Asm machine code steps from during execution of PP keyword ref(). The pre-allocated PAD TARG SV* just keeps getting a RC-- on the old HEK* inside SvPVX(), and a RC++ on the new HEK* written to SvPVX() of the PAD TARG SV*. Touching only 6 void*s/size_t adresses total, each one a single read/write CPU instruction pair. SvPVX, SvCUR, SvLEN, old_hek.shared_he.shared_he_he.he_valu.hent_refcount, new_hek.shared_he.shared_he_he.he_valu.hent_refcount, new_hek.shared_he.shared_he_hek.hek_len. This brings PP KW ref() closer to C++ style RTTI that just compares const read-only vtable pointers. Some design and optimization problems with the old and new pp_ref()/pp_reftype()/sv_ref()/sv_reftype()/sv_refhek()/sv_reftypehek() calls are intentionally not being fixed in this commit to keep this commit small. Check the associated PR of the commit for details.
1 parent 8b524d3 commit aa5e4ce

File tree

7 files changed

+241
-11
lines changed

7 files changed

+241
-11
lines changed

builtin.c

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -563,17 +563,43 @@ 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+
/* If a PAD TARG exists, returning &PL_sv_undef will force a slow trip
584+
through sv_setsv() in next OP, so do the undef assignment here,
585+
with the streamlined sv_set_undef() call, vs universal and complex
586+
sv_setsv() call. Note, the prior code here, only fired SMG magic
587+
on the sv_sethek()/sv_setpvs() branch, not on the sv_set_undef()
588+
branch. */
589+
else
590+
sv_set_undef(rsv);
591+
SvSETMAGIC(rsv);
592+
rpp_replace_1_1_NN(rsv); /* no RC_STK =, RC_STK RC++ = */
593+
}
594+
else {
595+
if (!hek)
596+
rpp_replace_1_IMM_NN(&PL_sv_undef);
597+
else {
598+
rsv = newSVhek(hek);
599+
rpp_replace_at_norc(svp, rsv); /* no RC_STK mortal =, RC_STK RC++ = */
600+
}
601+
}
575602

576-
rpp_replace_1_1_NN(TARG);
577603
return NORMAL;
578604
}
579605

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

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: 174 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,150 @@ Perl_sv_reftype(pTHX_ const SV *const sv, const int ob)
1063410635
}
1063510636
}
1063610637

10638+
/* Experimental faster variant of sv_reftype(). Identical to sv_reftype() except
10639+
it returns a HEK* from PL_strtab. Does not bump the RC on the HEK*.
10640+
Caller must bump the RC of the HEK* if they want to preserve it for future
10641+
use. The easiest way to do the RC bump is with sv_sethek() or newSVhek().
10642+
It is exported for private P5P experiments with Inline::C or EU::PXS, but
10643+
it is not a public API for CPAN authors and may change at any time. */
10644+
10645+
HEK*
10646+
Perl_sv_reftypehek(pTHX_ const SV *const sv, const int ob)
10647+
{
10648+
PERL_ARGS_ASSERT_SV_REFTYPEHEK;
10649+
10650+
if (ob && SvOBJECT(sv))
10651+
return sv_refhek(sv, ob);
10652+
else {
10653+
SV* rsv;
10654+
/* The SV_CONST() macro can not be used here or else the switch table
10655+
below will have 17 unique callsites to exported libperl symbol
10656+
Perl_newSVpvn_share(). In the future, this switch statement can be
10657+
optimized to an array mapping U8 sv_type codes, to U8 or U16s indexes
10658+
pointing into array PL_sv_consts, with negative indexs being
10659+
if (){} else {} special cases needing extra checks.*/
10660+
U32 idx;
10661+
/* WARNING - There is code, for instance in mg.c, that assumes that
10662+
* the only reason that sv_reftype(sv,0) would return a string starting
10663+
* with 'L' or 'S' is that it is a LVALUE or a SCALAR.
10664+
* Yes this a dodgy way to do type checking, but it saves practically reimplementing
10665+
* this routine inside other subs, and it saves time.
10666+
* Do not change this assumption without searching for "dodgy type check" in
10667+
* the code.
10668+
* - Yves */
10669+
switch (SvTYPE(sv)) {
10670+
case SVt_NULL:
10671+
case SVt_IV:
10672+
case SVt_NV:
10673+
case SVt_PV:
10674+
case SVt_PVIV:
10675+
case SVt_PVNV:
10676+
case SVt_PVMG:
10677+
if (SvVOK(sv)) {
10678+
idx = SV_CONST_VSTRING;
10679+
break;
10680+
}
10681+
else if (SvROK(sv)) {
10682+
idx = SV_CONST_REF;
10683+
break;
10684+
}
10685+
else {
10686+
idx = SV_CONST_SCALAR;
10687+
break;
10688+
}
10689+
case SVt_PVLV: idx = (SvROK(sv) ? SV_CONST_REF
10690+
/* tied lvalues should appear to be
10691+
* scalars for backwards compatibility */
10692+
: (isALPHA_FOLD_EQ(LvTYPE(sv), 't'))
10693+
? SV_CONST_SCALAR : SV_CONST_LVALUE); break;
10694+
case SVt_PVAV: { idx = SV_CONST_ARRAY; break; }
10695+
case SVt_PVHV: { idx = SV_CONST_HASH; break; }
10696+
case SVt_PVCV: { idx = SV_CONST_CODE; break; }
10697+
case SVt_PVGV: {
10698+
idx = isGV_with_GP(sv) ? SV_CONST_GLOB : SV_CONST_SCALAR;
10699+
break;
10700+
}
10701+
case SVt_PVFM: { idx = SV_CONST_FORMAT; break; }
10702+
case SVt_PVIO: { idx = SV_CONST_IO; break; }
10703+
case SVt_INVLIST: { idx = SV_CONST_INVLIST; break; }
10704+
case SVt_REGEXP: { idx = SV_CONST_REGEXP; break; }
10705+
case SVt_PVOBJ: { idx = SV_CONST_OBJECT; break; }
10706+
default: { idx = SV_CONST_UNKNOWN; break; }
10707+
}
10708+
10709+
rsv = PL_sv_consts[idx];
10710+
if (!rsv) {
10711+
U8 len;
10712+
const char * pv;
10713+
switch(idx) {
10714+
case SV_CONST_ARRAY:
10715+
pv = "ARRAY";
10716+
len = STRLENs("ARRAY");
10717+
break;
10718+
case SV_CONST_CODE:
10719+
pv = "CODE";
10720+
len = STRLENs("CODE");
10721+
break;
10722+
case SV_CONST_FORMAT:
10723+
pv = "FORMAT";
10724+
len = STRLENs("FORMAT");
10725+
break;
10726+
case SV_CONST_GLOB:
10727+
pv = "GLOB";
10728+
len = STRLENs("GLOB");
10729+
break;
10730+
case SV_CONST_HASH:
10731+
pv = "HASH";
10732+
len = STRLENs("HASH");
10733+
break;
10734+
case SV_CONST_INVLIST:
10735+
pv = "INVLIST";
10736+
len = STRLENs("INVLIST");
10737+
break;
10738+
case SV_CONST_IO:
10739+
pv = "IO";
10740+
len = STRLENs("IO");
10741+
break;
10742+
case SV_CONST_LVALUE:
10743+
pv = "LVALUE";
10744+
len = STRLENs("LVALUE");
10745+
break;
10746+
case SV_CONST_OBJECT:
10747+
pv = "OBJECT";
10748+
len = STRLENs("OBJECT");
10749+
break;
10750+
case SV_CONST_REF:
10751+
pv = "REF";
10752+
len = STRLENs("REF");
10753+
break;
10754+
case SV_CONST_REGEXP:
10755+
pv = "REGEXP";
10756+
len = STRLENs("REGEXP");
10757+
break;
10758+
case SV_CONST_SCALAR:
10759+
pv = "SCALAR";
10760+
len = STRLENs("SCALAR");
10761+
break;
10762+
case SV_CONST_UNKNOWN:
10763+
pv = "UNKNOWN";
10764+
len = STRLENs("UNKNOWN");
10765+
break;
10766+
case SV_CONST_VSTRING:
10767+
pv = "VSTRING";
10768+
len = STRLENs("VSTRING");
10769+
break;
10770+
default: /* unreachable, don't make a verbose long string */
10771+
croak_no_mem_ext(STR_WITH_LEN("sv_reftypehek"));
10772+
}
10773+
rsv = newSVpvn_share(pv, len, 0);
10774+
PL_sv_consts[idx] = rsv;
10775+
} /* Never return our secret PL_sv_consts[idx] SV*s to any caller for
10776+
any reason. We don't SvREADONLY_on(), and we don't trust PP code.
10777+
Example: my $klass = \ref($self); ${$klass} .= "::Base"; */
10778+
return SvSHARED_HEK_FROM_PV(SvPVX_const(rsv));
10779+
}
10780+
}
10781+
1063710782
/*
1063810783
=for apidoc sv_ref
1063910784

@@ -10669,6 +10814,33 @@ Perl_sv_ref(pTHX_ SV *dst, const SV *const sv, const int ob)
1066910814
return dst;
1067010815
}
1067110816

10817+
/* Experimental faster variant of sv_ref(). Identical to sv_ref() except
10818+
it returns a HEK* from PL_strtab. Does not bump the RC on the HEK*.
10819+
Caller must bump the RC of the HEK* if they want to preserve it for future
10820+
use. The easiest way to do the RC bump is with sv_sethek() or newSVhek().
10821+
It is exported for private P5P experiments with Inline::C or EU::PXS, but
10822+
it is not a public API for CPAN authors and may change at any time. */
10823+
10824+
HEK *
10825+
Perl_sv_refhek(pTHX_ const SV *const sv, const int ob)
10826+
{
10827+
HEK * hek;
10828+
PERL_ARGS_ASSERT_SV_REFHEK;
10829+
10830+
if (ob && SvOBJECT(sv)) {
10831+
HV* stash = SvSTASH(sv);
10832+
if (HvHasNAME(stash))
10833+
hek = HvNAME_HEK(stash);
10834+
else {
10835+
SV * rsv = SV_CONST(__ANON__);
10836+
hek = SvSHARED_HEK_FROM_PV(SvPVX_const(rsv));
10837+
}
10838+
}
10839+
else
10840+
hek = sv_reftypehek(sv, 0);
10841+
return hek;
10842+
}
10843+
1067210844
/*
1067310845
=for apidoc sv_isobject
1067410846

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)