Skip to content

Commit 36f61f9

Browse files
committed
WIP add API for refcounting CopFILE names with threads #8
This has large memory savings, test prog, perl -MTest::More -e"system 'pause'" before 2196KB Private Bytes Win 7 32 bit to after 2092KB. -On a CHEK the refcount is a U32 for memory savings on 64 bit CPUs while SHEKs are Size_t for refcount because of HE struct, on 32 bit Size_t and U32 happen to be the same thing, if there is future integration the refcount members will have to be the same type, then duping a SHEK or a CHEK is the same code, except that HVhek_COMPILING controls whether to aquire OP_REFCNT_LOCK before touching the ref count, in the future with atomic operations, the refcount can be manipulated with atomic operations regardless if it is a SHEK or CHEK since OP_REFCNT_LOCK lines were removed -TODO figure out how to do static const CHEKs, hash member must be 0 since its process specific randomized (rurban's B stores HEKs in RW static memory and fixes up the hash #s at runtime), add test and branch so that refcount isn't read and written or passed to PerlMemShared_free if static flag is on inidicating static const CHEK -TODO Perl_newGP uses CHEKs not CopFILE, no memcpy and add _< that way -TODO optimize the former alloca to smallbuf or Safefree or savestack newx free
1 parent 65dd9e5 commit 36f61f9

File tree

16 files changed

+284
-45
lines changed

16 files changed

+284
-45
lines changed

cop.h

+17-17
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ struct cop {
383383
#ifdef USE_ITHREADS
384384
PADOFFSET cop_stashoff; /* offset into PL_stashpad, for the
385385
package the line was compiled in */
386-
char * cop_file; /* file name the following line # is from */
386+
char * cop_file; /* a CHEK allocated file name, part of line # */
387387
#else
388388
HV * cop_stash; /* package line was compiled in */
389389
GV * cop_filegv; /* file the following line # is from */
@@ -398,34 +398,32 @@ struct cop {
398398
};
399399

400400
#ifdef USE_ITHREADS
401+
/* make this unassignable with a "+0" force ppl to use _set(), but what about setting
402+
the ptr directly in the CHEK code? what suffix to use on the _setptr() variant
403+
"setptr" isn't perl XS API nomenclature
404+
the fact you can assign a Newx ptr to CopFILE is very dangerous and will
405+
cause mem corruption, and it did in Perl_gv_check */
401406
# define CopFILE(c) ((c)->cop_file)
407+
# define CopFILE_len(c) (HEK_LEN(FNPV2HEK(CopFILE(c)))-2)
402408
# define CopFILEGV(c) (CopFILE(c) \
403-
? gv_fetchfile(CopFILE(c)) : NULL)
409+
? Perl_gv_fetchfile_hek(aTHX_ FNPV2HEK(CopFILE(c))) : NULL)
404410

405-
# ifdef NETWARE
406-
# define CopFILE_set(c,pv) ((c)->cop_file = savepv(pv))
407-
# define CopFILE_setn(c,pv,l) ((c)->cop_file = savepvn((pv),(l)))
408-
# else
409-
# define CopFILE_set(c,pv) ((c)->cop_file = savesharedpv(pv))
410-
# define CopFILE_setn(c,pv,l) ((c)->cop_file = savesharedpvn((pv),(l)))
411-
# endif
411+
#define CopFILE_set(c,pv) ((c)->cop_file = newchek((pv),0))
412+
#define CopFILE_setn(c,pv,l) ((c)->cop_file = newchek((pv),(l)))
412413

413414
# define CopFILESV(c) (CopFILE(c) \
414-
? GvSV(gv_fetchfile(CopFILE(c))) : NULL)
415+
? GvSV(Perl_gv_fetchfile_hek(aTHX_ FNPV2HEK(CopFILE(c)))) : NULL)
415416
# define CopFILEAV(c) (CopFILE(c) \
416-
? GvAV(gv_fetchfile(CopFILE(c))) : NULL)
417+
? GvAV(Perl_gv_fetchfile_hek(aTHX_ FNPV2HEK(CopFILE(c)))) : NULL)
417418
# define CopFILEAVx(c) (assert_(CopFILE(c)) \
418-
GvAV(gv_fetchfile(CopFILE(c))))
419+
GvAV(Perl_gv_fetchfile_hek(aTHX_ FNPV2HEK(CopFILE(c)))))
419420

