Skip to content

Commit c005319

Browse files
mauketonycoz
authored andcommitted
Reapply "op.c: re-enable coderef-in-stash optimization"
When seeing 'sub foo { ... }', perl does not need to allocate a full typeglob (with the subroutine being stored in the CODE slot). Instead, it can just store a coderef directly in the stash. This optimization was first announced in perl5220delta: > - Subroutines in packages no longer need to be stored in typeglobs: > declaring a subroutine will now put a simple sub reference directly > in the stash if possible, saving memory. The typeglob still > notionally exists, so accessing it will cause the stash entry to be > upgraded to a typeglob (i.e. this is just an internal implementation > detail). This optimization does not currently apply to XSUBs or > exported subroutines, and method calls will undo it, since they > cache things in typeglobs. [GH #13392] However, due to a bug this optimization didn't actually work except for package main (GH #15671). The issue was fixed in v5.27.5, but the fix was backed out again in v5.27.9 because of CPAN breakage. It was enabled again in v5.41.9, but backed out again again in v5.41.10 because of too much CPAN breakage this late in the development cycle. This reverts commit 51f2346. Also fix class_cleanup_definition: We actually store a reference-to-CV in the stash, not a bare CV, so we need to dereference once to recognize methods defined this way. See #23131 for details (and expected CPAN breakage).
1 parent 5079c52 commit c005319

File tree

3 files changed

+8
-11
lines changed

3 files changed

+8
-11
lines changed

class.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -697,10 +697,13 @@ S_class_cleanup_definition(pTHX_ HV *stash) {
697697
SvREFCNT_dec_NN(cv);
698698
GvCV_set((GV*)entry, NULL);
699699
}
700-
else if (SvTYPE(entry) == SVt_PVCV
701-
&& (CvIsMETHOD((CV*)entry) || memEQs(kpv, klen, "new"))) {
702-
(void)hv_delete(stash, kpv, HeUTF8(he) ? -(I32)klen : (I32)klen,
703-
G_DISCARD);
700+
else if (SvROK(entry)) {
701+
SV *sv = SvRV(entry);
702+
if (SvTYPE(sv) == SVt_PVCV
703+
&& (CvIsMETHOD((CV*)sv) || memEQs(kpv, klen, "new"))) {
704+
(void)hv_delete(stash, kpv, HeUTF8(he) ? -(I32)klen : (I32)klen,
705+
G_DISCARD);
706+
}
704707
}
705708
}
706709
++PL_sub_generation;

op.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11102,12 +11102,9 @@ Perl_newATTRSUB_x(pTHX_ I32 floor, OP *o, OP *proto, OP *attrs,
1110211102
Also, we may be called from load_module at run time, so
1110311103
PL_curstash (which sets CvSTASH) may not point to the stash the
1110411104
sub is stored in. */
11105-
/* XXX This optimization is currently disabled for packages other
11106-
than main, since there was too much CPAN breakage. */
1110711105
const I32 flags =
1110811106
ec ? GV_NOADD_NOINIT
1110911107
: (IN_PERL_RUNTIME && PL_curstash != CopSTASH(PL_curcop))
11110-
|| PL_curstash != PL_defstash
1111111108
|| memchr(name, ':', namlen) || memchr(name, '\'', namlen)
1111211109
? gv_fetch_flags
1111311110
: GV_ADDMULTI | GV_NOINIT | GV_NOTQUAL;

t/op/sub.t

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -398,10 +398,7 @@ is ref($main::{rt_129916}), 'CODE', 'simple sub stored as CV in stash (main::)';
398398
package RT129916;
399399
sub foo { 42 }
400400
}
401-
{
402-
local $::TODO = "disabled for now";
403-
is ref($RT129916::{foo}), 'CODE', 'simple sub stored as CV in stash (non-main::)';
404-
}
401+
is ref($RT129916::{foo}), 'CODE', 'simple sub stored as CV in stash (non-main::)';
405402

406403
# Calling xsub via ampersand syntax when @_ has holes
407404
SKIP: {

0 commit comments

Comments
 (0)