refactor sv_add_backref

-don't assign NULL twice to var mg, seen in machine code
-use sv_magicext to get MAGIC * instead of a 2nd call to mg_find, less
 passes through the linked list is faster
-move the mg_flags MGf_REFCOUNTED statement to before av, to reduce scope
 of liveness for var mg, it is never used again in the function
-av can not be null since it was just created, use
 SvREFCNT_inc_simple_void_NN
-on a tsv, that is being upgraded to be an AV backref holder, call
 av_extend only once with the correct slice count based on whether tsv was
 a SV backref holder or not previously
-since on a newly AV backref tsv, the number of backrefs will only be 1 or
 2, and if we made an AV type backref holder, doing the SV type
 optimization is now impossible, so don't check av for NULL when we know
 it wont be if we just made the AV, and don't do the bounds check and 2nd
 av_extend either since the AV is new and under our control
-if tsv is already an AV type backref, the AV can have 1 to infinity
 elements so do the bounds check and av_extend as before

It is intentional that on the new AV type holder branch, that after
"av_extend(av, *svp ? 2 : 1);" there are no further function calls in the
execution path until the return.

1 files changed, 18 insertions, 20 deletions

diff --git a/sv.c b/sv.c
index 489c5ec6a0..99b1182bd0 100644
--- a/sv.c
+++ b/sv.c
@@ -5702,12 +5702,10 @@ Perl_sv_add_backref(pTHX_ SV *const tsv, SV *const sv)
     if (SvTYPE(tsv) == SVt_PVHV) {
 	svp = (SV**)Perl_hv_backreferences_p(aTHX_ MUTABLE_HV(tsv));
     } else {
-	if (! ((mg =
-	    (SvMAGICAL(tsv) ? mg_find(tsv, PERL_MAGIC_backref) : NULL))))
-	{
-	    sv_magic(tsv, NULL, PERL_MAGIC_backref, NULL, 0);
-	    mg = mg_find(tsv, PERL_MAGIC_backref);
-	}
+        if (SvMAGICAL(tsv))
+            mg = mg_find(tsv, PERL_MAGIC_backref);
+	if (!mg)
+            mg = sv_magicext(tsv, NULL, PERL_MAGIC_backref, &PL_vtbl_backref, NULL, 0);
 	svp = &(mg->mg_obj);
     }
 
@@ -5717,32 +5715,32 @@ Perl_sv_add_backref(pTHX_ SV *const tsv, SV *const sv)
 	|| (*svp && SvTYPE(*svp) != SVt_PVAV)
     ) {
 	/* create array */
+	if (mg)
+	    mg->mg_flags |= MGf_REFCOUNTED;
 	av = newAV();
 	AvREAL_off(av);
-	SvREFCNT_inc_simple_void(av);
+	SvREFCNT_inc_simple_void_NN(av);
 	/* av now has a refcnt of 2; see discussion above */
+	av_extend(av, *svp ? 2 : 1);
 	if (*svp) {
 	    /* move single existing backref to the array */
-	    av_extend(av, 1);
 	    AvARRAY(av)[++AvFILLp(av)] = *svp; /* av_push() */
 	}
 	*svp = (SV*)av;
-	if (mg)
-	    mg->mg_flags |= MGf_REFCOUNTED;
     }
-    else
+    else {
 	av = MUTABLE_AV(*svp);
-
-    if (!av) {
-	/* optimisation: store single backref directly in HvAUX or mg_obj */
-	*svp = sv;
-	return;
+        if (!av) {
+            /* optimisation: store single backref directly in HvAUX or mg_obj */
+            *svp = sv;
+            return;
+        }
+        assert(SvTYPE(av) == SVt_PVAV);
+        if (AvFILLp(av) >= AvMAX(av)) {
+            av_extend(av, AvFILLp(av)+1);
+        }
     }
     /* push new backref */
-    assert(SvTYPE(av) == SVt_PVAV);
-    if (AvFILLp(av) >= AvMAX(av)) {
-        av_extend(av, AvFILLp(av)+1);
-    }
     AvARRAY(av)[++AvFILLp(av)] = sv; /* av_push() */
 }