Mailing List Archive

6% runtime speedup, ~20% memory gain (typical Tk app)
All for the price of a patch applicator! Buy now!

After a few more optimizations, I submit the attached patch for your
consideration (and enjoyment).

The patch as supplied seems give the best performance in my limited
tests. This does string-sharing for all hashes in the system, including for
symbol tables, literal keys, quoted keys, variable keys, and what have
you. This implementation does not have *any* extra cost associated with hash
FETCHes. Only STORE operations go through an extra hash lookup, and only
when the key is installed for the first time in the hash. The impact of this
is marginal, since the hash function is never computed more than once for
looking up the same string.

I have generalized the sharing of strings with a sharepvn() function that
could find use in other places in the source (in lieu of savepv() or
savepvn()). There is a complementary nosharepvn() function that is
applicable when refcounting. You can define REFCNT_STRTAB to do reference
counting of the strings in the table in order to free them when neccessary,
but I found that this only increases the run time without yielding any
significant memory savings. Without this define, any string, once installed
in the table, will have a full lifetime. There may be a case for
maintaining *two* string tables in the system, one refcounted and the other
not, but I haven't seen any test cases where this is warranted yet.

There is a new xhv_flags field (a U8) in the xpvhv structure that I have
used for the HVf_SHAREKEYS flag. (HVf_SVKEYS might soon be another). You
may explicitly mark a HV with the flag to enable/disable string sharing.

There a small change in the interface: the he_free() and he_delayfree()
functions now take an additional argument, a flag that specifies how the
entry must be destroyed. I could have made up two differently named clone
functions instead, but I think it is a good idea to add the flag parameter
for the future.

There are two additional functions to the interface to perl hashes:

HE *hv_fetch_ent(hv, key, klen, lval, hash);
HE *hv_store_ent(hv, key, klen, sv, hash);

Both return a hash entry structure when successful, which makes it possible
to carry the hash value around after calling one of these with a NULL hash
value.

And finally, you could define NODEFAULT_SHAREKEYS to turn off all sharing of
strings (essentially the perl of old).

I will post some excerpts from my bechmarking runs when I find more time
to compile something that's easier on the eyes.

Meanwhile, let me know if this works for you.

- Sarathy.
gsar@engin.umich.edu

P.S: Summary of my perl5 (patchlevel 2) configuration:
Platform:
osname=linux, osver=1, archname=i586-linux
uname='linux pujyam 1.2.13 #2 sun oct 8 17:33:58 edt 1995 i586 '
hint=recommended
Compiler:
cc='cc', optimize='-O -g', ld='cc'
cppflags='-DDEBUGGING -D__USE_BSD_SIGNAL -Dbool=char -DHAS_BOOL'
ccflags ='-DDEBUGGING -D__USE_BSD_SIGNAL -Dbool=char -DHAS_BOOL'
ldflags =' -L/usr/local/lib'
stdchar='char', d_stdstdio=define, usevfork=false
voidflags=15, castflags=0, d_casti32=undef, d_castneg=define
intsize=4, alignbytes=4, usemymalloc=n, randbits=31
Libraries:
so=so
libpth=/usr/local/lib /lib /usr/lib
libs=-lgdbm -ldbm -ldb -ldl -lm -lc -lbsd
libc=/usr/lib/libc.a
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef
cccdlflags='-fpic', ccdlflags='-rdynamic', lddlflags='-shared
-L/usr/local/lib'
---------------------------------8<---------------------------------
*** perl.h.dist Wed Nov 15 17:13:16 1995
--- perl.h Wed Dec 20 05:01:43 1995
***************
*** 1282,1287 ****
--- 1282,1288 ----
IEXT AV * Iendav; /* names of END subroutines */
IEXT AV * Ipad; /* storage for lexically scoped temporaries */
IEXT AV * Ipadname; /* variable names for "my" variables */
+ IEXT HV * Istrtab; /* shared string table */

/* memory management */
IEXT SV ** Itmps_stack;
*** perl.c.dist Sun Nov 19 16:11:29 1995
--- perl.c Wed Dec 20 05:00:18 1995
***************
*** 1165,1170 ****
--- 1165,1173 ----
init_main_stash()
{
GV *gv;
+
+ strtab = newHV();
+ HvSHAREKEYS_off(strtab); /* mandatory */
curstash = defstash = newHV();
curstname = newSVpv("main",4);
gv = gv_fetchpv("main::",TRUE, SVt_PVHV);
*** interp.sym.dist Fri Nov 10 17:17:32 1995
--- interp.sym Wed Dec 20 04:59:38 1995
***************
*** 139,144 ****
--- 139,145 ----
statusvalue
stdingv
strchop
+ strtab
sv_count
sv_objcount
sv_root
*** util.c.dist Tue Nov 14 10:46:37 1995
--- util.c Wed Dec 20 04:57:58 1995
***************
*** 690,695 ****
--- 690,759 ----
return newaddr;
}

+ /* get a (constant) string ptr from the global string table */
+ /* string will get added if it is not already there */
+
+ char *
+ sharepvn(sv, len, hash)
+ char *sv;
+ I32 len;
+ register U32 hash;
+ {
+ register HE *entry;
+
+ if (!hash) {
+ register char *s;
+ register I32 i;
+ i = len;
+ s = sv;
+ while (i--)
+ hash = hash * 33 + *s++;
+ }
+
+ if (!((entry = hv_fetch_ent(strtab, sv, len, TRUE, hash)) ||
+ (entry = hv_store_ent(strtab, sv, len, Nullsv, hash))))
+ croak("panic: fetch from string table failed");
+ #ifdef REFCNT_STRTAB
+ if (entry->hent_val == Nullsv) { /* new entry */
+ entry->hent_val = (SV*)1; /* use value slot as REFCNT */
+ }
+ else
+ ++(I32)entry->hent_val;
+ #endif
+ return entry->hent_key;
+ }
+
+ /* possibly free a shared string if no one has access to it */
+
+ void
+ unsharepvn(sv, len, hash)
+ char *sv;
+ I32 len;
+ register U32 hash;
+ {
+ #ifdef REFCNT_STRTAB
+ register HE *entry;
+
+ if (!hash) {
+ register char *s;
+ register I32 i;
+ i = len;
+ s = sv;
+ while (i--)
+ hash = hash * 33 + *s++;
+ }
+ /* XXX could play perilous pointer games here instead */
+ if ((entry = hv_fetch_ent(strtab, sv, len, FALSE, hash))) {
+ if (!--(I32)entry->hent_val) {
+ entry->hent_val = 0;
+ hv_delete(strtab, sv, len, G_DISCARD);
+ }
+ }
+ else
+ warn("Attempt to free non-existent shared string");
+ #endif
+ }
+
#if !defined(I_STDARG) && !defined(I_VARARGS)