420421
# define CopSTASH(c) PL_stashpad[(c)->cop_stashoff]
421422
# define CopSTASH_set(c,hv) ((c)->cop_stashoff = (hv) \
422423
? alloccopstash(hv) \
423424
: 0)
424-
# ifdef NETWARE
425-
# define CopFILE_free(c) SAVECOPFILE_FREE(c)
426-
# else
427-
# define CopFILE_free(c) (PerlMemShared_free(CopFILE(c)),(CopFILE(c) = NULL))
428-
# endif
425+
# define CopFILE_free(c) free_copfile(c)
426+
429427
#else
430428
# define CopFILEGV(c) ((c)->cop_filegv)
431429
# define CopFILEGV_set(c,gv) ((c)->cop_filegv = (GV*)SvREFCNT_inc(gv))
@@ -440,6 +438,8 @@ struct cop {
440438
# endif
441439
# define CopFILE(c) (CopFILEGV(c) \
442440
? GvNAME(CopFILEGV(c))+2 : NULL)
441+
# define CopFILE_len(c) (CopFILEGV(c) \
442+
? GvNAMELEN(CopFILEGV(c))-2 : 0)
443443
# define CopSTASH(c) ((c)->cop_stash)
444444
# define CopSTASH_set(c,hv) ((c)->cop_stash = (hv))
445445
# define CopFILE_free(c) (SvREFCNT_dec(CopFILEGV(c)),(CopFILEGV(c) = NULL))

cv.h

