From: Nicholas Clark <nick@ccl4.org>
Date: Sat, 25 Feb 2006 00:39:30 +0000 (+0000)
Subject: Store GvGP in the SV head union. For all the common lookups [eg GvCV()]
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=f7877b281b4;p=p5sagit%2Fp5-mst-13.2.git

Store GvGP in the SV head union. For all the common lookups [eg GvCV()]
this avoids 1 pointer dereference and the associated risk of a CPU
cache miss. Although this patch looks deceptively small, I fear its
CBV(*) might be rather high.
(* Crack By Volume)

p4raw-id: //depot/perl@27323
---

diff --git a/dump.c b/dump.c
index b63ad5c..2c6aa23 100644
--- a/dump.c
+++ b/dump.c
@@ -1322,7 +1322,7 @@ Perl_do_sv_dump(pTHX_ I32 level, PerlIO *file, SV *sv, I32 nest, I32 maxnest, bo
 	SvREFCNT_dec(d);
 	return;
     }
-    if (type <= SVt_PVLV) {
+    if (type <= SVt_PVLV && !isGV_with_GP(sv)) {
 	if (SvPVX_const(sv)) {
 	    Perl_dump_indent(aTHX_ level, file,"  PV = 0x%"UVxf" ", PTR2UV(SvPVX_const(sv)));
 	    if (SvOOK(sv))
@@ -1550,6 +1550,8 @@ Perl_do_sv_dump(pTHX_ I32 level, PerlIO *file, SV *sv, I32 nest, I32 maxnest, bo
 	Perl_dump_indent(aTHX_ level, file, "  NAME = \"%s\"\n", GvNAME(sv));
 	Perl_dump_indent(aTHX_ level, file, "  NAMELEN = %"IVdf"\n", (IV)GvNAMELEN(sv));
 	do_hv_dump (level, file, "  GvSTASH", GvSTASH(sv));
+	if (!isGV_with_GP(sv))
+	    break;
 	Perl_dump_indent(aTHX_ level, file, "  GP = 0x%"UVxf"\n", PTR2UV(GvGP(sv)));
 	if (!GvGP(sv))
 	    break;
diff --git a/ext/Devel/Peek/t/Peek.t b/ext/Devel/Peek/t/Peek.t
index f21ca6c..fdf57a9 100644
--- a/ext/Devel/Peek/t/Peek.t
+++ b/ext/Devel/Peek/t/Peek.t
@@ -308,7 +308,6 @@ do_test(17,
   FLAGS = \\(SCREAM,MULTI(?:,IN_PAD)?\\)
   IV = 0
   NV = 0
-  PV = 0
   NAME = "a"
   NAMELEN = 1
   GvSTASH = $ADDR\\t"main"
diff --git a/gv.c b/gv.c
index 0ec33ef..be5aeb7 100644
--- a/gv.c
+++ b/gv.c
@@ -192,6 +192,7 @@ Perl_gv_init(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len, int multi)
 	    Safefree(SvPVX_mutable(gv));
     }
     Newxz(gp, 1, GP);
+    SvSCREAM_on(gv);
     GvGP(gv) = gp_ref(gp);
 #ifdef PERL_DONT_CREATE_GVSV
     GvSV(gv) = NULL;
@@ -204,7 +205,6 @@ Perl_gv_init(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len, int multi)
     GvFILE(gv) = CopFILE(PL_curcop) ? CopFILE(PL_curcop) : (char *) "";
     GvCVGEN(gv) = 0;
     GvEGV(gv) = gv;
-    SvSCREAM_on(gv);
     GvSTASH(gv) = stash;
     if (stash)
 	Perl_sv_add_backref(aTHX_ (SV*)stash, (SV*)gv);
@@ -1374,7 +1374,7 @@ Perl_gp_free(pTHX_ GV *gv)
     dVAR;
     GP* gp;
 
-    if (!gv || !(gp = GvGP(gv)))
+    if (!gv || !isGV_with_GP(gv) || !(gp = GvGP(gv)))
 	return;
     if (gp->gp_refcnt == 0) {
 	if (ckWARN_d(WARN_INTERNAL))
diff --git a/gv.h b/gv.h
index da9da4a..1e4797a 100644
--- a/gv.h
+++ b/gv.h
@@ -31,9 +31,10 @@ struct gp {
     ((!defined(_MSC_VER) || _MSC_VER > 1200) && !defined(__BORLANDC__))
 #  define GvGP(gv)	(*(assert(SvTYPE(gv) == SVt_PVGV || \
 				  SvTYPE(gv) == SVt_PVLV), \
-			   &(GvXPVGV(gv)->xgv_gp)))
+			assert(isGV_with_GP(gv)),	\
+			   &((gv)->sv_u.svu_gp)))
 #else
-#  define GvGP(gv)	(GvXPVGV(gv)->xgv_gp)
+#  define GvGP(gv)	((gv)->sv_u.svu_gp)
 #endif
 
 #define GvNAME(gv)	(GvXPVGV(gv)->xgv_name)
diff --git a/pp.c b/pp.c
index d63f039..1724ab0 100644
--- a/pp.c
+++ b/pp.c
@@ -3016,7 +3016,9 @@ PP(pp_substr)
 			Perl_warner(aTHX_ packWARN(WARN_SUBSTR),
 				"Attempt to use reference as lvalue in substr");
 		}
