Re: hash policy patch

[Paolo Carlini <paolo dot carlini at oracle dot com>]

Hi,
>     After some additional time spent on hashtable I prefer to do this 
> new proposal. It fixes
> - Yet some issues with hash policy that was still able to give a 
> bucket count for which load_factor == max_load_factor, Standard says 
> load_factor < max_load_factor. Note that in fact the refactoring to 
> generalize use of _M_next_bkt had indeed a side effect which is that 
> when looking for instance for 11.5 lower_bound was returning 13 but 
> now that it is casted to integer it will return 11. Not only that 
> reason now _M_next_bkt takes an optional __strict bool parameter 
> signaling if the returned value shall be not only not shorter but even 
> larger.
> - In hashtable implementation I removed usages of std::max that was 
> potentially leaving the hashtable in an inconsistent state with a hash 
> policy next resize value not matching the current bucket count. It was 
> not really a bug because the next resize value was updated on the next 
> insertion but at the cost of a useless floating point operation.
> - I deal with allocation failure directly in _M_rehash method to avoid 
> introducing new try/catch blocks. I also reset hash policy next resize 
> value when the container is emptied on a hash functor exception.
> - In __rehash_policy I only commit the new hash policy instance if the 
> rehash operation succeeded. The associated test change_load_factor.cc 
> requires exception support, is there already a way to detect it or I 
> need to add a new dg-require-exceptions dejaGnu macro ?
I think it can wait: I suppose there are *many* existing tests failing 
when exceptions are disabled.
> On a design point of view, with the new interactions introduced 
> between hash policy and hashtable, I wonder if accepting the hash 
> policy as a template parameter is still necessary. We could perhaps 
> simplify the _Hashtable template type unless you prefer to find a new 
> cleaner hash policy contract.
>
> 2011-08-03  François Dumont <francois.cppdevs@free.fr>
>
>         * include/bits/hashtable_policy.h (_Prime_rehash_policy): Reuse
>         _M_next_bkt as much as possible. Fix corner case leading to load
>         factor equals max load factor.
>         * include/bits/hashtable.h: Fix management of hash policy next 
> resize
>         value so that it stays in sync with bucket count.
>         * testsuite/23_containers/unordered_set/cons/range_cons.cc: New.
>         * testsuite/23_containers/unordered_set/hash_policy/
>         change_load_factor.cc, rehash.cc: New.
Before further discussing the details of the patch - for sure we have 
bugs in this area which we have to fix asap, and with something like 
your patch - can you please double check the patch on 32-bit targets? I 
ran the testsuite on x86_64-linux -m32 and got:

FAIL: 23_containers/unordered_set/hash_policy/change_load_factor.cc 
execution test

(it's: terminate called after throwing an instance of 'St9bad_alloc'
  what():  std::bad_alloc)

Thanks,
Paolo.