+3-7
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,9 @@ See L<perlguts/Autoloading with XSUBs>.
5353
#define CvGV_set(cv,gv) Perl_cvgv_set(aTHX_ cv, gv)
5454
#define CvHASGV(cv) cBOOL(SvANY(cv)->xcv_gv_u.xcv_gv)
5555
#define CvFILE(sv) ((XPVCV*)MUTABLE_PTR(SvANY(sv)))->xcv_file
56-
#ifdef USE_ITHREADS
57-
# define CvFILE_set_from_cop(sv, cop) \
58-
(CvFILE(sv) = savepv(CopFILE(cop)), CvDYNFILE_on(sv))
59-
#else
60-
# define CvFILE_set_from_cop(sv, cop) \
61-
(CvFILE(sv) = CopFILE(cop), CvDYNFILE_off(sv))
62-
#endif
56+
/* remove assert once stable */
57+
#define CvFILE_set_from_cop(sv, cop) \
58+
(assert_(!CvDYNFILE(cv)) CvFILE(sv) = CopFILE(cop), CvDYNFILE_off(sv))
6359
#define CvFILEGV(sv) (gv_fetchfile(CvFILE(sv)))
6460
#define CvDEPTH(sv) (*S_CvDEPTHp((const CV *)sv))
6561
/* For use when you only have a XPVCV*, not a real CV*.

embed.fnc

+10
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,9 @@ Ap |GV* |gv_fetchfile |NN const char* name
508508
Am |GV* |gv_fetchfile_flags|NN const char *const name|const STRLEN len\
509509
|const U32 flags
510510
pX |GV* |gv_fetchfile_x|NN const char *const name|const STRLEN len
511+
#ifdef USE_ITHREADS
512+
pX |GV* |gv_fetchfile_hek|NN const HEK * const hek
513+
#endif
511514
Amd |GV* |gv_fetchmeth |NULLOK HV* stash|NN const char* name \
512515
|STRLEN len|I32 level
513516
Apd |GV* |gv_fetchmeth_sv |NULLOK HV* stash|NN SV* namesv|I32 level|U32 flags
@@ -2870,6 +2873,13 @@ Apon |void |sys_init3 |NN int* argc|NN char*** argv|NN char*** env
28702873
Apon |void |sys_term
28712874
ApoM |const char *|cop_fetch_label|NN COP *const cop \
28722875
|NULLOK STRLEN *len|NULLOK U32 *flags
2876+
#ifdef USE_ITHREADS
2877+
p |char * |newchek |NN const char *str |I32 len
2878+
p |void |free_copfile |NN COP * cop
2879+
p |void |chek_inc |NN CHEK * chek
2880+
p |void |chek_dec |NN CHEK * chek
2881+
p |void |save_copfile |NN COP * cop
2882+
#endif
28732883
: Only used in op.c and the perl compiler
28742884
ApoM |void|cop_store_label \
28752885
|NN COP *const cop|NN const char *label|STRLEN len|U32 flags

embed.h

+6
Original file line numberDiff line numberDiff line change
@@ -1810,10 +1810,16 @@
18101810
#define get_c_backtrace(a,b) Perl_get_c_backtrace(aTHX_ a,b)
18111811
# endif
18121812
# if defined(USE_ITHREADS)
1813+
#define chek_dec(a) Perl_chek_dec(aTHX_ a)
1814+
#define chek_inc(a) Perl_chek_inc(aTHX_ a)
1815+
#define free_copfile(a) Perl_free_copfile(aTHX_ a)
1816+
#define gv_fetchfile_hek(a) Perl_gv_fetchfile_hek(aTHX_ a)
18131817
#define mro_meta_dup(a,b) Perl_mro_meta_dup(aTHX_ a,b)
1818+
#define newchek(a,b) Perl_newchek(aTHX_ a,b)
18141819
#define padlist_dup(a,b) Perl_padlist_dup(aTHX_ a,b)
18151820
#define padname_dup(a,b) Perl_padname_dup(aTHX_ a,b)
18161821
#define padnamelist_dup(a,b) Perl_padnamelist_dup(aTHX_ a,b)
1822+
#define save_copfile(a) Perl_save_copfile(aTHX_ a)
18171823
# endif
18181824
# if defined(USE_LOCALE) && defined(PERL_IN_LOCALE_C)
18191825
#define stdize_locale(a) S_stdize_locale(aTHX_ a)

ext/Devel-Peek/t/Peek.t

+10-10
Original file line numberDiff line numberDiff line change
@@ -298,8 +298,8 @@ do_test('reference to anon sub with empty prototype',
298298
RV = $ADDR
299299
SV = PVCV\\($ADDR\\) at $ADDR
300300
REFCNT = 2
301-
FLAGS = \\($PADMY,POK,pPOK,ANON,WEAKOUTSIDE,CVGV_RC\\) # $] < 5.015 || !thr
302-
FLAGS = \\($PADMY,POK,pPOK,ANON,WEAKOUTSIDE,CVGV_RC,DYNFILE\\) # $] >= 5.015 && thr
301+
FLAGS = \\($PADMY,POK,pPOK,ANON,WEAKOUTSIDE,CVGV_RC\\) # $] < 5.015 || !thr || $] >= 5.023008
302+
FLAGS = \\($PADMY,POK,pPOK,ANON,WEAKOUTSIDE,CVGV_RC,DYNFILE\\) # $] >= 5.015 && $] < 5.023008 && thr
303303
PROTOTYPE = ""
304304
COMP_STASH = $ADDR\\t"main"
305305
START = $ADDR ===> \\d+
@@ -309,8 +309,8 @@ do_test('reference to anon sub with empty prototype',
309309
DEPTH = 0(?:
310310
MUTEXP = $ADDR
311311
OWNER = $ADDR)?
312-
FLAGS = 0x490 # $] < 5.015 || !thr
313-
FLAGS = 0x1490 # $] >= 5.015 && thr
312+
FLAGS = 0x490 # $] < 5.015 || !thr || $] >= 5.023008
313+
FLAGS = 0x1490 # $] >= 5.015 && $] < 5.023008 && thr
314314
OUTSIDE_SEQ = \\d+
315315
PADLIST = $ADDR
316316
PADNAME = $ADDR\\($ADDR\\) PAD = $ADDR\\($ADDR\\)
@@ -324,8 +324,8 @@ do_test('reference to named subroutine without prototype',
324324
RV = $ADDR
325325
SV = PVCV\\($ADDR\\) at $ADDR
326326
REFCNT = (3|4)
327-
FLAGS = \\((?:HASEVAL(?:,NAMED)?)?\\) # $] < 5.015 || !thr
328-
FLAGS = \\(DYNFILE(?:,HASEVAL(?:,NAMED)?)?\\) # $] >= 5.015 && thr
327+
FLAGS = \\((?:HASEVAL(?:,NAMED)?)?\\) # $] < 5.015 || !thr || $] >= 5.023008
328+
FLAGS = \\(DYNFILE(?:,HASEVAL(?:,NAMED)?)?\\) # $] >= 5.015 && $] < 5.023008 && thr
329329
COMP_STASH = $ADDR\\t"main"
330330
START = $ADDR ===> \\d+
331331
ROOT = $ADDR
@@ -734,8 +734,8 @@ do_test('FORMAT',
734734
RV = $ADDR
735735
SV = PVFM\\($ADDR\\) at $ADDR
736736
REFCNT = 2
737-
FLAGS = \\(\\) # $] < 5.015 || !thr
738-
FLAGS = \\(DYNFILE\\) # $] >= 5.015 && thr
737+
FLAGS = \\(\\) # $] < 5.015 || !thr || $] >= 5.023008
738+
FLAGS = \\(DYNFILE\\) # $] >= 5.015 && $] < 5.023008 && thr
739739
(?: PV = 0
740740
)? COMP_STASH = 0x0
741741
START = $ADDR ===> \\d+
@@ -745,8 +745,8 @@ do_test('FORMAT',
745745
DEPTH = 0)?(?:
746746
MUTEXP = $ADDR
747747
OWNER = $ADDR)?
748-
FLAGS = 0x0 # $] < 5.015 || !thr
749-
FLAGS = 0x1000 # $] >= 5.015 && thr
748+
FLAGS = 0x0 # $] < 5.015 || !thr || $] >= 5.023008
749+
FLAGS = 0x1000 # $] >= 5.015 && $] < 5.023008 && thr
750750
OUTSIDE_SEQ = \\d+
751751
LINES = 0 # $] < 5.017_003
752752
PADLIST = $ADDR

gv.c

+32-1
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,34 @@ Perl_gv_fetchfile_x(pTHX_ const char *const name, const STRLEN namelen)
134134
return gv;
135135
}
136136

137+
#ifdef USE_ITHREADS
138+
/* HEK must start with "_<" */
139+
GV *
140+
Perl_gv_fetchfile_hek(pTHX_ const HEK * const hek)
141+
{
142+
GV *gv;
143+
144+
PERL_ARGS_ASSERT_GV_FETCHFILE_HEK;
145+
146+
if (!PL_defstash)
147+
return NULL;
148+
assert(HEK_LEN(hek) >= 2
149+
&& HEK_KEY(hek)[0] == '_' && HEK_KEY(hek)[1] == '<');
150+
gv = *(GV**)hv_fetchhek(PL_defstash, hek, TRUE);
151+
if (!isGV(gv)) {
152+
gv_init(gv, PL_defstash, HEK_KEY(hek), HEK_LEN(hek), FALSE);
153+
#ifdef PERL_DONT_CREATE_GVSV
154+
GvSV(gv) = newSVpvn(HEK_KEY(hek)+2, HEK_LEN(hek)-2);
155+
#else
156+
sv_setpvn(GvSV(gv), HEK_KEY(hek)+2, HEK_LEN(hek)-2);
157+
#endif
158+
}
159+
if (PERLDB_LINE_OR_SAVESRC && !GvAV(gv))
160+
hv_magic(GvHVn(gv), GvAVn(gv), PERL_MAGIC_dbfile);
161+
return gv;
162+
}
163+
#endif
164+
137165
/*
138166
=for apidoc gv_const_sv
139167
@@ -2443,9 +2471,12 @@ Perl_gv_check(pTHX_ HV *stash)
24432471
if (SvTYPE(gv) != SVt_PVGV || GvMULTI(gv))
24442472
continue;
24452473
file = GvFILE(gv);
2474+
/* how is this thread safe ???????? aren't ops immutable after creation??*/
24462475
CopLINE_set(PL_curcop, GvLINE(gv));
24472476
#ifdef USE_ITHREADS
2448-
CopFILE(PL_curcop) = (char *)file; /* set for warning */
2477+
CopFILE_free(PL_curcop);
2478+
assert(CopFILE(PL_curcop) == NULL);
2479+
CopFILE_set(PL_curcop, file); /* set for warning */
24492480
#else
24502481
CopFILEGV(PL_curcop)
24512482
= gv_fetchfile_flags(file, HEK_LEN(GvFILE_HEK(gv)), 0);

