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

[François Dumont]

On 09/19/2018 01:07 PM, Jonathan Wakely wrote:
> 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.
>
>
>
I forgot I only run unordered_map tests. I'll run the others this 
evening and will fix those.

François