/*
*** global.sym.dist Wed Nov 15 14:58:14 1995
--- global.sym Wed Dec 20 05:32:03 1995
***************
*** 397,402 ****
--- 397,403 ----
hv_delete
hv_exists
hv_fetch
+ hv_fetch_ent
hv_stashpv
hv_iterinit
hv_iterkey
***************
*** 405,410 ****
--- 406,412 ----
hv_iterval
hv_magic
hv_store
+ hv_store_ent
hv_undef
ibcmp
ingroup
***************
*** 874,879 ****
--- 876,882 ----
run
savepv
savepvn
+ sharepvn
save_I32
save_aptr
save_ary
***************
*** 973,978 ****
--- 976,982 ----
taint_proper
too_few_arguments
too_many_arguments
+ unsharepvn
wait4pid
warn
watch
*** proto.h.dist Wed Nov 15 21:55:23 1995
--- proto.h Wed Dec 20 04:47:51 1995
***************
*** 128,140 ****
void gv_init _((GV *gv, HV *stash, char *name, STRLEN len, int multi));
HV* gv_stashpv _((char* name, I32 create));
HV* gv_stashsv _((SV* sv, I32 create));
! void he_delayfree _((HE* hent));
! void he_free _((HE* hent));
void hoistmust _((PMOP* pm));
void hv_clear _((HV* tb));
SV* hv_delete _((HV* tb, char* key, U32 klen, I32 flags));
bool hv_exists _((HV* tb, char* key, U32 klen));
SV** hv_fetch _((HV* tb, char* key, U32 klen, I32 lval));
I32 hv_iterinit _((HV* tb));
char* hv_iterkey _((HE* entry, I32* retlen));
HE* hv_iternext _((HV* tb));
--- 128,141 ----
void gv_init _((GV *gv, HV *stash, char *name, STRLEN len, int multi));
HV* gv_stashpv _((char* name, I32 create));
HV* gv_stashsv _((SV* sv, I32 create));
! void he_delayfree _((HE* hent, I32 shared));
! void he_free _((HE* hent, I32 shared));
void hoistmust _((PMOP* pm));
void hv_clear _((HV* tb));
SV* hv_delete _((HV* tb, char* key, U32 klen, I32 flags));
bool hv_exists _((HV* tb, char* key, U32 klen));
SV** hv_fetch _((HV* tb, char* key, U32 klen, I32 lval));
+ HE* hv_fetch_ent _((HV* tb, char* key, U32 klen, I32 lval, U32 hash));
I32 hv_iterinit _((HV* tb));
char* hv_iterkey _((HE* entry, I32* retlen));
HE* hv_iternext _((HV* tb));
***************
*** 142,147 ****
--- 143,149 ----
SV* hv_iterval _((HV* tb, HE* entry));
void hv_magic _((HV* hv, GV* gv, int how));
SV** hv_store _((HV* tb, char* key, U32 klen, SV* val, U32 hash));
+ HE* hv_store_ent _((HV* tb, char* key, U32 klen, SV* val, U32 hash));
void hv_undef _((HV* tb));
I32 ibcmp _((U8* a, U8* b, I32 len));
I32 ingroup _((I32 testgid, I32 effective));
***************
*** 356,361 ****
--- 358,365 ----
#endif
char* savepv _((char* sv));
char* savepvn _((char* sv, I32 len));
+ char* sharepvn _((char* sv, I32 len, U32 hash));
+ void unsharepvn _((char* sv, I32 len, U32 hash));
void savestack_grow _((void));
void save_aptr _((AV** aptr));
AV* save_ary _((GV* gv));
*** hv.c.dist Wed Nov 15 15:00:05 1995
--- hv.c Wed Dec 20 04:44:29 1995
***************
*** 127,132 ****
--- 127,217 ----
return 0;
}

+ /* returns a HE * structure with the all fields set */
+ /* note that hent_val will be a mortal sv for MAGICAL hashes */
+ HE *
+ hv_fetch_ent(hv,key,klen,lval,hash)
+ HV *hv;
+ char *key;
+ U32 klen;
+ I32 lval;
+ register U32 hash;
+ {
+ register XPVHV* xhv;
+ register char *s;
+ register I32 i;
+ register HE *entry;
+ SV *sv;
+
+ if (!hv)
+ return 0;
+
+ xhv = (XPVHV*)SvANY(hv);
+ if (!hash) {
+ i = klen;
+ hash = 0;
+ s = key;
+ while (i--)
+ hash = hash * 33 + *s++;
+ }
+
+ if (SvRMAGICAL(hv) && mg_find((SV*)hv,'P')) {
+ if (!(entry = xhv->xhv_eiter)) {
+ xhv->xhv_eiter = entry = new_he(); /* only one HE per MAGICAL hash */
+ Zero(entry, 1, HE);
+ }
+ sv = sv_newmortal();
+ mg_copy((SV*)hv, sv, key, klen);
+ entry->hent_val = sv;
+ if (HvSHAREKEYS(hv))
+ entry->hent_key = sharepvn(key,klen,hash);
+ else {
+ Safefree(entry->hent_key);
+ entry->hent_key = savepvn(key, klen);
+ }
+ entry->hent_klen = klen;
+ return entry;
+ }
+
+ if (!xhv->xhv_array) {
+ if (lval
+ #ifdef DYNAMIC_ENV_FETCH /* if it's an %ENV lookup, we may get it on the fly */
+ || (HvNAME(hv) && strEQ(HvNAME(hv),ENV_HV_NAME))
+ #endif
+ )
+ Newz(503,xhv->xhv_array, sizeof(HE*) * (xhv->xhv_max + 1), char);
+ else
+ return 0;
+ }
+
+ entry = ((HE**)xhv->xhv_array)[hash & (I32) xhv->xhv_max];
+ for (; entry; entry = entry->hent_next) {
+ if (entry->hent_hash != hash) /* strings can't be equal */
+ continue;
+ if (entry->hent_klen != klen)
+ continue;
+ if (bcmp(entry->hent_key,key,klen)) /* is this it? */
+ continue;
+ return entry;
+ }
+ #ifdef DYNAMIC_ENV_FETCH /* %ENV lookup? If so, try to fetch the value now */
+ if (HvNAME(hv) && strEQ(HvNAME(hv),ENV_HV_NAME)) {
+ char *gotenv;
+
+ gotenv = my_getenv(key);
+ if (gotenv != NULL) {
+ sv = newSVpv(gotenv,strlen(gotenv));
+ return hv_store_ent(hv,key,klen,sv,hash);
+ }
+ }
+ #endif
+ if (lval) { /* gonna assign to this, so it better be there */
+ sv = NEWSV(61,0);
+ return hv_store_ent(hv,key,klen,sv,hash);
+ }
+ return 0;
+ }
+
SV**
hv_store(hv,key,klen,val,hash)
HV *hv;
***************
*** 183,189 ****

entry = new_he();
entry->hent_klen = klen;
! entry->hent_key = savepvn(key,klen);
entry->hent_val = val;
entry->hent_hash = hash;
entry->hent_next = *oentry;
--- 268,277 ----

entry = new_he();
entry->hent_klen = klen;
! if (HvSHAREKEYS(hv))
! entry->hent_key = sharepvn(key, klen, hash);
! else /* gotta do the real thing */
! entry->hent_key = savepvn(key,klen);
entry->hent_val = val;
entry->hent_hash = hash;
entry->hent_next = *oentry;
***************
*** 199,204 ****
--- 287,367 ----
return &entry->hent_val;
}