-		if (SvOK(sv))		/* is it defined ? */
+		if (isGV_with_GP(sv))
+		    SvPV_force_nolen(sv);
+		else if (SvOK(sv))	/* is it defined ? */
 		    (void)SvPOK_only_UTF8(sv);
 		else
 		    sv_setpvn(sv,"",0);	/* avoid lexical reincarnation */
diff --git a/pp_hot.c b/pp_hot.c
index 8d590ca..d26d8f0 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -1611,6 +1611,9 @@ Perl_do_readline(pTHX)
 	sv = TARG;
 	if (SvROK(sv))
 	    sv_unref(sv);
+	else if (isGV_with_GP(sv)) {
+	    SvPV_force_nolen(sv);
+	}
 	SvUPGRADE(sv, SVt_PV);
 	tmplen = SvLEN(sv);	/* remember if already alloced */
 	if (!tmplen && !SvREADONLY(sv))
diff --git a/sv.c b/sv.c
index 462dc9e..6d550d9 100644
--- a/sv.c
+++ b/sv.c
@@ -472,7 +472,7 @@ static void
 do_clean_named_objs(pTHX_ SV *sv)
 {
     dVAR;
-    if (SvTYPE(sv) == SVt_PVGV && GvGP(sv)) {
+    if (SvTYPE(sv) == SVt_PVGV && isGV_with_GP(sv) && GvGP(sv)) {
 	if ((
 #ifdef PERL_DONT_CREATE_GVSV
 	     GvSV(sv) &&
@@ -2117,12 +2117,9 @@ S_sv_2iuv_common(pTHX_ SV *sv) {
 	}
     }
     else  {
-	if (((SvFLAGS(sv) & (SVp_POK|SVp_SCREAM)) == SVp_SCREAM)
-	    && (SvTYPE(sv) == SVt_PVGV || SvTYPE(sv) == SVt_PVLV)) {
+	if (isGV_with_GP(sv)) {
 	    return PTR2IV(glob_2inpuv((GV *)sv, NULL, TRUE));
 	}
-	if (SvTYPE(sv) == SVt_PVGV)
-	    sv_dump(sv);
 
 	if (!(SvFLAGS(sv) & SVs_PADTMP)) {
 	    if (!PL_localizing && ckWARN(WARN_UNINITIALIZED))
@@ -2471,8 +2468,7 @@ Perl_sv_2nv(pTHX_ register SV *sv)
 #endif /* NV_PRESERVES_UV */
     }
     else  {
-	if (((SvFLAGS(sv) & (SVp_POK|SVp_SCREAM)) == SVp_SCREAM)
-	    && (SvTYPE(sv) == SVt_PVGV || SvTYPE(sv) == SVt_PVLV)) {
+	if (isGV_with_GP(sv)) {
 	    glob_2inpuv((GV *)sv, NULL, TRUE);
 	    return 0.0;
 	}
@@ -2809,8 +2805,7 @@ Perl_sv_2pv_flags(pTHX_ register SV *sv, STRLEN *lp, I32 flags)
 #endif
     }
     else {
-	if (((SvFLAGS(sv) & (SVp_POK|SVp_SCREAM)) == SVp_SCREAM)
-	    && (SvTYPE(sv) == SVt_PVGV || SvTYPE(sv) == SVt_PVLV)) {
+	if (isGV_with_GP(sv)) {
 	    return glob_2inpuv((GV *)sv, lp, FALSE);
 	}
 
@@ -2945,8 +2940,7 @@ Perl_sv_2bool(pTHX_ register SV *sv)
 	    if (SvNOKp(sv))
 		return SvNVX(sv) != 0.0;
 	    else {
-		if ((SvFLAGS(sv) & SVp_SCREAM)
-		    && (SvTYPE(sv) == (SVt_PVGV) || SvTYPE(sv) == (SVt_PVLV)))
+		if (isGV_with_GP(sv))
 		    return TRUE;
 		else
 		    return FALSE;
@@ -3189,8 +3183,15 @@ S_glob_assign_glob(pTHX_ SV *dstr, SV *sstr, const int dtype)
 	const char * const name = GvNAME(sstr);
 	const STRLEN len = GvNAMELEN(sstr);
 	/* don't upgrade SVt_PVLV: it can hold a glob */
-	if (dtype != SVt_PVLV)
+	if (dtype != SVt_PVLV) {
+	    if (dtype >= SVt_PV) {
+		SvPV_free(dstr);
+		SvPV_set(dstr, 0);
+		SvLEN_set(dstr, 0);
+		SvCUR_set(dstr, 0);
+	    }
 	    sv_upgrade(dstr, SVt_PVGV);
+	}
 	GvSTASH(dstr) = GvSTASH(sstr);
 	if (GvSTASH(dstr))
 	    Perl_sv_add_backref(aTHX_ (SV*)GvSTASH(dstr), dstr);
@@ -3205,10 +3206,11 @@ S_glob_assign_glob(pTHX_ SV *dstr, SV *sstr, const int dtype)
     }
 #endif
 
+    gp_free((GV*)dstr);
+    SvSCREAM_off(dstr);
     (void)SvOK_off(dstr);
-    SvSCREAM_on(dstr);
     GvINTRO_off(dstr);		/* one-shot flag */
-    gp_free((GV*)dstr);
+    SvSCREAM_on(dstr);
     GvGP(dstr) = gp_ref(GvGP(sstr));
     if (SvTAINTED(sstr))
 	SvTAINT(dstr);
@@ -3685,8 +3687,7 @@ Perl_sv_setsv_flags(pTHX_ SV *dstr, register SV *sstr, I32 flags)
 	}
     }
     else {
-	if ((stype == SVt_PVGV || stype == SVt_PVLV)
-		 && (sflags & SVp_SCREAM)) {
+	if (isGV_with_GP(sstr)) {
 	    /* This stringification rule for globs is spread in 3 places.
 	       This feels bad. FIXME.  */
 	    const U32 wasfake = sflags & SVf_FAKE;
@@ -7692,8 +7693,9 @@ S_sv_unglob(pTHX_ SV *sv)
     SvFAKE_off(sv);
     gv_efullname3(temp, (GV *) sv, "*");
 
-    if (GvGP(sv))
+    if (GvGP(sv)) {
 	gp_free((GV*)sv);
+    }
     if (GvSTASH(sv)) {
 	sv_del_backref((SV*)GvSTASH(sv), sv);
 	GvSTASH(sv) = NULL;
@@ -9630,7 +9632,10 @@ Perl_rvpv_dup(pTHX_ SV *dstr, const SV *sstr, CLONE_PARAMS* param)
 	}
 	else {
 	    /* Special case - not normally malloced for some reason */
-	    if ((SvREADONLY(sstr) && SvFAKE(sstr))) {
+	    if (isGV_with_GP(sstr)) {
+		/* Don't need to do anything here.  */
+	    }
+	    else if ((SvREADONLY(sstr) && SvFAKE(sstr))) {
 		/* A "shared" PV - clone it as "shared" PV */
 		SvPV_set(dstr,
 			 HEK_KEY(hek_dup(SvSHARED_HEK_FROM_PV(SvPVX_const(sstr)),
@@ -9774,7 +9779,8 @@ Perl_sv_dup(pTHX_ const SV *sstr, CLONE_PARAMS* param)
 		 sv_type_details->body_size + sv_type_details->offset, char);
 #endif
 
-	    if (sv_type != SVt_PVAV && sv_type != SVt_PVHV)
+	    if (sv_type != SVt_PVAV && sv_type != SVt_PVHV
+		&& !isGV_with_GP(dstr))
 		Perl_rvpv_dup(aTHX_ dstr, sstr, param);
 
 	    /* The Copy above means that all the source (unduplicated) pointers
@@ -9815,11 +9821,16 @@ Perl_sv_dup(pTHX_ const SV *sstr, CLONE_PARAMS* param)
 		break;
 	    case SVt_PVGV:
 		GvNAME(dstr)	= SAVEPVN(GvNAME(dstr), GvNAMELEN(dstr));
-		GvSTASH(dstr)	= hv_dup(GvSTASH(dstr), param);
 		/* Don't call sv_add_backref here as it's going to be created
 		   as part of the magic cloning of the symbol table.  */
-		GvGP(dstr)	= gp_dup(GvGP(dstr), param);
-		(void)GpREFCNT_inc(GvGP(dstr));
+		GvSTASH(dstr)	= hv_dup(GvSTASH(dstr), param);
+		if(isGV_with_GP(sstr)) {
+		    /* Danger Will Robinson - GvGP(dstr) isn't initialised
+		       at the point of this comment.  */
+		    GvGP(dstr)	= gp_dup(GvGP(sstr), param);
+		    (void)GpREFCNT_inc(GvGP(dstr));
+		} else
+		    Perl_rvpv_dup(aTHX_ dstr, sstr, param);
 		break;
 	    case SVt_PVIO:
 		IoIFP(dstr)	= fp_dup(IoIFP(dstr), IoTYPE(dstr), param);
diff --git a/sv.h b/sv.h
index 102c790..c509ac6 100644
--- a/sv.h
+++ b/sv.h
@@ -97,6 +97,7 @@ typedef struct hek HEK;
 	char*   svu_pv;		/* pointer to malloced string */	\
 	SV**    svu_array;		\
 	HE**	svu_hash;		\
+	GP*	svu_gp;			\
     }	sv_u
 
 
@@ -403,7 +404,6 @@ struct xpvlv {
     HV*		xmg_stash;	/* class package */
 
     /* a full glob fits into this */
-    GP*		xgv_gp;
     char*	xgv_name;
     STRLEN	xgv_namelen;
     HV*		xgv_stash;
@@ -432,7 +432,6 @@ struct xpvgv {
     } xmg_u;
     HV*		xmg_stash;	/* class package */
 
-    GP*		xgv_gp;
     char*	xgv_name;
     STRLEN	xgv_namelen;
     HV*		xgv_stash;
@@ -763,12 +762,14 @@ Set the actual length of the string which is in the SV.  See C<SvIV_set>.
 
 #if defined(__GNUC__) && !defined(PERL_GCC_BRACE_GROUPS_FORBIDDEN)
 #define assert_not_ROK(sv)	({assert(!SvROK(sv) || !SvRV(sv));}),
+#define assert_not_glob(sv)	({assert(!isGV_with_GP(sv));}),
 #else
 #define assert_not_ROK(sv)	
+#define assert_not_glob(sv)	
 #endif
 
 #define SvOK(sv)		(SvFLAGS(sv) & SVf_OK)
-#define SvOK_off(sv)		(assert_not_ROK(sv)			\
+#define SvOK_off(sv)		(assert_not_ROK(sv) assert_not_glob(sv)	\
 				 SvFLAGS(sv) &=	~(SVf_OK|SVf_AMAGIC|	\
 						  SVf_IVisUV|SVf_UTF8),	\
 							SvOOK_off(sv))
@@ -779,21 +780,21 @@ Set the actual length of the string which is in the SV.  See C<SvIV_set>.
 
 #define SvOKp(sv)		(SvFLAGS(sv) & (SVp_IOK|SVp_NOK|SVp_POK))
 #define SvIOKp(sv)		(SvFLAGS(sv) & SVp_IOK)
-#define SvIOKp_on(sv)		(SvRELEASE_IVX(sv), \
+#define SvIOKp_on(sv)		(assert_not_glob(sv) SvRELEASE_IVX(sv), \
 				    SvFLAGS(sv) |= SVp_IOK)
 #define SvNOKp(sv)		(SvFLAGS(sv) & SVp_NOK)
-#define SvNOKp_on(sv)		(SvFLAGS(sv) |= SVp_NOK)
+#define SvNOKp_on(sv)		(assert_not_glob(sv) SvFLAGS(sv) |= SVp_NOK)
 #define SvPOKp(sv)		(SvFLAGS(sv) & SVp_POK)
-#define SvPOKp_on(sv)		(assert_not_ROK(sv)			\
+#define SvPOKp_on(sv)		(assert_not_ROK(sv) assert_not_glob(sv)	\
 				 SvFLAGS(sv) |= SVp_POK)
 
 #define SvIOK(sv)		(SvFLAGS(sv) & SVf_IOK)
-#define SvIOK_on(sv)		(SvRELEASE_IVX(sv), \
+#define SvIOK_on(sv)		(assert_not_glob(sv) SvRELEASE_IVX(sv), \
 				    SvFLAGS(sv) |= (SVf_IOK|SVp_IOK))
 #define SvIOK_off(sv)		(SvFLAGS(sv) &= ~(SVf_IOK|SVp_IOK|SVf_IVisUV))
 #define SvIOK_only(sv)		(SvOK_off(sv), \
 				    SvFLAGS(sv) |= (SVf_IOK|SVp_IOK))
-#define SvIOK_only_UV(sv)	(SvOK_off_exc_UV(sv), \
+#define SvIOK_only_UV(sv)	(assert_not_glob(sv) SvOK_off_exc_UV(sv), \
 				    SvFLAGS(sv) |= (SVf_IOK|SVp_IOK))
 
 #define SvIOK_UV(sv)		((SvFLAGS(sv) & (SVf_IOK|SVf_IVisUV))	\
@@ -807,7 +808,8 @@ Set the actual length of the string which is in the SV.  See C<SvIV_set>.
 #define SvIsUV_off(sv)		(SvFLAGS(sv) &= ~SVf_IVisUV)
 
 #define SvNOK(sv)		(SvFLAGS(sv) & SVf_NOK)
-#define SvNOK_on(sv)		(SvFLAGS(sv) |= (SVf_NOK|SVp_NOK))
+#define SvNOK_on(sv)		(assert_not_glob(sv) \
+				 SvFLAGS(sv) |= (SVf_NOK|SVp_NOK))
 #define SvNOK_off(sv)		(SvFLAGS(sv) &= ~(SVf_NOK|SVp_NOK))
 #define SvNOK_only(sv)		(SvOK_off(sv), \
 				    SvFLAGS(sv) |= (SVf_NOK|SVp_NOK))
@@ -837,14 +839,14 @@ in gv.h: */
 #define SvUTF8_off(sv)		(SvFLAGS(sv) &= ~(SVf_UTF8))
 
 #define SvPOK(sv)		(SvFLAGS(sv) & SVf_POK)
-#define SvPOK_on(sv)		(assert_not_ROK(sv)			\
+#define SvPOK_on(sv)		(assert_not_ROK(sv) assert_not_glob(sv)	\
 				 SvFLAGS(sv) |= (SVf_POK|SVp_POK))
 #define SvPOK_off(sv)		(SvFLAGS(sv) &= ~(SVf_POK|SVp_POK))
-#define SvPOK_only(sv)		(assert_not_ROK(sv)			\
+#define SvPOK_only(sv)		(assert_not_ROK(sv) assert_not_glob(sv)	\
 				 SvFLAGS(sv) &= ~(SVf_OK|SVf_AMAGIC|	\
 						  SVf_IVisUV|SVf_UTF8),	\
 				    SvFLAGS(sv) |= (SVf_POK|SVp_POK))
-#define SvPOK_only_UTF8(sv)	(assert_not_ROK(sv)			\
+#define SvPOK_only_UTF8(sv)	(assert_not_ROK(sv) assert_not_glob(sv)	\
 				 SvFLAGS(sv) &= ~(SVf_OK|SVf_AMAGIC|	\
 						  SVf_IVisUV),		\
 				    SvFLAGS(sv) |= (SVf_POK|SVp_POK))
@@ -997,13 +999,20 @@ in gv.h: */
 #    define SvSTASH(sv)	(0 + ((XPVMG*)  SvANY(sv))->xmg_stash)
 #  endif
 #else
-#  define SvPVX(sv) ((sv)->sv_u.svu_pv)
 #  define SvCUR(sv) ((XPV*) SvANY(sv))->xpv_cur
 #  define SvLEN(sv) ((XPV*) SvANY(sv))->xpv_len
 #  define SvEND(sv) ((sv)->sv_u.svu_pv + ((XPV*)SvANY(sv))->xpv_cur)
 
 #  if defined (DEBUGGING) && defined(__GNUC__) && !defined(PERL_GCC_BRACE_GROUPS_FORBIDDEN)
 /* These get expanded inside other macros that already use a variable _sv  */
+#    define SvPVX(sv)							\
+	(*({ SV *const _svi = (SV *) sv;				\
+	    assert(SvTYPE(_svi) >= SVt_PV);				\
+	    assert(SvTYPE(_svi) != SVt_PVAV);				\
+	    assert(SvTYPE(_svi) != SVt_PVHV);				\
+	    assert(!isGV_with_GP(_svi));				\
+	    &((_svi)->sv_u.svu_pv);					\
+	 }))
 #    define SvIVX(sv)							\
 	(*({ SV *const _svi = (SV *) sv;				\
 	    assert(SvTYPE(_svi) == SVt_IV || SvTYPE(_svi) >= SVt_PVIV);	\
@@ -1039,6 +1048,7 @@ in gv.h: */
 	    &(((XPVMG*) SvANY(_svi))->xmg_stash);			\
 	  }))
 #  else
+#   define SvPVX(sv) ((sv)->sv_u.svu_pv)
 #    define SvIVX(sv) ((XPVIV*) SvANY(sv))->xiv_iv
 #    define SvUVX(sv) ((XPVUV*) SvANY(sv))->xuv_uv
 #    define SvNVX(sv) ((XPVNV*) SvANY(sv))->xnv_nv
@@ -1100,6 +1110,7 @@ in gv.h: */
 		(((XPV*)  SvANY(sv))->xpv_cur = (val)); } STMT_END
 #define SvLEN_set(sv, val) \
 	STMT_START { assert(SvTYPE(sv) >= SVt_PV); \
+		assert(!isGV_with_GP(sv));	\
 		(((XPV*)  SvANY(sv))->xpv_len = (val)); } STMT_END
 #define SvEND_set(sv, val) \
 	STMT_START { assert(SvTYPE(sv) >= SVt_PV); \
@@ -1681,6 +1692,11 @@ Returns a pointer to the character buffer.
 #define boolSV(b) ((b) ? &PL_sv_yes : &PL_sv_no)
 
 #define isGV(sv) (SvTYPE(sv) == SVt_PVGV)
+/* If I give every macro argument a different name, then there won't be bugs
+   where nested macros get confused. Been there, done that.  */
+#define isGV_with_GP(pwadak) \
+	(((SvFLAGS(pwadak) & (SVp_POK|SVp_SCREAM)) == SVp_SCREAM)	\
+	&& (SvTYPE(pwadak) == SVt_PVGV || SvTYPE(pwadak) == SVt_PVLV))
 
 #define SvGROW(sv,len) (SvLEN(sv) < (len) ? sv_grow(sv,len) : SvPVX(sv))
 #define SvGROW_mutable(sv,len) \