Skip to content

Commit f2aa556

Browse files
committed
mro.xs more SVPV COW HEK * optimizations
-S_mro_get_linear_isa_c3() dont call av_count() many times -the fn call mro_newSVsvhekok(), or any ISO C fn call located between static inlines newAV_alloc_xz() and av_push_simple() severely degrades or destroys any chance of anything const folding or any thing being removed by the CC for common sub exp elimination AV* isalin = newAV_alloc_xz(4); av_push_simple(isalin, mro_newSVsvhekok(classname)); Changing it to SV* nsv = mro_newSVsvhekok(classname); AV* isalin = newAV_alloc_xz(4); av_push_simple(isalin, nsv); fixes the problem. The CC is now allowed to fuse bottom half of newAV_alloc_xz() and top half of av_push_simple(). -"const I32 throw_nomethod = SvIVX(ST(1));" silence CC truncate warning
1 parent efda4d6 commit f2aa556

File tree

1 file changed

+54
-19
lines changed

1 file changed

+54
-19
lines changed

ext/mro/mro.xs

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ static struct mro_alg c3_alg =
1515
typedef struct {
1616
SV *sv_UNIVERSAL;
1717
SV *sv_dfs;
18+
SV *sv_c3;
1819
SV *sv_ISA;
1920
} my_cxt_t;
2021