+ HE *
+ hv_store_ent(hv,key,klen,val,hash)
+ HV *hv;
+ char *key;
+ U32 klen;
+ SV *val;
+ register U32 hash;
+ {
+ register XPVHV* xhv;
+ register char *s;
+ register I32 i;
+ register HE *entry;
+ register HE **oentry;
+
+ if (!hv)
+ return 0;
+
+ xhv = (XPVHV*)SvANY(hv);
+ if (SvMAGICAL(hv)) {
+ mg_copy((SV*)hv, val, key, klen);
+ #ifndef OVERLOAD
+ if (!xhv->xhv_array)
+ return 0;
+ #else
+ if (!xhv->xhv_array && (SvMAGIC(hv)->mg_type != 'A'
+ || SvMAGIC(hv)->mg_moremagic))
+ return 0;
+ #endif /* OVERLOAD */
+ }
+ if (!hash) {
+ i = klen;
+ s = key;
+ while (i--)
+ hash = hash * 33 + *s++;
+ }
+
+ if (!xhv->xhv_array)
+ Newz(505, xhv->xhv_array, sizeof(HE**) * (xhv->xhv_max + 1), char);
+
+ oentry = &((HE**)xhv->xhv_array)[hash & (I32) xhv->xhv_max];
+ i = 1;
+
+ for (entry = *oentry; entry; i=0, entry = entry->hent_next) {
+ if (entry->hent_hash != hash) /* strings can't be equal */
+ continue;
+ if (entry->hent_klen != klen)
+ continue;
+ if (bcmp(entry->hent_key,key,klen)) /* is this it? */
+ continue;
+ SvREFCNT_dec(entry->hent_val);
+ entry->hent_val = val;
+ return entry;
+ }
+
+ entry = new_he();
+ entry->hent_klen = klen;
+ if (HvSHAREKEYS(hv))
+ entry->hent_key = sharepvn(key, klen, hash);
+ else /* gotta do the real thing */
+ entry->hent_key = savepvn(key,klen);
+ entry->hent_val = val;
+ entry->hent_hash = hash;
+ entry->hent_next = *oentry;
+ *oentry = entry;
+
+ xhv->xhv_keys++;
+ if (i) { /* initial entry? */
+ ++xhv->xhv_fill;
+ if (xhv->xhv_keys > xhv->xhv_max)
+ hsplit(hv);
+ }
+
+ return entry;
+ }
+
SV *
hv_delete(hv,key,klen,flags)
HV *hv;
***************
*** 253,259 ****
if (entry == xhv->xhv_eiter)
entry->hent_klen = -1;
else
! he_free(entry);
--xhv->xhv_keys;
return sv;
}
--- 416,422 ----
if (entry == xhv->xhv_eiter)
entry->hent_klen = -1;
else
! he_free(entry, HvSHAREKEYS(hv));
--xhv->xhv_keys;
return sv;
}
***************
*** 337,342 ****
--- 500,506 ----
assert(tmp >= newsize);
New(2,a, tmp, HE*);
Copy(xhv->xhv_array, a, oldsize, HE*);
+ #if 0
if (oldsize >= 64 && *(char*)&xhv->xnv_nv == 0) {
sv_add_arena((char*)xhv->xhv_array, oldsize * sizeof(HE*), 0);
sv_add_arena(((char*)a) + newsize * sizeof(HE*),
***************
*** 344,349 ****
--- 508,514 ----
SVf_FAKE);
}
else
+ #endif
Safefree(xhv->xhv_array);
#endif

***************
*** 384,389 ****
--- 549,557 ----
xhv = (XPVHV*)SvANY(hv);
SvPOK_off(hv);
SvNOK_off(hv);
+ #ifndef NODEFAULT_SHAREKEYS
+ HvSHAREKEYS_on(hv); /* key-sharing on by default */
+ #endif
xhv->xhv_max = 7; /* start with 8 buckets */
xhv->xhv_fill = 0;
xhv->xhv_pmroot = 0;
***************
*** 393,416 ****
}

void
! he_free(hent)
register HE *hent;
{
if (!hent)
return;
SvREFCNT_dec(hent->hent_val);
! Safefree(hent->hent_key);
del_he(hent);
}

void
! he_delayfree(hent)
register HE *hent;
{
if (!hent)
return;
sv_2mortal(hent->hent_val); /* free between statements */
! Safefree(hent->hent_key);
del_he(hent);
}

--- 561,600 ----
}

void
! he_free(hent, shared)
register HE *hent;
+ I32 shared;
{
if (!hent)
return;
SvREFCNT_dec(hent->hent_val);
! if (shared)
! #ifdef REFCNT_STRTAB
! unsharepvn(hent->hent_key, hent->hent_klen, hent->hent_hash);
! #else
! hent->hent_key = 0;
! #endif
! else
! Safefree(hent->hent_key);
del_he(hent);
}

void
! he_delayfree(hent, shared)
register HE *hent;
+ I32 shared;
{
if (!hent)
return;
sv_2mortal(hent->hent_val); /* free between statements */
! if (shared)
! #ifdef REFCNT_STRTAB
! unsharepvn(hent->hent_key, hent->hent_klen, hent->hent_hash);
! #else
! hent->hent_key = 0;
! #endif
! else
! Safefree(hent->hent_key);
del_he(hent);
}

***************
*** 441,446 ****
--- 625,631 ----
register HE *ohent = Null(HE*);
I32 riter;
I32 max;
+ I32 shared;

