Change 29856 by [EMAIL PROTECTED] on 2007/01/17 19:49:29

        Integrate:
        [ 26981]
        The flags manipulation in sv_setsv_flags can be more efficient.
        
        [ 26982]
        Merge the IOK and NOK clauses together in sv_setsv_flags.
        
        [ 26987]
        Squeeze more flag manipulations together in sv_setsv_flags.
        
        [ 26997]
        Assert that IVs and NVs can never be tainted.
        
        [ 27043]
        Correct my comment, so that it's actually useful. :-)
        
        [ 27107]
        Remove a duplicate flag copy line from Perl_sv_setsv_flags.
        
        [ 27110]
        I think that "merge Perl_sv_2[inpu]v" and "reduce duplication in
        sv_setsv_flags" are about as done as they can be.
        
        [ 29855]
        Replace SvRELEASE_IVX(dstr) with SvOOK_off(dstr) in sv_setsv_flags(),
        because it's not possible for dstr to be COW at this point, due to an
        earlier force_normal.

Affected files ...

... //depot/maint-5.8/perl/pod/perltodo.pod#26 integrate
... //depot/maint-5.8/perl/sv.c#284 edit

Differences ...

==== //depot/maint-5.8/perl/pod/perltodo.pod#26 (text) ====
Index: perl/pod/perltodo.pod
--- perl/pod/perltodo.pod#25~29846~     2007-01-17 03:36:40.000000000 -0800
+++ perl/pod/perltodo.pod       2007-01-17 11:49:29.000000000 -0800
@@ -373,18 +373,6 @@
 might want to determine what ops I<really> are the most commonly used. And in
 turn suggest evictions and promotions to achieve a better F<pp_hot.c>.
 
-=head2 reduce duplication in sv_setsv_flags
-
-C<Perl_sv_setsv_flags> has a comment
-C</* There's a lot of redundancy below but we're going for speed here */>
-
-Whilst this was true 10 years ago, the growing disparity between RAM and CPU
-speeds mean that the trade offs have changed. In addition, the duplicate code
-adds to the maintenance burden. It would be good to see how much of the
-redundancy can be pruned, particular in the less common paths. (Profiling
-tools at the ready...). For example, why does the test for
-"Can't redefine active sort subroutine" need to occur in two places?
-
 
 
 
@@ -405,16 +393,6 @@
 approach would find savings in C<GV>s and C<CV>s, if not all the other
 larger-than-C<PVMG> types.
 
-=head2 merge Perl_sv_2[inpu]v
-
-There's a lot of code shared between C<Perl_sv_2iv_flags>,
-C<Perl_sv_2uv_flags>, C<Perl_sv_2nv>, and C<Perl_sv_2pv_flags>. It would be
-interesting to see if some of it can be merged into common shared static
-functions. In particular, C<Perl_sv_2uv_flags> started out as a cut&paste
-from C<Perl_sv_2iv_flags> around 5.005_50 time, and it may be possible to
-replace both with a single function that returns a value or union which is
-split out by the macros in F<sv.h>
-
 =head2 Implicit Latin 1 => Unicode translation
 
 Conversions from byte strings to UTF-8 currently map high bit characters

==== //depot/maint-5.8/perl/sv.c#284 (text) ====
Index: perl/sv.c
--- perl/sv.c#283~29851~        2007-01-17 07:41:23.000000000 -0800
+++ perl/sv.c   2007-01-17 11:49:29.000000000 -0800
@@ -3090,8 +3090,10 @@
            SvIV_set(dstr,  SvIVX(sstr));
            if (SvIsUV(sstr))
                SvIsUV_on(dstr);
-           if (SvTAINTED(sstr))
-               SvTAINT(dstr);
+           /* SvTAINTED can only be true if the SV has taint magic, which in
+              turn means that the SV type is PVMG (or greater). This is the
+              case statement for SVt_IV, so this cannot be true (whatever gcov
+              may say).  */
            return;
        }
        goto undef_sstr;
@@ -3111,8 +3113,10 @@
            }
            SvNV_set(dstr, SvNVX(sstr));
            (void)SvNOK_only(dstr);
-           if (SvTAINTED(sstr))
-               SvTAINT(dstr);
+           /* SvTAINTED can only be true if the SV has taint magic, which in
+              turn means that the SV type is PVMG (or greater). This is the
+              case statement for SVt_NV, so this cannot be true (whatever gcov
+              may say).  */
            return;
        }
        goto undef_sstr;
