Stop using PL_sortstash

Originally, the value of PL_sortstash was used to determine whether
PL_firstgv and PL_secondgv (which point to *a and *b in the current
package) needed to be updated (involving symbol lookup).  PL_sortstash
would contain the last package sorted in.  If PL_sortstash did not
point to the current stash, then they would be updated, along with
PL_sortstash.

Sort was made reëntrant in 2000 in commit 8e664e1028b.  This involved
saving and restoring the values of PL_firstgv and PL_secondgv via the
savestack.  The logic introduced by that commit was thus:

+               if (PL_sortstash != stash || !PL_firstgv || !PL_secondgv) {
+                   SAVESPTR(PL_firstgv);
+                   SAVESPTR(PL_secondgv);
+                   PL_firstgv = gv_fetchpv("a", TRUE, SVt_PV);
+                   PL_secondgv = gv_fetchpv("b", TRUE, SVt_PV);
+                   PL_sortstash = stash;
+               }

PL_firstgv and PL_secondgv would be null if no sort were already hap-
pening, causing this block to be executed.  For a nested sort, this
would only be executed if the nested sort were in a different package
from the outer sort.

Because the value of PL_sortstash was not being restored, this block
would not trigger the second time the outer sort called the inner sort
(in a different package).  This was bug #36430 (a duplicate of #7063).

This was fixed in commit 9850bf21fc4e, which did not explain anything
in its commit message.  A preliminary version of the patch was submit-
ted for review in <20051026173901.GA3105@rpc142.cs.man.ac.uk>, which
mentions that the patch fixes bug #36430.

It fixed the bug by localising the value of PL_sortstash, causing the
if() condition to become redundant, so that was removed.

Now, it happened that that if() condition was the only place that
PL_sortstash was used.  So if we are going to localise PL_firstgv
and PL_secondgv unconditionally when sorting, PL_sortstash serves
no purpose.

While I could restore the if() condition and leave the localisation of
PL_sortstash in place, that would only benefit code that does nested
sort calls in the same package, which is rare, and the resulting
speed-up is barely measurable.

PL_sortstash is referenced by some CPAN modules, so I am taking
the cautious route of leaving it in for now, though the core
doesn’t use it.

diff --git a/pp_sort.c b/pp_sort.c
index bf7182b..56c0aac 100644 (file)
--- a/pp_sort.c
+++ b/pp_sort.c
@@ -1648,10 +1648,8 @@ PP(pp_sort)
            if (!hasargs && !is_xsub) {
                SAVESPTR(PL_firstgv);
                SAVESPTR(PL_secondgv);
-               SAVESPTR(PL_sortstash);
                PL_firstgv = gv_fetchpvs("a", GV_ADD|GV_NOTQUAL, SVt_PV);
                PL_secondgv = gv_fetchpvs("b", GV_ADD|GV_NOTQUAL, SVt_PV);
-               PL_sortstash = stash;
                SAVESPTR(GvSV(PL_firstgv));
                SAVESPTR(GvSV(PL_secondgv));
            }

diff --git a/sv.c b/sv.c
index 8cd2cb2..bcc9a22 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -13653,7 +13653,6 @@ perl_clone_using(PerlInterpreter *proto_perl, UV flags,
     PL_errors          = sv_dup_inc(proto_perl->Ierrors, param);
 
     PL_sortcop         = (OP*)any_dup(proto_perl->Isortcop, proto_perl);
-    PL_sortstash       = hv_dup(proto_perl->Isortstash, param);
     PL_firstgv         = gv_dup(proto_perl->Ifirstgv, param);
     PL_secondgv                = gv_dup(proto_perl->Isecondgv, param);