if (!hv)
return;
***************
*** 451,461 ****
max = HvMAX(hv);
array = HvARRAY(hv);
hent = array[0];
for (;;) {
if (hent) {
ohent = hent;
hent = hent->hent_next;
! he_free(ohent);
}
if (!hent) {
if (++riter > max)
--- 636,647 ----
max = HvMAX(hv);
array = HvARRAY(hv);
hent = array[0];
+ shared = HvSHAREKEYS(hv);
for (;;) {
if (hent) {
ohent = hent;
hent = hent->hent_next;
! he_free(ohent, shared);
}
if (!hent) {
if (++riter > max)
***************
*** 478,487 ****
--- 664,677 ----
#ifdef STRANGE_MALLOC
Safefree(xhv->xhv_array);
#else
+ #if 0
if (xhv->xhv_max < 127 || *(char*)&xhv->xnv_nv)
Safefree(xhv->xhv_array);
else /* We used last half, so use first half for SV arena too. */
sv_add_arena((char*)xhv->xhv_array, (xhv->xhv_max + 1) * sizeof(HE*),0);
+ #else
+ Safefree(xhv->xhv_array);
+ #endif
#endif
if (HvNAME(hv)) {
Safefree(HvNAME(hv));
***************
*** 504,510 ****
register XPVHV* xhv = (XPVHV*)SvANY(hv);
HE *entry = xhv->xhv_eiter;
if (entry && entry->hent_klen < 0) /* was deleted earlier? */
! he_free(entry);
xhv->xhv_riter = -1;
xhv->xhv_eiter = Null(HE*);
return xhv->xhv_fill;
--- 694,700 ----
register XPVHV* xhv = (XPVHV*)SvANY(hv);
HE *entry = xhv->xhv_eiter;
if (entry && entry->hent_klen < 0) /* was deleted earlier? */
! he_free(entry, HvSHAREKEYS(hv));
xhv->xhv_riter = -1;
xhv->xhv_eiter = Null(HE*);
return xhv->xhv_fill;
***************
*** 526,547 ****

if (SvRMAGICAL(hv) && (mg = mg_find((SV*)hv,'P'))) {
SV *key = sv_newmortal();
if (entry) {
! sv_usepvn(key, entry->hent_key, entry->hent_klen);
entry->hent_key = 0;
}
else {
! xhv->xhv_eiter = entry = new_he();
Zero(entry, 1, HE);
}
magic_nextpack((SV*) hv,mg,key);
if (SvOK(key)) {
STRLEN len;
! entry->hent_key = SvPV_force(key, len);
entry->hent_klen = len;
! SvPOK_off(key);
! SvPVX(key) = 0;
! return entry;
}
if (entry->hent_val)
SvREFCNT_dec(entry->hent_val);
--- 716,748 ----

if (SvRMAGICAL(hv) && (mg = mg_find((SV*)hv,'P'))) {
SV *key = sv_newmortal();
+ I32 shared = HvSHAREKEYS(hv);
if (entry) {
! if (shared)
! sv_setpvn(key, entry->hent_key, entry->hent_klen);
! else
! sv_usepvn(key, entry->hent_key, entry->hent_klen);
entry->hent_key = 0;
}
else {
! xhv->xhv_eiter = entry = new_he(); /* only one HE per MAGICAL hash */
Zero(entry, 1, HE);
}
magic_nextpack((SV*) hv,mg,key);
if (SvOK(key)) {
STRLEN len;
! char *ckey;
!
! ckey = SvPV_force(key, len);
! if (shared)
! entry->hent_key = sharepvn(ckey, len, 0);
! else {
! entry->hent_key = ckey;
! SvPOK_off(key);
! SvPVX(key) = 0;
! }
entry->hent_klen = len;
! return entry; /* note that hent_val is not set at all */
}
if (entry->hent_val)
SvREFCNT_dec(entry->hent_val);
***************
*** 566,572 ****
} while (!entry);

if (oldentry && oldentry->hent_klen < 0) /* was deleted earlier? */
! he_free(oldentry);

xhv->xhv_eiter = entry;
return entry;
--- 767,773 ----
} while (!entry);

if (oldentry && oldentry->hent_klen < 0) /* was deleted earlier? */
! he_free(oldentry, HvSHAREKEYS(hv));

xhv->xhv_eiter = entry;
return entry;
*** hv.h.dist Tue Oct 18 12:20:08 1994
--- hv.h Wed Dec 20 00:58:12 1995
***************
*** 30,37 ****
--- 30,41 ----
HE *xhv_eiter; /* current entry of iterator */
PMOP *xhv_pmroot; /* list of pm's for this package */
char *xhv_name; /* name, if a symbol table */
+ U8 xhv_flags;
};

+ #define HVf_SHAREKEYS 1 /* shares its (const) keys */
+ /*#define HVf_SVKEYS 2 *//* uses SV keys */
+
#define Nullhv Null(HV*)
#define HvARRAY(hv) ((HE**)((XPVHV*) SvANY(hv))->xhv_array)
#define HvFILL(hv) ((XPVHV*) SvANY(hv))->xhv_fill
***************
*** 41,46 ****
--- 45,56 ----
#define HvEITER(hv) ((XPVHV*) SvANY(hv))->xhv_eiter
#define HvPMROOT(hv) ((XPVHV*) SvANY(hv))->xhv_pmroot
#define HvNAME(hv) ((XPVHV*) SvANY(hv))->xhv_name
+
+ #define HvFLAGS(hv) ((XPVHV*) SvANY(hv))->xhv_flags
+
+ #define HvSHAREKEYS(hv) (HvFLAGS(hv) & HVf_SHAREKEYS)
+ #define HvSHAREKEYS_on(hv) (HvFLAGS(hv) |= HVf_SHAREKEYS)
+ #define HvSHAREKEYS_off(hv) (HvFLAGS(hv) &= ~HVf_SHAREKEYS)

#ifdef OVERLOAD

*** av.c.dist Wed Nov 15 14:26:17 1995
--- av.c Sat Dec 9 23:03:32 1995
***************
*** 82,90 ****
newmax = tmp - 1;
New(2,ary, newmax+1, SV*);
Copy(AvALLOC(av), ary, AvMAX(av)+1, SV*);
if (AvMAX(av) > 64 && !AvREUSED(av))
! sv_add_arena((char*)AvALLOC(av), AvMAX(av) * sizeof(SV*),0);
else
Safefree(AvALLOC(av));
AvALLOC(av) = ary;
#endif
--- 82,92 ----
newmax = tmp - 1;
New(2,ary, newmax+1, SV*);
Copy(AvALLOC(av), ary, AvMAX(av)+1, SV*);
+ #ifdef 0
if (AvMAX(av) > 64 && !AvREUSED(av))
! sv_add_arena((char*)AvALLOC(av), (AvMAX(av)+1) * sizeof(SV*),0);
else
+ #endif
Safefree(AvALLOC(av));
AvALLOC(av) = ary;
#endif
***************
*** 317,326 ****
key = AvFILL(av) + 1;
while (key)
SvREFCNT_dec(AvARRAY(av)[--key]);
- }
- if (key = AvARRAY(av) - AvALLOC(av)) {
- AvMAX(av) += key;
- SvPVX(av) = (char*)AvALLOC(av);
}
Safefree(AvALLOC(av));
AvALLOC(av) = 0;
--- 319,324 ----
*** av.h.dist Wed Nov 15 14:26:41 1995
--- av.h Sat Dec 9 18:45:40 1995
***************
*** 44,48 ****
#define AvREUSED_on(av) (AvFLAGS(av) |= AVf_REUSED)
#define AvREUSED_off(av) (AvFLAGS(av) &= ~AVf_REUSED)

! #define AvREALISH(av) AvFLAGS(av) /* REAL or REIFY -- shortcut */

--- 44,48 ----
#define AvREUSED_on(av) (AvFLAGS(av) |= AVf_REUSED)
#define AvREUSED_off(av) (AvFLAGS(av) &= ~AVf_REUSED)

! #define AvREALISH(av) (AvFLAGS(av) & (AVf_REAL|AVf_REIFY))
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
>Hmm, interesting - maybe with that I can loose some tcl-ish hashes...

Were they tied previously? :-)

I know what 6% speed up means, but is 20% memory gain meaning it
runs in 80% of the previous memory or 120%?

--tom
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
> From: Gurusamy Sarathy <gsar@engin.umich.edu>
>
> All for the price of a patch applicator! Buy now!

Pay later! (sorry, only joking :-)

Wow! This is good work.

I've only got a few very minor nit-picks in an otherwise well thought out patch.


> There a small change in the interface: the he_free() and he_delayfree()
> functions now take an additional argument

Not a problem since I'm sure nobody's used them in external code.


> + char *
> + sharepvn(sv, len, hash)
> + char *sv;

Calling the char* 'sv' is not a good idea!


> --- hv.h Wed Dec 20 00:58:12 1995
> HE *xhv_eiter; /* current entry of iterator */
> PMOP *xhv_pmroot; /* list of pm's for this package */
> char *xhv_name; /* name, if a symbol table */
> + U8 xhv_flags;
> };
>
> + #define HVf_SHAREKEYS 1 /* shares its (const) keys */
> + /*#define HVf_SVKEYS 2 *//* uses SV keys */
> +

> + #define HvFLAGS(hv) ((XPVHV*) SvANY(hv))->xhv_flags
> +
> + #define HvSHAREKEYS(hv) (HvFLAGS(hv) & HVf_SHAREKEYS)
> + #define HvSHAREKEYS_on(hv) (HvFLAGS(hv) |= HVf_SHAREKEYS)
> + #define HvSHAREKEYS_off(hv) (HvFLAGS(hv) &= ~HVf_SHAREKEYS)

You could define SV private flags for the HV type in sv.h:

#define SVphv_SHAREKEYS 0x80000000
#define SVphv_SVKEYS 0x40000000

and so avoid the overhead of an extra field in every hash.


Speaking of which, it seems very inefficient for every hash to carry
xhv_pmroot and xhv_name fields when they only apply to symbol tables.

This is especially important when you realise that currently the xpvhv
struct is 64 bytes long which means that malloc overhead forces each
xpvhv struct into a 128 byte bucket! 64 wasted bytes per HV!

I think these two fields could be combined into a pointer to another
struct which contains the xhv_pmroot and xhv_name fields. That struct
only need be allocated for symbol table hashes.

That would recover the four bytes needed to keep HV's in the 64 byte bucket
(so long as xhv_flags is also deleted in favor of an SV private flag).

Gurusamy, could you implement this in an updated patch? (if no one complains)
That should increase the memory saving even further!


> --- av.h Sat Dec 9 18:45:40 1995
>
> ! #define AvREALISH(av) AvFLAGS(av) /* REAL or REIFY -- shortcut */
>
> --- 44,48 ----
>
> ! #define AvREALISH(av) (AvFLAGS(av) & (AVf_REAL|AVf_REIFY))

This seems unrelated. What impact does this have?

Tim.
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
Gurusamy Sarathy wrote :
|| All for the price of a patch applicator! Buy now!

Less code. Less filling. More taste. Sounds great.

|| I have generalized the sharing of strings with a sharepvn() function that
|| could find use in other places in the source (in lieu of savepv() or
|| savepvn()). There is a complementary nosharepvn() function that is
|| applicable when refcounting. You can define REFCNT_STRTAB to do reference
|| counting of the strings in the table in order to free them when neccessary,
|| but I found that this only increases the run time without yielding any
|| significant memory savings. Without this define, any string, once installed
|| in the table, will have a full lifetime. There may be a case for
|| maintaining *two* string tables in the system, one refcounted and the other
|| not, but I haven't seen any test cases where this is warranted yet.

I'm probably misunderstanding, but I read this as meaning that
once a string gets used as a hash then a copy of it is kept in
memory until the end of the program. This could be a serious
proble for server programs that build up a scratch hash for each
service request and then delete that hash when the request has
been satisfied. If any part of the key is unique to the
particular request (e.g. a request-ID or timestamp) then this
will end up being a memory leak (or at least looking like one to
the user). I hope I'm wrong about this.

--
Maybe we can fix reality one of these days. | John Macdonald
<- Larry Wall Carl Dichter -> | jmm@Elegant.COM
I'd be happy if we could just index it. |
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995 06:35:08 MST, Tom Christiansen wrote:
>>Hmm, interesting - maybe with that I can loose some tcl-ish hashes...
>
>Were they tied previously? :-)
>
>I know what 6% speed up means, but is 20% memory gain meaning it
>runs in 80% of the previous memory or 120%?
>