gv.h

+7
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,13 @@ Return the CV from the GV.
138138
#define GvGPFLAGS(gv) (GvGP(gv)->gp_flags)
139139

140140
#define GvLINE(gv) (GvGP(gv)->gp_line)
141+
/*XXXX gp_file_hek seems to always come from curcop in gv_init(), so shouldn't
142+
*this be a CHEK instead of a SHEK ????
143+
*GvFILE and GvFILEx will be the +2 versions that DONT include _< for back compat
144+
*that way gv_fetchfile(GvFILEx(gv)) will be gv_fetchfile_hek(chek_ptr) and not
145+
*turn the no _< string into a temporary _< prefixed string to do the hash lookup
146+
147+
*XXXX*/
141148
#define GvFILE_HEK(gv) (GvGP(gv)->gp_file_hek)
142149
#define GvFILEx(gv) HEK_KEY(GvFILE_HEK(gv))
143150
#define GvFILE(gv) (GvFILE_HEK(gv) ? GvFILEx(gv) : NULL)

hv.c

+113-3
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,32 @@ S_new_he(pTHX)
7272

7373
#endif
7474

75+
#ifdef USE_ITHREADS
76+
char *
77+
Perl_newchek(pTHX_ const char *str, I32 len)
78+
{
79+
dVAR;
80+
HEK * hek;
81+
U32 hash;
82+
char * buf;
83+
PERL_ARGS_ASSERT_NEWCHEK;
84+
85+
if(!len)
86+
len = strlen(str);
87+
len +=2;
88+
/* was alloca */
89+
buf = sv_grow(sv_newmortal(),len);
90+
buf[0] = '_';
91+
buf[1] = '<';
92+
memcpy(&buf[2], str, len-2);
93+
PERL_HASH(hash, buf, len);
94+
hek = save_hek_flags(buf, len, hash, HVhek_COMPILING);
95+
return HEK2FNPV(hek);
96+
}
97+
#endif
98+
99+
/* When this creates CHEKs, it returns a HEK * from inside a CHEK.
100+
* The HEK * can be converted to a CHEK * if needed by the caller */
75101
STATIC HEK *
76102
S_save_hek_flags(const char *str, I32 len, U32 hash, int flags)
77103
{
@@ -81,8 +107,20 @@ S_save_hek_flags(const char *str, I32 len, U32 hash, int flags)
81107

82108
PERL_ARGS_ASSERT_SAVE_HEK_FLAGS;
83109

84-
Newx(k, HEK_BASESIZE + len + 2, char);
85-
hek = (HEK*)k;
110+
#ifdef USE_ITHREADS
111+
if(flags & HVhek_COMPILING) {
112+
dTHX;
113+
CHEK * chek = (CHEK*)PerlMemShared_malloc(STRUCT_OFFSET(CHEK, chek_hek.hek_key[0]) + len + 2);
114+
chek->chek_refcount = 1;
115+
hek = &chek->chek_hek;
116+
}
117+
else {
118+
#endif
119+
Newx(k, HEK_BASESIZE + len + 2, char);
120+
hek = (HEK*)k;
121+
#ifdef USE_ITHREADS
122+
}
123+
#endif
86124
Copy(str, HEK_KEY(hek), len, char);
87125
HEK_KEY(hek)[len] = 0;
88126
HEK_LEN(hek) = len;
@@ -94,6 +132,73 @@ S_save_hek_flags(const char *str, I32 len, U32 hash, int flags)
94132
return hek;
95133
}
96134

