Skip to content

Commit 5239d5c

Browse files
committed
make sv_clear() iterate over AVs
In sv_clear(), rather than calling av_undef(), iterate over the AV's elements. This is the first stage in making sv_clear() non-recursive, and thus non-stack-blowing when freeing deeply nested structures. Since we no longer have the stack to maintain the chain of AVs currently being iterated over, we instead store a pointer to the previous AV in the AvARRAY[AvMAX] slot of the currently-being-iterated AV. Since our first action is to pop the first SV, that slot is guaranteed to be free, and (in theory) nothing should be messing with the AV while we iterate over its elements, so that slot should remain undisturbed.
1 parent de61950 commit 5239d5c

File tree

3 files changed

+97
-13
lines changed

3 files changed

+97
-13
lines changed

embed.fnc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1160,7 +1160,7 @@ Apd |void |sv_chop |NN SV *const sv|NULLOK const char *const ptr
11601160
pd |I32 |sv_clean_all
11611161
: Used only in perl.c
11621162
pd |void |sv_clean_objs
1163-
Apd |void |sv_clear |NN SV *const sv
1163+
Apd |void |sv_clear |NN SV *const orig_sv
11641164
Apd |I32 |sv_cmp |NULLOK SV *const sv1|NULLOK SV *const sv2
11651165
Apd |I32 |sv_cmp_flags |NULLOK SV *const sv1|NULLOK SV *const sv2|const I32 flags
11661166
Apd |I32 |sv_cmp_locale |NULLOK SV *const sv1|NULLOK SV *const sv2

proto.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3897,10 +3897,10 @@ PERL_CALLCONV void Perl_sv_chop(pTHX_ SV *const sv, const char *const ptr)
38973897

38983898
PERL_CALLCONV I32 Perl_sv_clean_all(pTHX);
38993899
PERL_CALLCONV void Perl_sv_clean_objs(pTHX);
3900-
PERL_CALLCONV void Perl_sv_clear(pTHX_ SV *const sv)
3900+
PERL_CALLCONV void Perl_sv_clear(pTHX_ SV *const orig_sv)
39013901
__attribute__nonnull__(pTHX_1);
39023902
#define PERL_ARGS_ASSERT_SV_CLEAR \
3903-
assert(sv)
3903+
assert(orig_sv)
39043904

39053905
PERL_CALLCONV I32 Perl_sv_cmp(pTHX_ SV *const sv1, SV *const sv2);
39063906
PERL_CALLCONV I32 Perl_sv_cmp_flags(pTHX_ SV *const sv1, SV *const sv2, const I32 flags);

sv.c

Lines changed: 94 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5814,15 +5814,26 @@ instead.
58145814
*/
58155815

58165816
void
5817-
Perl_sv_clear(pTHX_ register SV *const sv)
5817+
Perl_sv_clear(pTHX_ SV *const orig_sv)
58185818
{
58195819
dVAR;
5820-
const U32 type = SvTYPE(sv);
5821-
const struct body_details *const sv_type_details
5822-
= bodies_by_type + type;
58235820
HV *stash;
5821+
U32 type;
5822+
const struct body_details *sv_type_details;
5823+
SV* iter_sv = NULL;
5824+
SV* next_sv = NULL;
5825+
register SV *sv = orig_sv;
58245826

58255827
PERL_ARGS_ASSERT_SV_CLEAR;
5828+
5829+
/* within this loop, sv is the SV currently being freed, and
5830+
* iter_sv is the most recent AV or whatever that's being iterated
5831+
* over to provide more SVs */
5832+
5833+
while (sv) {
5834+
5835+
type = SvTYPE(sv);
5836+
58265837
assert(SvREFCNT(sv) == 0);
58275838
assert(SvTYPE(sv) != SVTYPEMASK);
58285839

@@ -5833,7 +5844,7 @@ Perl_sv_clear(pTHX_ register SV *const sv)
58335844
goto free_rv;
58345845
SvFLAGS(sv) &= SVf_BREAK;
58355846
SvFLAGS(sv) |= SVTYPEMASK;
5836-
return;
5847+
goto free_head;
58375848
}
58385849

58395850
if (SvOBJECT(sv)) {
@@ -5885,7 +5896,7 @@ Perl_sv_clear(pTHX_ register SV *const sv)
58855896
Perl_croak(aTHX_ "DESTROY created new reference to dead object '%s'",
58865897
HvNAME_get(stash));
58875898
/* DESTROY gave object new lease on life */
5888-
return;
5899+
goto get_next_sv;
58895900
}
58905901
}
58915902