Didn't you know that perlers always get the best of both worlds?

In this case actually, I was shooting for memory savings with shared
strings, but the speed increase was a pleasant surprise. I hadn't realized
that all those little bazzillion mallocs and free's add up to that much.

"We do it with strings." (Er, more like an Indian rope trick, actually.)

- Sarathy.
gsar@engin.umich.edu

(Hark! Is it a dream, or do I hear the favorite jingle of the season wafting
through the ether..)

String o' perls, string o' perls,
String 'em all the way;
Hash 'em--share 'em--string 'em,
And stash 'em all away..

..ad mellifluum..
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
>..ad mellifluum..

ah, the sweet smoothness of latin honey. :-)

--tom
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
>..ad mellifluum..

ah, the sweet smoothness of latin honey. :-)

--tom
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995 14:02:33 GMT, Tim Bunce wrote:
>
>> From: Gurusamy Sarathy <gsar@engin.umich.edu>
>>
>> All for the price of a patch applicator! Buy now!
>
>Pay later! (sorry, only joking :-)

I was trying to get some last-minute Christmas season sales wrapped up :-)

>
>> + char *
>> + sharepvn(sv, len, hash)
>> + char *sv;
>
>Calling the char* 'sv' is not a good idea!
>

Actually, this is a result of cut-n-paste from from savepvn(). Perl has,
char *sv in many places. "string value", I tend to mutter to myself,
when I see one of those.

Perhaps 'tis time for Tom to go over the sources and make the variable names
suitably august Greaco-Roman to give them that one-ness of purpose :-)

>>
>> + #define HVf_SHAREKEYS 1 /* shares its (const) keys */
>> + /*#define HVf_SVKEYS 2 *//* uses SV keys */
>> +
>
>> + #define HvFLAGS(hv) ((XPVHV*) SvANY(hv))->xhv_flags
>> +
>> + #define HvSHAREKEYS(hv) (HvFLAGS(hv) & HVf_SHAREKEYS)
>> + #define HvSHAREKEYS_on(hv) (HvFLAGS(hv) |= HVf_SHAREKEYS)
>> + #define HvSHAREKEYS_off(hv) (HvFLAGS(hv) &= ~HVf_SHAREKEYS)
>
>You could define SV private flags for the HV type in sv.h:
>
> #define SVphv_SHAREKEYS 0x80000000
> #define SVphv_SVKEYS 0x40000000
>
>and so avoid the overhead of an extra field in every hash.

I had considered this, but Ilya (Hi!) seemed to have consumed the last bits
of the edible portions. But seeing this:

/* #define SVpgv_badAM 0x20000000 */
[..]
/*
#define Gv_AMG(stash) \
(HV_AMAGICmb(stash) && \
((!HV_AMAGICbad(stash) && HV_AMAGIC(stash)) || Gv_AMupdate(stash)))
*/
[..]
/* #define HV_AMAGICmb(hv) (SvFLAGS(hv) & (SVpgv_badAM | SVpgv_AM)) */
[..]
/*
#define HV_AMAGICbad(hv) (SvFLAGS(hv) & SVpgv_badAM)
#define HV_badAMAGIC_on(hv) (SvFLAGS(hv) |= SVpgv_badAM)
#define HV_badAMAGIC_off(hv) (SvFLAGS(hv) &= ~SVpgv_badAM)
*/

had some HV_UNKNOWNbad effect on my brain. Perhaps Ilya is marking bit
territory here? :-) Anyway, I decided we couldn't rely on just one
free slot, late last night. If we can get those slots for the HV's,
I certainly can reorient myself to the privacy of them not-so-private
flags.

>
>Speaking of which, it seems very inefficient for every hash to carry
>xhv_pmroot and xhv_name fields when they only apply to symbol tables.
>
>This is especially important when you realise that currently the xpvhv
>struct is 64 bytes long which means that malloc overhead forces each
>xpvhv struct into a 128 byte bucket! 64 wasted bytes per HV!
>
>I think these two fields could be combined into a pointer to another
>struct which contains the xhv_pmroot and xhv_name fields. That struct
>only need be allocated for symbol table hashes.

This will force the code to make the ever-so-slight distinction between
symbol-tables and hashes. I just want to be sure that's OK, and won't
bite us later.

>
>That would recover the four bytes needed to keep HV's in the 64 byte bucket
>(so long as xhv_flags is also deleted in favor of an SV private flag).
>
>Gurusamy, could you implement this in an updated patch? (if no one complains)
:-)
>That should increase the memory saving even further!