135+
#ifdef USE_ITHREADS
136+
137+
void
138+
Perl_free_copfile(pTHX_ COP * cop)
139+
{
140+
PERL_ARGS_ASSERT_FREE_COPFILE;
141+
if(CopFILE(cop)) {
142+
CHEK * chek = FNPV2CHEK(CopFILE(cop));
143+
CopFILE(cop) = NULL;
144+
chek_dec(chek);
145+
}
146+
}
147+
148+
void
149+
Perl_restore_copfile(pTHX_ void * idx)
150+
{
151+
SSCHEK* ssent = SSPTRt((Size_t)idx, SSCHEK);
152+
if(*ssent->where != CHEK2FNPV(ssent->what)) {
153+
CHEK * existing = FNPV2CHEK(*ssent->where);
154+
*ssent->where = CHEK2FNPV(ssent->what);
155+
chek_dec(existing);
156+
}
157+
else
158+
chek_dec(ssent->what);
159+
}
160+
161+
/* instead of SSNEW and SAVEDESTRUCTOR_X this probably needs its own save type
162+
* and croak if its save type is ever tried to be dup-ed. I need to research
163+
* what happens if 2 different threads restore at 2 random points the CopFILE */
164+
void
165+
Perl_save_copfile(pTHX_ COP * cop)
166+
{
167+
I32 idx = SSNEW(sizeof(void *)*2);
168+
SSCHEK* ssent = SSPTR(idx, SSCHEK*);
169+
CHEK * old = FNPV2CHEK(CopFILE(cop));
170+
PERL_ARGS_ASSERT_SAVE_COPFILE;
171+
ssent->what = old;
172+
ssent->where = &CopFILE(cop);
173+
SAVEDESTRUCTOR_X(Perl_restore_copfile,(void*)(Size_t)idx);
174+
chek_inc(old);
175+
}
176+
177+
void
178+
Perl_chek_inc(pTHX_ CHEK * chek)
179+
{
180+
dVAR;
181+
PERL_ARGS_ASSERT_CHEK_INC;
182+
OP_REFCNT_LOCK; /* atomic in future ? */
183+
chek->chek_refcount++;
184+
OP_REFCNT_UNLOCK;
185+
}
186+
187+
void
188+
Perl_chek_dec(pTHX_ CHEK * chek)
189+
{
190+
dVAR;
191+
U32 refcnt;
192+
PERL_ARGS_ASSERT_CHEK_DEC;
193+
OP_REFCNT_LOCK; /* atomic in future ? */
194+
refcnt = --chek->chek_refcount;
195+
OP_REFCNT_UNLOCK;
196+
if(!refcnt)
197+
PerlMemShared_free(chek);
198+
}
199+
200+
#endif
201+
97202
/* free the pool of temporary HE/HEK pairs returned by hv_fetch_ent
98203
* for tied hashes */
99204

@@ -1622,7 +1727,7 @@ S_hv_free_ent_ret(pTHX_ HV *hv, HE *entry)
16221727
}
16231728
else if (HvSHAREKEYS(hv))
16241729
unshare_hek(HeKEY_hek(entry));
1625-
else
1730+
else /* ??? research if a CHEK can wind up in a HE */
16261731
Safefree(HeKEY_hek(entry));
16271732
del_HE(entry);
16281733
return val;
@@ -2843,6 +2948,11 @@ S_unshare_hek_or_pvn(pTHX_ const HEK *hek, const char *str, I32 len, U32 hash)
28432948
struct shared_he *he = NULL;
28442949

28452950
if (hek) {
2951+
/* if CHEKs are stored in SVPVs like HEKs, for example caller
2952+
change here possibly */
2953+
#ifdef USE_ITHREADS
2954+
assert((HEK_FLAGS(hek) & HVhek_COMPILING) == 0);
2955+
#endif
28462956
/* Find the shared he which is just before us in memory. */
28472957
he = (struct shared_he *)(((char *)hek)
28482958
- STRUCT_OFFSET(struct shared_he,

0 commit comments

Comments
 (0)