From f6bb1c88770cae114ab47307e530a957e9e385a7 Mon Sep 17 00:00:00 2001 From: Yves Orton Date: Wed, 27 Mar 2013 13:20:08 +0100 Subject: eliminate the only internal uses of HvFILL The usages are as far as I know incorrect anyway. We resize the hash bucket array based on the number of keys it holds, not based on the number of buckets that are used, so this usage was wrong anyway. Another bug that this revealed is that the old code would allow HvMAX(hv) to fall to 0, even though every other part of the core expects it to have a minimum of 7 (meaning 8 buckets). As part of this we change the hard coded 7 to a defined constant PERL_HASH_DEFAULT_HvMAX. After this patch there remains one use of HvFILL in core, that used for scalar(%hash) which I plan to remove in a later patch. --- hv.c | 31 +++++++++++++++++++++---------- hv.h | 2 ++ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/hv.c b/hv.c index 7d69fe432d..ec1bfe866e 100644 --- a/hv.c +++ b/hv.c @@ -1236,6 +1236,21 @@ Perl_hv_ksplit(pTHX_ HV *hv, IV newmax) } } +/* IMO this should also handle cases where hv_max is smaller than hv_keys + * as tied hashes could play silly buggers and mess us around. We will + * do the right thing during hv_store() afterwards, but still - Yves */ +#define HV_SET_MAX_ADJUSTED_FOR_KEYS(hv,hv_max,hv_keys) STMT_START {\ + /* Can we use fewer buckets? (hv_max is always 2^n-1) */ \ + if (hv_max < PERL_HASH_DEFAULT_HvMAX) { \ + hv_max = PERL_HASH_DEFAULT_HvMAX; \ + } else { \ + while (hv_max > PERL_HASH_DEFAULT_HvMAX && hv_max + 1 >= hv_keys * 2) \ + hv_max = hv_max / 2; \ + } \ + HvMAX(hv) = hv_max; \ +} STMT_END + + HV * Perl_newHVhv(pTHX_ HV *ohv) { @@ -1297,12 +1312,9 @@ Perl_newHVhv(pTHX_ HV *ohv) HE *entry; const I32 riter = HvRITER_get(ohv); HE * const eiter = HvEITER_get(ohv); - STRLEN hv_fill = HvFILL(ohv); + STRLEN hv_keys = HvTOTALKEYS(ohv); - /* Can we use fewer buckets? (hv_max is always 2^n-1) */ - while (hv_max && hv_max + 1 >= hv_fill * 2) - hv_max = hv_max / 2; - HvMAX(hv) = hv_max; + HV_SET_MAX_ADJUSTED_FOR_KEYS(hv,hv_max,hv_keys); hv_iterinit(ohv); while ((entry = hv_iternext_flags(ohv, 0))) { @@ -1341,7 +1353,7 @@ Perl_hv_copy_hints_hv(pTHX_ HV *const ohv) if (ohv) { STRLEN hv_max = HvMAX(ohv); - STRLEN hv_fill = HvFILL(ohv); + STRLEN hv_keys = HvTOTALKEYS(ohv); HE *entry; const I32 riter = HvRITER_get(ohv); HE * const eiter = HvEITER_get(ohv); @@ -1349,9 +1361,7 @@ Perl_hv_copy_hints_hv(pTHX_ HV *const ohv) ENTER; SAVEFREESV(hv); - while (hv_max && hv_max + 1 >= hv_fill * 2) - hv_max = hv_max / 2; - HvMAX(hv) = hv_max; + HV_SET_MAX_ADJUSTED_FOR_KEYS(hv,hv_max,hv_keys); hv_iterinit(ohv); while ((entry = hv_iternext_flags(ohv, 0))) { @@ -1377,6 +1387,7 @@ Perl_hv_copy_hints_hv(pTHX_ HV *const ohv) hv_magic(hv, NULL, PERL_MAGIC_hints); return hv; } +#undef HV_SET_MAX_ADJUSTED_FOR_KEYS /* like hv_free_ent, but returns the SV rather than freeing it */ STATIC SV* @@ -1760,7 +1771,7 @@ Perl_hv_undef_flags(pTHX_ HV *hv, U32 flags) } if (!SvOOK(hv)) { Safefree(HvARRAY(hv)); - xhv->xhv_max = 7; /* HvMAX(hv) = 7 (it's a normal hash) */ + xhv->xhv_max = PERL_HASH_DEFAULT_HvMAX; /* HvMAX(hv) = 7 (it's a normal hash) */ HvARRAY(hv) = 0; } /* if we're freeing the HV, the SvMAGIC field has been reused for diff --git a/hv.h b/hv.h index 61bc5bd54b..270ae00157 100644 --- a/hv.h +++ b/hv.h @@ -208,6 +208,8 @@ C. =cut */ +#define PERL_HASH_DEFAULT_HvMAX 7 + /* these hash entry flags ride on hent_klen (for use only in magic/tied HVs) */ #define HEf_SVKEY -2 /* hent_key is an SV* */ -- cgit v1.2.1