It's already steaming in the patch steamer.

>>
>> ! #define AvREALISH(av) AvFLAGS(av) /* REAL or REIFY -- shortcut */
>> --- 44,48 ----
>> ! #define AvREALISH(av) (AvFLAGS(av) & (AVf_REAL|AVf_REIFY))
>
>This seems unrelated. What impact does this have?

That's just a "random cleanup". There has been a new AVf_REUSED flag added
since, so that wasn't ideal.

- Sarathy.
gsar@engin.umich.edu
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995 10:56:37 EST, John Macdonald wrote:
>|| You can define REFCNT_STRTAB to do reference
>|| counting of the strings in the table in order to free them when neccessary,
>|| but I found that this only increases the run time without yielding any
>|| significant memory savings. Without this define, any string, once installed
>|| in the table, will have a full lifetime. There may be a case for
>|| maintaining *two* string tables in the system, one refcounted and the other
>|| not, but I haven't seen any test cases where this is warranted yet.
>
>I'm probably misunderstanding, but I read this as meaning that
>once a string gets used as a hash then a copy of it is kept in
>memory until the end of the program. This could be a serious
>proble for server programs that build up a scratch hash for each
>service request and then delete that hash when the request has
>been satisfied. If any part of the key is unique to the
>particular request (e.g. a request-ID or timestamp) then this
>will end up being a memory leak (or at least looking like one to
>the user). I hope I'm wrong about this.

This is exactly the case I had in mind when I was thinking about
*two* string tables, but choosing one or the other for any given hash
will be a problem.

Oh well, we can probably live with ref-counting always, if you think
that is serious enough. Just make -DREFCNT_STRTAB the default. It still is
faster than current perl, but only about 2-3%, I think.

- Sarathy.
gsar@engin.umich.edu
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
Gurusamy Sarathy writes:
> >You could define SV private flags for the HV type in sv.h:
> >
> > #define SVphv_SHAREKEYS 0x80000000
> > #define SVphv_SVKEYS 0x40000000
> >
> >and so avoid the overhead of an extra field in every hash.
>
> I had considered this, but Ilya (Hi!) seemed to have consumed the last bits
> of the edible portions. But seeing this:
>
> /* #define SVpgv_badAM 0x20000000 */
> [..]
> /*
> #define Gv_AMG(stash) \
> (HV_AMAGICmb(stash) && \
> ((!HV_AMAGICbad(stash) && HV_AMAGIC(stash)) || Gv_AMupdate(stash)))
> */
> [..]
> /* #define HV_AMAGICmb(hv) (SvFLAGS(hv) & (SVpgv_badAM | SVpgv_AM)) */
> [..]
> /*
> #define HV_AMAGICbad(hv) (SvFLAGS(hv) & SVpgv_badAM)
> #define HV_badAMAGIC_on(hv) (SvFLAGS(hv) |= SVpgv_badAM)
> #define HV_badAMAGIC_off(hv) (SvFLAGS(hv) &= ~SVpgv_badAM)
> */
>
> had some HV_UNKNOWNbad effect on my brain. Perhaps Ilya is marking bit
> territory here? :-) Anyway, I decided we couldn't rely on just one
> free slot, late last night. If we can get those slots for the HV's,
> I certainly can reorient myself to the privacy of them not-so-private
> flags.
>

While it would be nice to have (eventually) 2 bits for AMAGIC, feel
free to delete these comments. They are needed on symbol stashes only, so
may be put into the additional parts (stashes - hashes) when you do
stashes and hashes as two different entities. (This may cost an extra
indirection, though.)

Ilya

P.S. Can you definitely explain whether

$c = 3; $xxx='a'; do {print qq{Found $xxx} if defined $$xxx} while $xxx++

a) Leaks now;
b) May be made non-leaking now;
c) Leaks with your patch;
d) May be made non-leaking with your patch.
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
: There may be a case for
: maintaining *two* string tables in the system, one refcounted and the other
: not, but I haven't seen any test cases where this is warranted yet.

Well, it's pretty easy to predict what it would take to grow forever.
A pathological application would be one that repeatedly brings in an
arbitrary set of keys and then discards them. This could well impact
some servers, especially any that are doing proxying on behalf of the
whole world.

#ifdefs aren't good enough--we have to be able to turn it off with a pragma:

no sharing;

Pity one can't apply prototypes to a "use" list:

no sharing %temp;

would have to be

no sharing '%temp';

or

no sharing \%temp;

Perhaps it's sufficient to have

my %shared;
{
no sharing;
my %unshared;
}

I dunno, maybe a pragma is just the wrong thing. But it seems like we
might need to know at compile time. Just thinking out loud here...

Larry
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995 13:02:32 EST, Ilya Zakharevich wrote:
>
>While it would be nice to have (eventually) 2 bits for AMAGIC, feel
>free to delete these comments. They are needed on symbol stashes only, so
>may be put into the additional parts (stashes - hashes) when you do
>stashes and hashes as two different entities. (This may cost an extra
>indirection, though.)
>

Ok. Thanks.

>Ilya
>
>P.S. Can you definitely explain whether
>
>$c = 3; $xxx='a'; do {print qq{Found $xxx} if defined $$xxx} while $xxx++
>
>a) Leaks now;

Yes. Here is a less "noisy" version:

while (++$a) { 1 if defined $$a }

I think there is a good explanation for it. That does the equivalent of a
gensym on the symbol-table, forever. Kinda same as

while (1) { $symtab{++$a} = '' }

only, eating more memory per iter.

>b) May be made non-leaking now;

Well, if you optimize anything leading to autovivification to not install
things in the symtab permanently when the target symbol is not getting
assigned to. So no, my patch won't work for that.

>c) Leaks with your patch;

Like hell.

>d) May be made non-leaking with your patch.

Hell, no. :-)

- Sarathy.
gsar@engin.umich.edu

P.S: By the way, I forgot to mention you need to run "make regen_headers" if
you apply my latest patch. Sorry if somebody had reason to swear at me
'cause of that.
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
Gurusamy Sarathy wrote :
|| >I'm probably misunderstanding, but I read this as meaning that
|| >once a string gets used as a hash then a copy of it is kept in
|| >memory until the end of the program. This could be a serious
|| >proble for server programs that build up a scratch hash for each
|| >service request and then delete that hash when the request has
|| >been satisfied. If any part of the key is unique to the
|| >particular request (e.g. a request-ID or timestamp) then this
|| >will end up being a memory leak (or at least looking like one to
|| >the user). I hope I'm wrong about this.
||
|| This is exactly the case I had in mind when I was thinking about
|| *two* string tables, but choosing one or the other for any given hash
|| will be a problem.
||
|| Oh well, we can probably live with ref-counting always, if you think
|| that is serious enough. Just make -DREFCNT_STRTAB the default. It still is
|| faster than current perl, but only about 2-3%, I think.

For that sort of speed difference, I'd make -DREFCNT_STRTAB the
default - it's still improving the speed and it is safely
reducing memory. Rather than a compile-time flag, it might be a
run-time flag triggered by "use less cpu;" (with the default
being reinstated by "use less memory;"). This speedup is only
going to be worth program modification though if there were
other speed/memory trade-offs bundled in - so I'd leave
switching to a run-time flag for 5.003 unless it is very easy.