@@ -3203,25 +3207,18 @@
        }
        (void)SvOK_off(dstr);
        SvRV_set(dstr, SvREFCNT_inc(SvRV(sstr)));
-       SvROK_on(dstr);
+       SvFLAGS(dstr) |= sflags & (SVf_IOK|SVp_IOK|SVf_NOK|SVp_NOK|SVf_ROK
+                                  |SVf_AMAGIC);
        if (sflags & SVp_NOK) {
-           SvNOKp_on(dstr);
-           /* Only set the public OK flag if the source has public OK.  */
-           if (sflags & SVf_NOK)
-               SvFLAGS(dstr) |= SVf_NOK;
            SvNV_set(dstr, SvNVX(sstr));
        }
        if (sflags & SVp_IOK) {
-           (void)SvIOKp_on(dstr);
-           if (sflags & SVf_IOK)
-               SvFLAGS(dstr) |= SVf_IOK;
+           /* Must do this otherwise some other overloaded use of 0x80000000
+              gets confused. Probably SVprv_WEAKREF */
            if (sflags & SVf_IVisUV)
                SvIsUV_on(dstr);
            SvIV_set(dstr, SvIVX(sstr));
        }
-       if (SvAMAGIC(sstr)) {
-           SvAMAGIC_on(dstr);
-       }
     }
     else if (sflags & SVp_POK) {
 
@@ -3263,22 +3260,18 @@
            *SvEND(dstr) = '\0';
            (void)SvPOK_only(dstr);
        }
-       if (sflags & SVf_UTF8)
-           SvUTF8_on(dstr);
        if (sflags & SVp_NOK) {
-           SvNOKp_on(dstr);
-           if (sflags & SVf_NOK)
-               SvFLAGS(dstr) |= SVf_NOK;
            SvNV_set(dstr, SvNVX(sstr));
        }
        if (sflags & SVp_IOK) {
-           (void)SvIOKp_on(dstr);
-           if (sflags & SVf_IOK)
-               SvFLAGS(dstr) |= SVf_IOK;
+           SvOOK_off(dstr);
+           SvIV_set(dstr, SvIVX(sstr));
+           /* Must do this otherwise some other overloaded use of 0x80000000
+              gets confused. I guess SVpbm_VALID */
            if (sflags & SVf_IVisUV)
                SvIsUV_on(dstr);
-           SvIV_set(dstr, SvIVX(sstr));
        }
+       SvFLAGS(dstr) |= sflags & (SVf_IOK|SVp_IOK|SVf_NOK|SVp_NOK|SVf_UTF8);
        if ( SvVOK(sstr) ) {
            const MAGIC * const smg = mg_find(sstr,PERL_MAGIC_vstring);
            sv_magic(dstr, NULL, PERL_MAGIC_vstring,
@@ -3286,34 +3279,17 @@
            SvRMAGICAL_on(dstr);
        } 
     }
-    else if (sflags & SVp_IOK) {
-       if (sflags & SVf_IOK)
-           (void)SvIOK_only(dstr);
-       else {
-           (void)SvOK_off(dstr);
-           (void)SvIOKp_on(dstr);
+    else if (sflags & (SVp_IOK|SVp_NOK)) {
+       (void)SvOK_off(dstr);
+       SvFLAGS(dstr) |= sflags & (SVf_IOK|SVp_IOK|SVf_IVisUV|SVf_NOK|SVp_NOK);
+       if (sflags & SVp_IOK) {
+           /* XXXX Do we want to set IsUV for IV(ROK)?  Be extra safe... */
+           SvIV_set(dstr, SvIVX(sstr));
        }
-       /* XXXX Do we want to set IsUV for IV(ROK)?  Be extra safe... */
-       if (sflags & SVf_IVisUV)
-           SvIsUV_on(dstr);
-       SvIV_set(dstr, SvIVX(sstr));
        if (sflags & SVp_NOK) {
-           if (sflags & SVf_NOK)
-               (void)SvNOK_on(dstr);
-           else
-               (void)SvNOKp_on(dstr);
            SvNV_set(dstr, SvNVX(sstr));
        }
     }
-    else if (sflags & SVp_NOK) {
-       if (sflags & SVf_NOK)
-           (void)SvNOK_only(dstr);
-       else {
-           (void)SvOK_off(dstr);
-           SvNOKp_on(dstr);
-       }
-       SvNV_set(dstr, SvNVX(sstr));
-    }
     else {
        if (dtype == SVt_PVGV) {
            if (ckWARN(WARN_MISC))
End of Patch.

Reply via email to