Skip to content

Commit 2eaf799

Browse files
author
Father Chrysostomos
committed
Avoid creating GVs when subs are declared
This patch changes ‘sub foo {...}’ declarations to store subroutine references in the stash, to save memory. Typeglobs still notionally exist. Accessing CvGV(cv) will reify them. Hence, currently the savings are lost when a sub call is compiled. $ ./miniperl -e 'sub foo{} BEGIN { warn $::{foo} } foo(); BEGIN { warn $::{foo} }' CODE(0x7f8ef082ad98) at -e line 1. *main::foo at -e line 1. This optimisation is skipped if the subroutine declaration contains a package separator. Concerning the changes in caller.t, this code: sub foo { print +(caller(0))[3],"\n" } my $fooref = delete $::{foo}; $fooref -> (); used to crash in 5.7.3 or thereabouts. It was fixed by 16658 (aka 07b8c80) to produce ‘(unknown)’ instead. Then in 5.13.3 it was changed (by 803f274) to produce ‘main::__ANON__’ instead. So the tests are really checking that we don’t get a crash. I think it is acceptable that it has now changed to ‘main::foo’.
1 parent c831c5e commit 2eaf799

File tree

11 files changed

+225
-53
lines changed

11 files changed

+225
-53
lines changed

embed.fnc

+1-1
Original file line numberDiff line numberDiff line change
@@ -1941,7 +1941,7 @@ s |OP* |newGIVWHENOP |NULLOK OP* cond|NN OP *block \
19411941
|I32 enter_opcode|I32 leave_opcode \
19421942
|PADOFFSET entertarg
19431943
s |OP* |ref_array_or_hash|NULLOK OP* cond
1944-
s |void |process_special_blocks |I32 floor \
1944+
s |bool |process_special_blocks |I32 floor \
19451945
|NN const char *const fullname\
19461946
|NN GV *const gv|NN CV *const cv
19471947
s |void |clear_special_blocks |NN const char *const fullname\

gv.c

+26-5
Original file line numberDiff line numberDiff line change
@@ -260,17 +260,25 @@ GV *
260260
Perl_cvgv_from_hek(pTHX_ CV *cv)
261261
{
262262
GV *gv;
263+
SV **svp;
263264
PERL_ARGS_ASSERT_CVGV_FROM_HEK;
264265
assert(SvTYPE(cv) == SVt_PVCV);
265266
if (!CvSTASH(cv)) return NULL;
266267
ASSUME(CvNAME_HEK(cv));
267-
gv = (GV *)newSV(0);
268-
gv_init_pvn(gv, CvSTASH(cv), HEK_KEY(CvNAME_HEK(cv)),
268+
svp = hv_fetchhek(CvSTASH(cv), CvNAME_HEK(cv), 0);
269+
gv = MUTABLE_GV(svp && *svp ? *svp : newSV(0));
270+
if (!isGV(gv))
271+
gv_init_pvn(gv, CvSTASH(cv), HEK_KEY(CvNAME_HEK(cv)),
269272
HEK_LEN(CvNAME_HEK(cv)),
270273
SVf_UTF8 * !!HEK_UTF8(CvNAME_HEK(cv)));
274+
if (!CvNAMED(cv)) { /* gv_init took care of it */
275+
assert (SvANY(cv)->xcv_gv_u.xcv_gv == gv);
276+
return gv;
277+
}
271278
unshare_hek(CvNAME_HEK(cv));
272279
CvNAMED_off(cv);
273280
SvANY(cv)->xcv_gv_u.xcv_gv = gv;
281+
if (svp && *svp) SvREFCNT_inc_simple_void_NN(gv);
274282
CvCVGV_RC_on(cv);
275283
return gv;
276284
}
@@ -370,10 +378,9 @@ Perl_gv_init_pvn(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len, U32 flag
370378
assert (!(proto && has_constant));
371379

372380
if (has_constant) {
373-
/* The constant has to be a simple scalar type. */
381+
/* The constant has to be a scalar, array or subroutine. */
374382
switch (SvTYPE(has_constant)) {
375383
case SVt_PVHV:
376-
case SVt_PVCV:
377384
case SVt_PVFM:
378385
case SVt_PVIO:
379386
Perl_croak(aTHX_ "Cannot convert a reference to %s to typeglob",
@@ -409,7 +416,21 @@ Perl_gv_init_pvn(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len, U32 flag
409416
gv_name_set(gv, name, len, GV_ADD | ( flags & SVf_UTF8 ? SVf_UTF8 : 0 ));
410417
if (flags & GV_ADDMULTI || doproto) /* doproto means it */
411418
GvMULTI_on(gv); /* _was_ mentioned */
412-
if (doproto) {
419+
if (has_constant && SvTYPE(has_constant) == SVt_PVCV) {
420+
/* Not actually a constant. Just a regular sub. */
421+
CV * const cv = (CV *)has_constant;
422+
GvCV_set(gv,cv);
423+
if (CvSTASH(cv) == stash && (
424+
CvNAME_HEK(cv) == GvNAME_HEK(gv)
425+
|| ( HEK_LEN(CvNAME_HEK(cv)) == HEK_LEN(GvNAME_HEK(gv))
426+
&& HEK_FLAGS(CvNAME_HEK(cv)) != HEK_FLAGS(GvNAME_HEK(gv))
427+
&& HEK_UTF8(CvNAME_HEK(cv)) == HEK_UTF8(GvNAME_HEK(gv))
428+
&& memEQ(HEK_KEY(CvNAME_HEK(cv)), GvNAME(gv), GvNAMELEN(gv))
429+
)
430+
))
431+
CvGV_set(cv,gv);
432+
}
433+
else if (doproto) {
413434
CV *cv;
414435
if (has_constant) {
415436
/* newCONSTSUB takes ownership of the reference from us. */

0 commit comments

Comments
 (0)