eliminate the argarray field from the CX struct

At the end of a normal sub call there are often 3 AVs of interest
associated with @_; these are:

    cx->blk_sub.savearray  the caller's @_
    GvAV(PL_defgv)         the current sub's current @_
    cx->blk_sub.argarray   the current sub's original @_

Note that those last two can differ: if for example the sub does

    local *_ = [];

Each sub's arg array is created and stored in pad slot 0 at the time
the sub is created. When a sub is called, pp_entersub() does

    cx->blk_sub.argarray = PL_curpad[0]

The argarray field is used in two main places:

* at subroutine exit, to clear/abandon the sub's original @_;
* in pp_caller, to set @DB::args to the caller(n)'s args.

In the first case, the last few commits have arranged things so that at
that point, PL_curpad always points to the current pad of the sub being
exited from. So we can just use PL_curpad[0] rather than
cx->blk_sub.argarray.

In the second case (which is not performance critical), we can just use
    cx->blk_sub.cv
and
    cx->blk_sub.olddepth+1
to find the pad of that call frame's CV.

So we can eliminate the argarray field. This saves a write during
a sub call, and potentially reduces the size of the context struct.

It also makes the code marginally less confusing: there are now 3 arrays
pointed to from 3 places, rather than 3 arrays pointed to from 4 places.

The asserts added in the previous commit confirmed that using argarray
is the same as using PL_curpad[0]; They also confirmed that the current
PL_curpad matches the current CV at the current depth. Those latter
asserts are kept for now.

diff --git a/cop.h b/cop.h
index 0077fe4..12ff11e 100644 (file)
--- a/cop.h
+++ b/cop.h
@@ -554,7 +554,6 @@ struct block_sub {
     CV *       cv;
     /* Above here is the same for sub and format.  */
     AV *       savearray;
-    AV *       argarray;
     I32                olddepth;
     PAD                *prevcomppad; /* the caller's PL_comppad */
 };
@@ -653,25 +652,22 @@ struct block_format {
                CopSTASHPV((COP*)CvSTART((const CV*)cx->blk_sub.cv)));  \
                                                                        \
        if (CxHASARGS(cx)) {                                            \
-            assert(cx->blk_sub.argarray == (AV*)PL_curpad[0]);          \
+            AV *av = MUTABLE_AV(PAD_SVl(0));                            \
             assert(AvARRAY(MUTABLE_AV(                                  \
                 PadlistARRAY(CvPADLIST(cx->blk_sub.cv))[                \
                         CvDEPTH(cx->blk_sub.cv)])) == PL_curpad);       \
            POP_SAVEARRAY();                                            \
            /* abandon @_ if it got reified */                          \
-           if (AvREAL(cx->blk_sub.argarray)) {                         \
-               const SSize_t fill = AvFILLp(cx->blk_sub.argarray);     \
-               SvREFCNT_dec_NN(cx->blk_sub.argarray);                  \
-               cx->blk_sub.argarray = newAV();                         \
-               av_extend(cx->blk_sub.argarray, fill);                  \
-               AvREIFY_only(cx->blk_sub.argarray);                     \
-                (AvARRAY(MUTABLE_AV(                                    \
-                    PadlistARRAY(CvPADLIST(cx->blk_sub.cv))[            \
-                            CvDEPTH(cx->blk_sub.cv)])))[0] =            \
-                    MUTABLE_SV(cx->blk_sub.argarray);                   \
+           if (AvREAL(av)) {                                           \
+               const SSize_t fill = AvFILLp(av);                       \
+               SvREFCNT_dec_NN(av);                                    \
+                av = newAV();                                           \
+               av_extend(av, fill);                                    \
+               AvREIFY_only(av);                                       \
+                PAD_SVl(0) = MUTABLE_SV(av);                            \
            }                                                           \
            else {                                                      \
-               CLEAR_ARGARRAY(cx->blk_sub.argarray);                   \
+               CLEAR_ARGARRAY(av);                                     \
            }                                                           \
        }                                                               \
         }                                                               \

diff --git a/pp.c b/pp.c
index 5b5347d..30a2dd0 100644 (file)
--- a/pp.c
+++ b/pp.c
@@ -6444,12 +6444,11 @@ PP(pp_coreargs)
                PERL_CONTEXT *cx = &cxstack[cxstack_ix];
                POP_SAVEARRAY();
                cx->cx_type &= ~ CXp_HASARGS;
-                assert(cx->blk_sub.argarray == (AV*)PL_curpad[0]);
                 assert(AvARRAY(MUTABLE_AV(
                     PadlistARRAY(CvPADLIST(cx->blk_sub.cv))[
                             CvDEPTH(cx->blk_sub.cv)])) == PL_curpad);
 
-               assert(!AvREAL(cx->blk_sub.argarray));
+               assert(!AvREAL(MUTABLE_AV(PAD_SVl(0))));
            }
          }
          break;

