This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
revamp pp_entersub()'s CV locating code
authorDavid Mitchell <davem@iabyn.com>
Mon, 13 Jul 2015 11:48:17 +0000 (12:48 +0100)
committerDavid Mitchell <davem@iabyn.com>
Wed, 3 Feb 2016 08:59:37 +0000 (08:59 +0000)
The big switch statement at the top of pp_entersub() is intended
to extract the CV from the passed argument, which might be a CV, a GV,
a ref to a CV, etc etc.

In 5.22.0, an optimisation was added to stashes such that if an entry
contained only a CV, then a ref to the CV was stored in the stash, rather
than a GV. This means that in the common case for a plain sub call, sv is
now a ref to a CV rather than a GV. So update the code to:

1) remove the special-casing of GVs;
2) add special casing for the sv being a non-magical, non-overloaded RV
   pointing to a CV;
3) add special casing for the sv being a CV (true on method calls);
3) move the rare "sv == &PL_sv_yes" test further down the condition chain;
4) generally rearrange the switch cases so that common things fall though
   and uncommon things do a goto;
5) sprinkle more LIKELY() pixie dust.

Part of the intention of special-casing is to avoid doing an indirect
branch by avoiding the switch statement completely.

pp_hot.c

index 5a8fc34..959b09c 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3369,37 +3369,50 @@ PP(pp_entersub)
     I32 old_savestack_ix;
 
     if (UNLIKELY(!sv))
     I32 old_savestack_ix;
 
     if (UNLIKELY(!sv))
-       DIE(aTHX_ "Not a CODE reference");
-    /* This is overwhelmingly the most common case:  */
-    if (!LIKELY(SvTYPE(sv) == SVt_PVGV && (cv = GvCVu((const GV *)sv)))) {
+       goto do_die;
+
+    /* Locate the CV to call:
+     * - most common case: RV->CV: f(), $ref->():
+     *   note that if a sub is compiled before its caller is compiled,
+     *   the stash entry will be a ref to a CV, rather than being a GV.
+     * - second most common case: CV: $ref->method()
+     */
+
+    /* a non-magic-RV -> CV ? */
+    if (LIKELY( (SvFLAGS(sv) & (SVf_ROK|SVs_GMG)) == SVf_ROK)) {
+        cv = MUTABLE_CV(SvRV(sv));
+        if (UNLIKELY(SvOBJECT(cv))) /* might be overloaded */
+            goto do_ref;
+    }
+    else
+        cv = MUTABLE_CV(sv);
+
+    /* a CV ? */
+    if (UNLIKELY(SvTYPE(cv) != SVt_PVCV)) {
+        /* handle all the weird cases */
         switch (SvTYPE(sv)) {
         switch (SvTYPE(sv)) {
+        case SVt_PVLV:
+            if (!isGV_with_GP(sv))
+                goto do_default;
+            /* FALLTHROUGH */
         case SVt_PVGV:
         case SVt_PVGV:
-          we_have_a_glob:
-            if (!(cv = GvCVu((const GV *)sv))) {
+            cv = GvCVu((const GV *)sv);
+            if (UNLIKELY(!cv)) {
                 HV *stash;
                 cv = sv_2cv(sv, &stash, &gv, 0);
                 HV *stash;
                 cv = sv_2cv(sv, &stash, &gv, 0);
-            }
-            if (!cv) {
-                old_savestack_ix = PL_savestack_ix;
-                goto try_autoload;
+                if (!cv) {
+                    old_savestack_ix = PL_savestack_ix;
+                    goto try_autoload;
+                }
             }
             break;
             }
             break;
-        case SVt_PVLV:
-            if(isGV_with_GP(sv)) goto we_have_a_glob;
-            /* FALLTHROUGH */
+
         default:
         default:
-            if (sv == &PL_sv_yes) {            /* unfound import, ignore */
-                if (hasargs)
-                    SP = PL_stack_base + POPMARK;
-                else
-                    (void)POPMARK;
-                if (GIMME_V == G_SCALAR)
-                    PUSHs(&PL_sv_undef);
-                RETURN;
-            }
+          do_default:
             SvGETMAGIC(sv);
             if (SvROK(sv)) {
             SvGETMAGIC(sv);
             if (SvROK(sv)) {
-                if (SvAMAGIC(sv)) {
+              do_ref:
+                if (UNLIKELY(SvAMAGIC(sv))) {
                     sv = amagic_deref_call(sv, to_cv_amg);
                     /* Don't SPAGAIN here.  */
                 }
                     sv = amagic_deref_call(sv, to_cv_amg);
                     /* Don't SPAGAIN here.  */
                 }
@@ -3407,8 +3420,19 @@ PP(pp_entersub)
             else {
                 const char *sym;
                 STRLEN len;
             else {
                 const char *sym;
                 STRLEN len;
-                if (!SvOK(sv))
+                if (UNLIKELY(!SvOK(sv)))
                     DIE(aTHX_ PL_no_usym, "a subroutine");
                     DIE(aTHX_ PL_no_usym, "a subroutine");
+
+                if (UNLIKELY(sv == &PL_sv_yes)) { /* unfound import, ignore */
+                    if (hasargs)
+                        SP = PL_stack_base + POPMARK;
+                    else
+                        (void)POPMARK;
+                    if (GIMME_V == G_SCALAR)
+                        PUSHs(&PL_sv_undef);
+                    RETURN;
+                }
+
                 sym = SvPV_nomg_const(sv, len);
                 if (PL_op->op_private & HINT_STRICT_REFS)
                     DIE(aTHX_ "Can't use string (\"%" SVf32 "\"%s) as a subroutine ref while \"strict refs\" in use", sv, len>32 ? "..." : "");
                 sym = SvPV_nomg_const(sv, len);
                 if (PL_op->op_private & HINT_STRICT_REFS)
                     DIE(aTHX_ "Can't use string (\"%" SVf32 "\"%s) as a subroutine ref while \"strict refs\" in use", sv, len>32 ? "..." : "");
@@ -3416,16 +3440,13 @@ PP(pp_entersub)
                 break;
             }
             cv = MUTABLE_CV(SvRV(sv));
                 break;
             }
             cv = MUTABLE_CV(SvRV(sv));
-            if (SvTYPE(cv) == SVt_PVCV)
+            if (LIKELY(SvTYPE(cv) == SVt_PVCV))
                 break;
             /* FALLTHROUGH */
         case SVt_PVHV:
         case SVt_PVAV:
                 break;
             /* FALLTHROUGH */
         case SVt_PVHV:
         case SVt_PVAV:
+          do_die:
             DIE(aTHX_ "Not a CODE reference");
             DIE(aTHX_ "Not a CODE reference");
-            /* This is the second most common case:  */
-        case SVt_PVCV:
-            cv = MUTABLE_CV(sv);
-            break;
         }
     }
 
         }
     }