--
Maybe we can fix reality one of these days. | John Macdonald
<- Larry Wall Carl Dichter -> | jmm@Elegant.COM
I'd be happy if we could just index it. |
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
Gurusamy Sarathy writes:
>
> On Wed, 20 Dec 1995 13:02:32 EST, Ilya Zakharevich wrote:
> >
> >While it would be nice to have (eventually) 2 bits for AMAGIC, feel
> >free to delete these comments. They are needed on symbol stashes only, so
> >may be put into the additional parts (stashes - hashes) when you do
> >stashes and hashes as two different entities. (This may cost an extra
> >indirection, though.)
> >
>
> Ok. Thanks.
>
> >Ilya
> >
> >P.S. Can you definitely explain whether
> >
> >$c = 3; $xxx='a'; do {print qq{Found $xxx} if defined $$xxx} while $xxx++
> >
> >a) Leaks now;
>
> Yes. Here is a less "noisy" version:
>
> while (++$a) { 1 if defined $$a }
>
> I think there is a good explanation for it. That does the equivalent of a
> gensym on the symbol-table, forever. Kinda same as
>
> while (1) { $symtab{++$a} = '' }
>
> only, eating more memory per iter.
>
> >b) May be made non-leaking now;
>
> Well, if you optimize anything leading to autovivification to not install
> things in the symtab permanently when the target symbol is not getting
> assigned to. So no, my patch won't work for that.

OK, this should not leak now:

$c = 3; $xxx='a'; do {print qq{Found $xxx} if defined $::{$xxx}} while $xxx++

>
> >c) Leaks with your patch;
>
> Like hell.
>
> >d) May be made non-leaking with your patch.
>
> Hell, no. :-)


I think this makes your patch into a grave :-(. Either have keys
refcounted, or have a global cache with the strings coming from
constant items, keeping the others as they are now.

The latter will lead to two lookups for keys which did not appear in
constant items, but the overhead will be neglegeable in the current
perl, when a lot of optimizations are not applied yet.

Maybe one may trigger writing a key into "eternal" hash using
"compiling" flag.

Ilya
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
Gurusamy Sarathy writes:
>
> On Wed, 20 Dec 1995 13:02:32 EST, Ilya Zakharevich wrote:
> >
> >While it would be nice to have (eventually) 2 bits for AMAGIC, feel
> >free to delete these comments. They are needed on symbol stashes only, so
> >may be put into the additional parts (stashes - hashes) when you do
> >stashes and hashes as two different entities. (This may cost an extra
> >indirection, though.)
> >
>
> Ok. Thanks.
>
> >Ilya
> >
> >P.S. Can you definitely explain whether
> >
> >$c = 3; $xxx='a'; do {print qq{Found $xxx} if defined $$xxx} while $xxx++
> >
> >a) Leaks now;
>
> Yes. Here is a less "noisy" version:
>
> while (++$a) { 1 if defined $$a }
>
> I think there is a good explanation for it. That does the equivalent of a
> gensym on the symbol-table, forever. Kinda same as
>
> while (1) { $symtab{++$a} = '' }
>
> only, eating more memory per iter.
>
> >b) May be made non-leaking now;
>
> Well, if you optimize anything leading to autovivification to not install
> things in the symtab permanently when the target symbol is not getting
> assigned to. So no, my patch won't work for that.

OK, this should not leak now:

$c = 3; $xxx='a'; do {print qq{Found $xxx} if defined $::{$xxx}} while $xxx++

>
> >c) Leaks with your patch;
>
> Like hell.
>
> >d) May be made non-leaking with your patch.
>
> Hell, no. :-)