@@ -27,6 +28,8 @@ init_MY_CXT(pTHX_ pMY_CXT)
2728
SvREADONLY_on(MY_CXT.sv_UNIVERSAL);
2829
MY_CXT.sv_dfs = newSVpvs_share("dfs");
2930
SvREADONLY_on(MY_CXT.sv_dfs);
31+
MY_CXT.sv_c3 = newSVpvn_share("c3", sizeof("c3")-1, c3_alg.hash);
32+
SvREADONLY_on(MY_CXT.sv_c3);
3033
MY_CXT.sv_ISA = newSVpvs_share("ISA");
3134
SvREADONLY_on(MY_CXT.sv_ISA);
3235
}
@@ -124,8 +127,9 @@ S_mro_get_linear_isa_c3(pTHX_ HV* stash, U32 level)
124127
if(!isa_item_stash) {
125128
/* if no stash, make a temporary fake MRO
126129
containing just itself */
130+
SV* nsv = mro_newSVsvhekok(isa_item);
127131
AV* const isa_lin = newAV_alloc_xz(4);
128-
av_push_simple(isa_lin, mro_newSVsvhekok(isa_item));
132+
av_push_simple(isa_lin, nsv);
129133
av_push_simple(seqs, MUTABLE_SV(isa_lin));
130134
}
131135
else {
@@ -205,11 +209,12 @@ S_mro_get_linear_isa_c3(pTHX_ HV* stash, U32 level)
205209
}
206210
}
207211
}
208-
212+
{
209213
/* Initialize retval to build the return value in */
210-
retval = newAV_alloc_xz(4);
211-
av_push_simple(retval, newSVhek(stashhek)); /* us first */
212-
214+
SV* nsv = newSVhek(stashhek);
215+
retval = newAV_alloc_xz(4);
216+
av_push_simple(retval, nsv); /* us first */
217+
}
213218
/* This loop won't terminate until we either finish building
214219
the MRO, or get an exception. */
215220
while(1) {
@@ -284,13 +289,15 @@ S_mro_get_linear_isa_c3(pTHX_ HV* stash, U32 level)
284289
hierarchy is not C3-incompatible */
285290
if(!winner) {
286291
SV *errmsg;
287-
Size_t i;
292+
SSize_t i;
293+
SSize_t count;
288294

289295
errmsg = newSVpvf(
290296
"Inconsistent hierarchy during C3 merge of class '%" HEKf "':\n\t"
291297
"current merge results [\n",
292298
HEKfARG(stashhek));
293-
for (i = 0; i < av_count(retval); i++) {
299+
count = av_count(retval);
300+
for (i = 0; i < count; i++) {
294301
SV **elem = av_fetch(retval, i, 0);
295302
sv_catpvf(errmsg, "\t\t%" SVf ",\n", SVfARG(*elem));
296303
}
@@ -306,9 +313,12 @@ S_mro_get_linear_isa_c3(pTHX_ HV* stash, U32 level)
306313
}
307314
}
308315
else { /* @ISA was undefined or empty */
316+
/* Do this 1st, the next 2 AV* API calls are likely to be inlined and
317+
optimize away alot of AvFILL/memset/Renew logic if nothing is between them. */
318+
SV* nsv = newSVhek(stashhek);
309319
/* build a retval containing only ourselves */
310320
retval = newAV_alloc_xz(4);
311-
av_push_simple(retval, newSVhek(stashhek));
321+
av_push_simple(retval, nsv);
312322
}
313323

314324
done:
@@ -365,6 +375,9 @@ PPCODE:
365375
sv = MY_CXT.sv_dfs;
366376
MY_CXT.sv_dfs = NULL;
367377
SvREFCNT_dec_NN(sv);
378+
sv = MY_CXT.sv_c3;
379+
MY_CXT.sv_c3 = NULL;
380+
SvREFCNT_dec_NN(sv);
368381
sv = MY_CXT.sv_ISA;
369382
MY_CXT.sv_ISA = NULL;
370383
SvREFCNT_dec_NN(sv);
@@ -388,8 +401,9 @@ mro_get_linear_isa(...)
388401

389402
if(!class_stash) {
390403
/* No stash exists yet, give them just the classname */
404+
SV* nsv = mro_newSVsvhekok(classname);
391405
AV* isalin = newAV_alloc_xz(4);
392-
av_push_simple(isalin, mro_newSVsvhekok(classname));
406+
av_push_simple(isalin, nsv);
393407
ST(0) = sv_2mortal(newRV_noinc(MUTABLE_SV(isalin)));
394408
XSRETURN(1);
395409
}
@@ -433,6 +447,7 @@ mro_get_mro(...)
433447
SV* classname;
434448
HV* class_stash;
435449
SV* retsv;
450+
U32 which_my_cxt; /* rel offset MY_CXT, prevents 2 sep dMY_CXT deref lines */
436451
PPCODE:
437452
if (items != 1)
438453
croak_xs_usage(cv, "classname");
@@ -442,12 +457,28 @@ mro_get_mro(...)
442457

443458
if (class_stash) {
444459
const struct mro_alg *const meta = HvMROMETA(class_stash)->mro_which;
445-
retsv = newSVpvn_flags(meta->name, meta->length,
446-
SVs_TEMP
447-
| ((meta->kflags & HVhek_UTF8) ? SVf_UTF8 : 0));
460+
if (memEQs(meta->name, meta->length, "dfs")) /* skipping meta->kflags & HVhek_UTF8 */
461+
goto ret_dfs;
462+
/* "c3" shows up here running mro's .t'es */
463+
else if (memEQs(meta->name, meta->length, "c3")) {
464+
which_my_cxt = STRUCT_OFFSET(my_cxt_t, sv_c3);
465+
goto ret_my_cxt_hek;
466+
}
467+
else { /* pretty sure this string already exists inside PL_strtab by now */
468+
I32 i32len = meta->kflags & HVhek_UTF8 ? -(I32)meta->length : (I32)meta->length;
469+
retsv = sv_2mortal(newSVpvn_share(meta->name, i32len, meta->hash));
470+
}
448471
} else {
449-
dMY_CXT;
450-
retsv = newSVhek_mortal(SvSHARED_HEK_FROM_PV(SvPVX(MY_CXT.sv_dfs)));
472+
ret_dfs:
473+
which_my_cxt = STRUCT_OFFSET(my_cxt_t, sv_dfs);
474+
475+
ret_my_cxt_hek:
476+
{
477+
dMY_CXT;
478+
SV** svp = NUM2PTR(SV**,(PTR2nat(&MY_CXT)+which_my_cxt));
479+
SV* svhek = *svp;
480+
retsv = newSVhek_mortal(SvSHARED_HEK_FROM_PV(SvPVX(svhek)));
481+
}
451482
}
452483
PUSHs(retsv);
453484

@@ -551,7 +582,7 @@ void
551582
mro__nextcan(...)
552583
PREINIT:
553584
SV* self = ST(0);
554-
const I32 throw_nomethod = SvIVX(ST(1));
585+
const bool throw_nomethod = cBOOL(SvIVX(ST(1)));
555586
I32 cxix = cxstack_ix;
556587
const PERL_CONTEXT *ccstack = cxstack;
557588
const PERL_SI *top_si = PL_curstackinfo;
@@ -654,8 +685,10 @@ mro__nextcan(...)
654685
/* Initialize the next::method cache for this stash
655686
if necessary */
656687
selfmeta = HvMROMETA(selfstash);
657-
if(!(nmcache = selfmeta->mro_nextmethod)) {
658-
nmcache = selfmeta->mro_nextmethod = newHV();
688+
nmcache = selfmeta->mro_nextmethod;
689+
if (!nmcache) {
690+
nmcache = newHV();
691+
selfmeta->mro_nextmethod = nmcache;
659692
}
660693
else { /* Use the cached coderef if it exists */
661694
HE* cache_entry = hv_fetch_ent(nmcache, sv, 0, 0);
@@ -678,8 +711,10 @@ mro__nextcan(...)
678711
/* beyond here is just for cache misses, so perf isn't as critical */
679712

680713
stashname_len = subname - fq_subname - 2;
681-
stashname = newSVpvn_flags(fq_subname, stashname_len,
682-
SVs_TEMP | (subname_utf8 ? SVf_UTF8 : 0));
714+
{ /* strs like "Qux::foo" "TTop::foo" show up here */
715+
I32 i32len = subname_utf8 ? -(I32)stashname_len : (I32)stashname_len;
716+
stashname = sv_2mortal(newSVpvn_share(fq_subname, i32len, 0));
717+
}
683718

684719
/* has ourselves at the top of the list */
685720
linear_av = S_mro_get_linear_isa_c3(aTHX_ selfstash, 0);

0 commit comments

Comments
 (0)