[PATCH] PR libstdc++/87135 Rehash only when necessary (LWG2156)

[Jonathan Wakely]

On 18/09/18 22:39 +0200, François Dumont wrote:
>On 09/18/2018 10:41 AM, Jonathan Wakely wrote:
>>On 13/09/18 07:49 +0200, François Dumont wrote:
>>>All changes limited to hashtable_c++0x.cc file.
>>>
>>>_Prime_rehash_policy::_M_next_bkt now really does what its comment 
>>>was declaring that is to say:
>>>Â  // Return a prime no smaller than n.
>>>
>>>_Prime_rehash_policy::_M_need_rehash rehash only when _M_next_size 
>>>is exceeded, not only when it is reach.
>>>
>>>Â Â Â  PR libstdc++/87135
>>>Â Â Â  * src/c++11/hashtable_c++0x.cc:
>>>Â Â Â  (_Prime_rehash_policy::_M_next_bkt): Return a prime no smaller than
>>>Â Â Â  requested size, but not necessarily greater.
>>>Â Â Â  (_Prime_rehash_policy::_M_need_rehash): Rehash only if target 
>>>size is
>>>Â Â Â  strictly greater than next resize threshold.
>>>Â Â Â  * testsuite/23_containers/unordered_map/modifiers/reserve.cc: 
>>>Adapt test
>>>Â Â Â  to validate that there is no rehash as long as number of 
>>>insertion is
>>>Â Â Â  lower or equal to the reserved number of elements.
>>>
>>>unordered_map tests successful, ok to commit once all other tests 
>>>completed ?
>>>
>>>François
>>
>>>diff --git a/libstdc++-v3/src/c++11/hashtable_c++0x.cc 
>>>b/libstdc++-v3/src/c++11/hashtable_c++0x.cc
>>>index a776a8506fe..ec6031b3f5b 100644
>>>--- a/libstdc++-v3/src/c++11/hashtable_c++0x.cc
>>>+++ b/libstdc++-v3/src/c++11/hashtable_c++0x.cc
>>>@@ -46,10 +46,10 @@ namespace __detail
>>>Â  {
>>>Â Â Â  // Optimize lookups involving the first elements of __prime_list.
>>>Â Â Â  // (useful to speed-up, eg, constructors)
>>>-Â Â Â  static const unsigned char __fast_bkt[13]
>>>-Â Â Â Â Â  = { 2, 2, 3, 5, 5, 7, 7, 11, 11, 11, 11, 13, 13 };
>>>+Â Â Â  static const unsigned char __fast_bkt[]
>>>+Â Â Â Â Â  = { 2, 2, 2, 3, 5, 5, 7, 7, 11, 11, 11, 11, 13, 13 };
>>>
>>>-Â Â Â  if (__n <= 12)
>>>+Â Â Â  if (__n < sizeof(__fast_bkt) / sizeof(unsigned char))
>>
>>sizeof(unsigned char) is defined to be 1, always.
>
>I also though it was overkill but there are so many platforms that I 
>prefer to be caution. Good to know that it can't be otherwise.
>
>>
>>I think this should be just sizeof(__fast_bkt), or if you're trying to
>>guard against the type of __fast_bkt changing, then use
>>sizeof(__fast_bkt) / sizeof(__fast_bkt[0]))
>
>I committed with simply sizeof(__fast_bkt)


This caused some testsuite regressions:

FAIL: 23_containers/unordered_set/hash_policy/71181.cc execution test
/home/jwakely/src/gcc/gcc/libstdc++-v3/testsuite/23_containers/unordered_set/hash_policy/71181.cc:42: void test(int) [with _USet = std::unordered_set<int>]: Assertion 'us.bucket_count() == bkts' failed.

FAIL: 23_containers/unordered_set/hash_policy/load_factor.cc execution test
/home/jwakely/src/gcc/gcc/libstdc++-v3/testsuite/23_containers/unordered_set/hash_policy/load_factor.cc:50: void test() [with _USet = std::unordered_set<int>]: Assertion 'us.load_factor() <= us.max_load_factor()' failed.

FAIL: 23_containers/unordered_set/hash_policy/prime_rehash.cc execution test
/home/jwakely/src/gcc/gcc/libstdc++-v3/testsuite/23_containers/unordered_set/hash_policy/prime_rehash.cc:37: void test01(): Assertion 'nxt == policy._M_next_bkt(mx)' failed.