@@ -5942,11 +5953,23 @@ Perl_sv_clear(pTHX_ register SV *const sv)
59425953
hv_undef(MUTABLE_HV(sv));
59435954
break;
59445955
case SVt_PVAV:
5945-
if (PL_comppad == MUTABLE_AV(sv)) {
5946-
PL_comppad = NULL;
5947-
PL_curpad = NULL;
5956+
{
5957+
AV* av = MUTABLE_AV(sv);
5958+
if (PL_comppad == av) {
5959+
PL_comppad = NULL;
5960+
PL_curpad = NULL;
5961+
}
5962+
if (AvREAL(av) && AvFILLp(av) > -1) {
5963+
next_sv = AvARRAY(av)[AvFILLp(av)--];
5964+
/* save old iter_sv in top-most slot of AV,
5965+
* and pray that it doesn't get wiped in the meantime */
5966+
AvARRAY(av)[AvMAX(av)] = iter_sv;
5967+
iter_sv = sv;
5968+
goto get_next_sv; /* process this new sv */
5969+
}
5970+
Safefree(AvALLOC(av));
59485971
}
5949-
av_undef(MUTABLE_AV(sv));
5972+
59505973
break;
59515974
case SVt_PVLV:
59525975
if (LvTYPE(sv) == 'T') { /* for tie: return HE to pool */
@@ -6029,16 +6052,77 @@ Perl_sv_clear(pTHX_ register SV *const sv)
60296052
break;
60306053
}
60316054

6055+
free_body:
6056+
60326057
SvFLAGS(sv) &= SVf_BREAK;
60336058
SvFLAGS(sv) |= SVTYPEMASK;
60346059

6060+
sv_type_details = bodies_by_type + type;
60356061
if (sv_type_details->arena) {
60366062
del_body(((char *)SvANY(sv) + sv_type_details->offset),
60376063
&PL_body_roots[type]);
60386064
}
60396065
else if (sv_type_details->body_size) {
60406066
safefree(SvANY(sv));
60416067
}
6068+
6069+
free_head:
6070+
/* caller is responsible for freeing the head of the original sv */
6071+
if (sv != orig_sv && !SvREFCNT(sv))
6072+
del_SV(sv);
6073+
6074+
/* grab and free next sv, if any */
6075+
get_next_sv:
6076+
while (1) {
6077+
sv = NULL;
6078+
if (next_sv) {
6079+
sv = next_sv;
6080+
next_sv = NULL;
6081+
}
6082+
else if (!iter_sv) {
6083+
break;
6084+
} else if (SvTYPE(iter_sv) == SVt_PVAV) {
6085+
AV *const av = (AV*)iter_sv;
6086+
if (AvFILLp(av) > -1) {
6087+
sv = AvARRAY(av)[AvFILLp(av)--];
6088+
}
6089+
else { /* no more elements of current AV to free */
6090+
sv = iter_sv;
6091+
type = SvTYPE(sv);
6092+
/* restore previous value, squirrelled away */
6093+
iter_sv = AvARRAY(av)[AvMAX(av)];
6094+
Safefree(AvALLOC(av));
6095+
goto free_body;
6096+
}
6097+
}
6098+
6099+
/* unrolled SvREFCNT_dec and sv_free2 follows: */
6100+
6101+
if (!sv)
6102+
continue;
6103+
if (!SvREFCNT(sv)) {
6104+
sv_free(sv);
6105+
continue;
6106+
}
6107+
if (--(SvREFCNT(sv)))
6108+
continue;
6109+
#ifdef DEBUGGING
6110+
if (SvTEMP(sv)) {
6111+
Perl_ck_warner_d(aTHX_ packWARN(WARN_DEBUGGING),
6112+
"Attempt to free temp prematurely: SV 0x%"UVxf
6113+
pTHX__FORMAT, PTR2UV(sv) pTHX__VALUE);
6114+
continue;
6115+
}
6116+
#endif
6117+
if (SvREADONLY(sv) && SvIMMORTAL(sv)) {
6118+
/* make sure SvREFCNT(sv)==0 happens very seldom */
6119+
SvREFCNT(sv) = (~(U32)0)/2;
6120+
continue;
6121+
}
6122+
break;
6123+
} /* while 1 */
6124+
6125+
} /* while sv */
60426126
}
60436127

60446128
/*

0 commit comments

Comments
 (0)