From bc8305cdd3378420a2a9ecaf19e75ae6c6f23e98 Mon Sep 17 00:00:00 2001 From: bulk88 Date: Tue, 15 Jul 2025 02:04:24 -0400 Subject: [PATCH] pp.c pp_argelem() dont make SV* tmp bufs in a loop to strip GMG Alloc the mortal string scratch pad SV* only once and reuse it for every iteration to strip GMG from each SV* key. hv_store_ent() never RC++s ownership of the callers SV* key arg. Although it makes no difference either way for pp_argelem(), because it is an internal detail of sv.c Use any type of PV COW if available. If a shared HEK COW is delivered into the tmp SV*, even better b/c of hv_store_ent()'s optimization for that. Worst case keynomg has a Newx() buffer that only reallocs 1x or 2x before flatlining for the rest of the loop which is still very good. I think, but I can't confirm, that this copy code was added b/c of a GMG getter firing order issue so val doesn't GMG b4 key, or because maybe its not possible to prevent hv_common() keysv GMG from firing so SvGETMAGIC() can't be used to reverse GMG firing order, or maybe if a keysv GMG getter throws an exception inside hv_common(), tmpsv is leaked. The make multiple mortal SV PVs in a loop code is from day 1 of this opcode in commit 4fa06845e7 - davem - 7/9/2016 5:41:08 AM "add OP_ARGELEM, OP_ARGDEFELEM, OP_ARGCHECK ops" --- pp.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pp.c b/pp.c index 189b2ddbdd34..7c0612c1b3d3 100644 --- a/pp.c +++ b/pp.c @@ -7824,6 +7824,7 @@ PP_wrapped(pp_argelem, return o->op_next; assert(argc % 2 == 0); + SV* keynomg = NULL; i = 0; while (argc) { SV **svp = av_fetch(defav, ix + i++, FALSE); @@ -7832,8 +7833,13 @@ PP_wrapped(pp_argelem, SV *val = svp ? *svp : &PL_sv_undef; argc -= 2; - if (UNLIKELY(SvGMAGICAL(key))) - key = sv_mortalcopy(key); + if (UNLIKELY(SvGMAGICAL(key))) { + if (!keynomg) + keynomg = sv_mortalcopy(key); + else + sv_setsv_flags(keynomg, key, SV_GMAGIC|SV_DO_COW_SVSETSV); + key = keynomg; + } SV *tmpsv = newSVsv_flags(val, SV_GMAGIC|SV_DO_COW_SVSETSV); hv_store_ent((HV*)targ, key, tmpsv, 0); TAINT_NOT;