Skip to content

Commit b492a59

Browse files
committed
Optimise magic handling in save* and leave_scope
There are several places that have code similar to if (SvMAGICAL(sv)) { PL_localizing = 2; SvSETMAGIC(sv) PL_localizing = 0; } The condition is sub-optimal (it should only test for set magic), and the SvSETMAGIC repeats a similar test. Other places didn't have the outer condition, so they set PL_localizing twice even when not needed.
1 parent 03dba56 commit b492a59

File tree

1 file changed

+25
-19
lines changed

1 file changed

+25
-19
lines changed

scope.c

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,11 @@ Perl_save_scalar(pTHX_ GV *gv)
214214

215215
PERL_ARGS_ASSERT_SAVE_SCALAR;
216216

217-
PL_localizing = 1;
218-
SvGETMAGIC(*sptr);
219-
PL_localizing = 0;
217+
if (SvGMAGICAL(*sptr)) {
218+
PL_localizing = 1;
219+
(void)mg_get(*sptr);
220+
PL_localizing = 0;
221+
}
220222
save_pushptrptr(SvREFCNT_inc_simple(gv), SvREFCNT_inc(*sptr), SAVEt_SV);
221223
return save_scalar_at(sptr, SAVEf_SETMAGIC); /* XXX - FIXME - see #60360 */
222224
}
@@ -808,9 +810,11 @@ Perl_leave_scope(pTHX_ I32 base)
808810
switch (type) {
809811
case SAVEt_ITEM: /* normal string */
810812
sv_replace(ARG1_SV, ARG0_SV);
811-
PL_localizing = 2;
812-
SvSETMAGIC(ARG1_SV);
813-
PL_localizing = 0;
813+
if (SvSMAGICAL(ARG1_SV)) {
814+
PL_localizing = 2;
815+
mg_set(ARG1_SV);
816+
PL_localizing = 0;
817+
}
814818
break;
815819

816820
/* This would be a mathom, but Perl_save_svref() calls a static
@@ -828,9 +832,11 @@ Perl_leave_scope(pTHX_ I32 base)
828832
SV * const sv = *svp;
829833
*svp = ARG0_SV;
830834
SvREFCNT_dec(sv);
831-
PL_localizing = 2;
832-
SvSETMAGIC(ARG0_SV);
833-
PL_localizing = 0;
835+
if (SvSMAGICAL(ARG0_SV)) {
836+
PL_localizing = 2;
837+
mg_set(ARG0_SV);
838+
PL_localizing = 0;
839+
}
834840
SvREFCNT_dec(ARG0_SV);
835841
SvREFCNT_dec(refsv);
836842
break;
@@ -884,21 +890,21 @@ Perl_leave_scope(pTHX_ I32 base)
884890
case SAVEt_AV: /* array reference */
885891
SvREFCNT_dec(GvAV(ARG1_GV));
886892
GvAV(ARG1_GV) = ARG0_AV;
887-
if (SvMAGICAL(ARG0_AV)) {
888-
PL_localizing = 2;
889-
SvSETMAGIC(MUTABLE_SV(ARG0_AV));
890-
PL_localizing = 0;
891-
}
893+
if (SvSMAGICAL(ARG0_SV)) {
894+
PL_localizing = 2;
895+
mg_set(ARG0_SV);
896+
PL_localizing = 0;
897+
}
892898
SvREFCNT_dec(ARG1_GV);
893899
break;
894900
case SAVEt_HV: /* hash reference */
895901
SvREFCNT_dec(GvHV(ARG1_GV));
896902
GvHV(ARG1_GV) = ARG0_HV;
897-
if (SvMAGICAL(ARG0_HV)) {
898-
PL_localizing = 2;
899-
SvSETMAGIC(MUTABLE_SV(ARG0_HV));
900-
PL_localizing = 0;
901-
}
903+
if (SvSMAGICAL(ARG0_SV)) {
904+
PL_localizing = 2;
905+
mg_set(ARG0_SV);
906+
PL_localizing = 0;
907+
}
902908
SvREFCNT_dec(ARG1_GV);
903909
break;
904910
case SAVEt_INT_SMALL:

0 commit comments

Comments
 (0)