I think this makes your patch into a grave :-(. Either have keys
refcounted, or have a global cache with the strings coming from
constant items, keeping the others as they are now.

The latter will lead to two lookups for keys which did not appear in
constant items, but the overhead will be neglegeable in the current
perl, when a lot of optimizations are not applied yet.

Maybe one may trigger writing a key into "eternal" hash using
"compiling" flag.

Ilya
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
Larry Wall wrote :
|| : There may be a case for
|| : maintaining *two* string tables in the system, one refcounted and the other
|| : not, but I haven't seen any test cases where this is warranted yet.
||
|| Well, it's pretty easy to predict what it would take to grow forever.
|| A pathological application would be one that repeatedly brings in an
|| arbitrary set of keys and then discards them. This could well impact
|| some servers, especially any that are doing proxying on behalf of the
|| whole world.
||
|| #ifdefs aren't good enough--we have to be able to turn it off with a pragma:

I don't think that it is necessary to be able to specify that a
particular hash doesn't get its keys shared. The important item
is doing the refcounting on those keys that are kept in the
shared area. If the program doesn't carefully delete obsolete
elements from its hashes, then it will run out of memory
regardles of whether that hash is getting its keys shared (it
just might affect *when* it runs out of memory in a significant
way - taking a day instead of a month might be noticeable :-).

Now, if a particular hash is going to always have essentially
unique keys, then it would be a bit cheaper to not share its
keys, I suppose - they get inserted and deleted in their own
(smaller) has rather than the big shared hash, but that may not
be a big enough saving to warrant the degree of programmer
responsibility to explicitly classify hashes.

--
Maybe we can fix reality one of these days. | John Macdonald
<- Larry Wall Carl Dichter -> | jmm@Elegant.COM
I'd be happy if we could just index it. |
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
In <199512201210.HAA16920@aatma.engin.umich.edu>
On Wed, 20 Dec 1995 07:10:57 -0500
Gurusamy Sarathy <gsar@engin.umich.edu> writes:
>There are two additional functions to the interface to perl hashes:
>
> HE *hv_fetch_ent(hv, key, klen, lval, hash);
> HE *hv_store_ent(hv, key, klen, sv, hash);
>
>Both return a hash entry structure when successful, which makes it possible
>to carry the hash value around after calling one of these with a NULL hash
>value.

Hmm, interesting - maybe with that I can loose some tcl-ish hashes...
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995 11:06:16 PST, Larry Wall wrote:
>
>Perhaps it's sufficient to have
>
> my %shared;
> {
> no sharing;
> my %unshared;
> }

I prefer the block scoped think if we go with the pragma. What would be a
$^CHAR to use? $^S perhaps. Or is that taken now?

>
>I dunno, maybe a pragma is just the wrong thing. But it seems like we
>might need to know at compile time. Just thinking out loud here...
>

Run-time identification of shareable hashes seems simpler (both to
implement, and to the user).

Just some mucking around in newHV(), set the HvSHAREDKEYS() flag based on
color of the Iwantshared global-to-be and we'll be done implementing.

User can set $^S to get needed effects at runtime. Simple.

Note that right now HvSHAREDKEYS() can be set only once in the lifetime of a
HV, *and* it is all-or-nothing. You get all keys shared or none at all,
which's OK, I say. I was considering adding support to iterate on said HASH
and savepvn() the keys to get a HvSHAREDKEYS_off() in midstream. That may
not be worth the while though, because I see little use for it this very
minute.

- Sarathy.
gsar@engin.umich.edu
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995 14:19:19 EST, John Macdonald wrote:
>|| Oh well, we can probably live with ref-counting always, if you think
>|| that is serious enough. Just make -DREFCNT_STRTAB the default. It still is
>|| faster than current perl, but only about 2-3%, I think.
>
>For that sort of speed difference, I'd make -DREFCNT_STRTAB the
>default - it's still improving the speed and it is safely
>reducing memory. Rather than a compile-time flag, it might be a
>run-time flag triggered by "use less cpu;" (with the default
>being reinstated by "use less memory;"). This speedup is only
>going to be worth program modification though if there were
>other speed/memory trade-offs bundled in - so I'd leave
>switching to a run-time flag for 5.003 unless it is very easy.

Run time flag/operator shouldn't be too hard. It is the handling of possible
transitions to the hash's immediate context that may be a bit more
difficult. I'm inclined to keep it so that it's all keys or nothing,
determined by the runtime context where the hash is "born", for now at
least.


- Sarathy.
gsar@engin.umich.edu
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995 14:34:03 EST, Ilya Zakharevich wrote:
>
>OK, this should not leak now:
>
>$c = 3; $xxx='a'; do {print qq{Found $xxx} if defined $::{$xxx}} while $xxx++

No leak.

>> >c) Leaks with your patch;
>>
>> Like hell.
>>
>> >d) May be made non-leaking with your patch.
>>
>> Hell, no. :-)
>
>I think this makes your patch into a grave :-(. Either have keys
>refcounted, or have a global cache with the strings coming from
>constant items, keeping the others as they are now.

I think I'll opt for keeping stuff properly refcounted. Distinguishing
between compile time constants and run-time strings is not worth the
bother (it seems), and more importantly, does not bear greater rewards.

>The latter will lead to two lookups for keys which did not appear in
>constant items, but the overhead will be neglegeable in the current
>perl, when a lot of optimizations are not applied yet.
>
>Maybe one may trigger writing a key into "eternal" hash using
>"compiling" flag.

I can test various scenarios once I spend some time putting in a global
flag rather than the #ifdef.

- Sarathy.
gsar@engin.umich.edu
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995 14:40:27 EST, John Macdonald wrote:
>I don't think that it is necessary to be able to specify that a
>particular hash doesn't get its keys shared. The important item
>is doing the refcounting on those keys that are kept in the
>shared area. If the program doesn't carefully delete obsolete
>elements from its hashes, then it will run out of memory
>regardles of whether that hash is getting its keys shared (it
>just might affect *when* it runs out of memory in a significant
>way - taking a day instead of a month might be noticeable :-).

You seem to be saying "always refcount no matter what". Having
the flexibility to not do it in some cases almost seems like
a vulnerability that the programmer may not want to live with.

>Now, if a particular hash is going to always have essentially
>unique keys, then it would be a bit cheaper to not share its
>keys, I suppose - they get inserted and deleted in their own
>(smaller) has rather than the big shared hash, but that may not
>be a big enough saving to warrant the degree of programmer
>responsibility to explicitly classify hashes.

Making -DREFCNT_STRTAB the default seems the sane way forward.

- Sarathy.
gsar@engin.umich.edu
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
In <199512202107.QAA23231@aatma.engin.umich.edu>
On Wed, 20 Dec 1995 16:07:17 -0500
Gurusamy Sarathy <gsar@engin.umich.edu> writes:
>
>I think I'll opt for keeping stuff properly refcounted. Distinguishing
>between compile time constants and run-time strings is not worth the
>bother (it seems), and more importantly, does not bear greater rewards.
>
Please don't exclude run-time strings from sharing.
Many of Tk's hashed are populated/accessed via

$hash{$key}

type code - but the value of $key will typically be one of the common
(and hence shareable) values.
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Wed, 20 Dec 1995, Gurusamy Sarathy wrote:

> [...]
> This is exactly the case I had in mind when I was thinking about
> *two* string tables, but choosing one or the other for any given hash
> will be a problem.
>
> Oh well, we can probably live with ref-counting always, if you think
> that is serious enough. Just make -DREFCNT_STRTAB the default. It still is
> faster than current perl, but only about 2-3%, I think.

Is ref-counting actually needed, though? Or would it be sufficient to
allow a hash to be aimed towards a different, separate, string bank, so
that bank can be cleaned out afterwards?

> - Sarathy.
> gsar@engin.umich.edu
>

--
Kenneth Albanowski (kjahds@kjahds.com, CIS: 70705,126)
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
Kenneth Albanowski wrote :
|| On Wed, 20 Dec 1995, Gurusamy Sarathy wrote:
||
|| > [...]
|| > This is exactly the case I had in mind when I was thinking about
|| > *two* string tables, but choosing one or the other for any given hash
|| > will be a problem.
|| >
|| > Oh well, we can probably live with ref-counting always, if you think
|| > that is serious enough. Just make -DREFCNT_STRTAB the default. It still is
|| > faster than current perl, but only about 2-3%, I think.
||
|| Is ref-counting actually needed, though? Or would it be sufficient to
|| allow a hash to be aimed towards a different, separate, string bank, so
|| that bank can be cleaned out afterwards?

As long as always doing refcounting is not *too* onerous, there
is an incredible advantage in always using it. This means that
no action is required by the programmer - things just work
better with no change.

(Just as perl does refcounts on scalar values without providing
any way for the programmer to identify values that need not be
refcounted - there is too much opportunity for such a facility
to be used wrong and cause bugs and yet there would be very
little actual saving.)

A big hash is slightly more expensive to use than a small hash,
but not much (it's not linear, but logarithmic). It is quite
possible that a hash that uses unique keys will in many cases
also use some keys that are not unique. The memory saving on
those non-unique keys might still pay for the cost of
refcounting the unique keys. (And it really wins when a later
redesign of the program adds a second hash that uses the same
"unique" keys to store auxillary info.)

For programs which are relatively short-lived and CPU-heavy,
there might be an advantage to turning off refcounting - saving
CPU for such a program is more important than saving memory
which is going to be freed at exit time anyhow.

So, I can see an argument for a global refcount/norefcount run
time flag, but I don't think that there is much point in trying
to refcount *some* keys but not others.

--
Maybe we can fix reality one of these days. | John Macdonald
<- Larry Wall Carl Dichter -> | jmm@Elegant.COM
I'd be happy if we could just index it. |
Re: 6% runtime speedup, ~20% memory gain (typical Tk app) [ In reply to ]
On Fri, 22 Dec 1995 10:24:05 EST, John Macdonald wrote:
>Kenneth Albanowski wrote :
>|| Is ref-counting actually needed, though? Or would it be sufficient to
>|| allow a hash to be aimed towards a different, separate, string bank, so
>|| that bank can be cleaned out afterwards?
>
>As long as always doing refcounting is not *too* onerous, there
>is an incredible advantage in always using it. This means that
>no action is required by the programmer - things just work
>better with no change.
>

The *only* reason refcounting is significantly less efficient now is because
we have char* keys and I took the easy way out to store the refcnt in the
hent_val slot. This means finding a particular char*'s refcnt is a hash
fetch. It needn't neccessarily be that way. For instance, we could keep a
global array of char*'s with the keys and refcnt's alternating in the
array. In short, a more elaborate, efficient structure for the all important
string table than the presently simplistic global hash.

Then again, when we move to SV* based keys, the current overhead will
disappear into insignificance, since the hash fetch will become a simple
member dereference.

This morning, I really don't see why the pragma to turn off refcounting will
be needed when there are avenues to make it as efficient as not.

- Sarathy.
gsar@engin.umich.edu

1 2  View All