diff --git a/pp_ctl.c b/pp_ctl.c
index ed846ba..f89aad0 100644 (file)
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -1865,13 +1865,12 @@ PP(pp_caller)
     if (CxTYPE(cx) == CXt_SUB && CxHASARGS(cx)
        && CopSTASH_eq(PL_curcop, PL_debstash))
     {
-       AV * const ary = cx->blk_sub.argarray;
+        /* slot 0 of the pad contains the original @_ */
+       AV * const ary = MUTABLE_AV(AvARRAY(MUTABLE_AV(
+                            PadlistARRAY(CvPADLIST(cx->blk_sub.cv))[
+                                cx->blk_sub.olddepth+1]))[0]);
        const SSize_t off = AvARRAY(ary) - AvALLOC(ary);
 
-        assert((AV*)((AvARRAY(MUTABLE_AV(
-            PadlistARRAY(CvPADLIST(cx->blk_sub.cv))[
-                cx->blk_sub.olddepth+1])))[0]) == cx->blk_sub.argarray);
-
        Perl_init_dbargs(aTHX);
 
        if (AvMAX(PL_dbargs) < AvFILLp(ary) + off)
@@ -2731,9 +2730,7 @@ PP(pp_goto)
             /* partial unrolled POPSUB(): */
 
            if (CxTYPE(cx) == CXt_SUB && CxHASARGS(cx)) {
-               AV* av = cx->blk_sub.argarray;
-
-                assert(cx->blk_sub.argarray == (AV*)PL_curpad[0]);
+               AV* av = MUTABLE_AV(PAD_SVl(0));
                 assert(AvARRAY(MUTABLE_AV(
                     PadlistARRAY(CvPADLIST(cx->blk_sub.cv))[
                             CvDEPTH(cx->blk_sub.cv)])) == PL_curpad);
@@ -2744,7 +2741,7 @@ PP(pp_goto)
                    SvREFCNT_dec(av);
                    av = newAV();
                    AvREIFY_only(av);
-                   PAD_SVl(0) = MUTABLE_SV(cx->blk_sub.argarray = av);
+                   PAD_SVl(0) = (SV*)av;
                }
                else CLEAR_ARGARRAY(av);
            }
@@ -2855,7 +2852,8 @@ PP(pp_goto)
                PAD_SET_CUR_NOSAVE(padlist, CvDEPTH(cv));
                if (CxHASARGS(cx))
                {
-                   /* cx->blk_sub.argarray has no reference count, so we
+                   /* XXX
+                       cx->blk_sub.argarray has no reference count, so we
                       need something to hang on to our argument array so
                       that cx->blk_sub.argarray does not end up pointing
                       to freed memory as the result of undef *_.  So put
@@ -2865,7 +2863,6 @@ PP(pp_goto)
                        SvREFCNT_dec(PAD_SVl(0));
                        PAD_SVl(0) = (SV *)arg;
                    }
-                    cx->blk_sub.argarray = (AV*)PAD_SVl(0);
 
                    /* GvAV(PL_defgv) might have been modified on scope
                       exit, so restore it. */

diff --git a/pp_hot.c b/pp_hot.c
index c7dfca1..edd4cf7 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3490,7 +3490,6 @@ PP(pp_entersub)
            defavp = &GvAV(PL_defgv);
            cx->blk_sub.savearray = *defavp;
            *defavp = MUTABLE_AV(SvREFCNT_inc_simple_NN(av));
-           cx->blk_sub.argarray = av;
             items = SP - MARK;
 
            if (UNLIKELY(items - 1 > AvMAX(av))) {

diff --git a/pp_sort.c b/pp_sort.c
index 73f520d..1e48042 100644 (file)
--- a/pp_sort.c
+++ b/pp_sort.c
@@ -1688,7 +1688,6 @@ PP(pp_sort)
 
                        cx->blk_sub.savearray = GvAV(PL_defgv);
                        GvAV(PL_defgv) = MUTABLE_AV(SvREFCNT_inc_simple(av));
-                       cx->blk_sub.argarray = av;
                    }
 
                }

diff --git a/sv.c b/sv.c
index 15c0c54..87c48ac 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -13944,10 +13944,8 @@ Perl_cx_dup(pTHX_ PERL_CONTEXT *cxs, I32 ix, I32 max, CLONE_PARAMS* param)
            case CXt_SUB:
                ncx->blk_sub.cv         = cv_dup_inc(ncx->blk_sub.cv, param);
                if(CxHASARGS(ncx)){
-                   ncx->blk_sub.argarray = av_dup_inc(ncx->blk_sub.argarray,param);
                    ncx->blk_sub.savearray = av_dup_inc(ncx->blk_sub.savearray,param);
                } else {
-                   ncx->blk_sub.argarray = NULL;
                    ncx->blk_sub.savearray = NULL;
                }
                ncx->blk_sub.prevcomppad = (PAD*)ptr_table_fetch(PL_ptr_table,