Re: hash policy patch

[François Dumont <francois dot cppdevs at free dot fr>]

On 07/24/2011 01:31 AM, Paolo Carlini wrote:
> On 07/23/2011 10:31 PM, François Dumont wrote:
> > Hi
> >
> >     While working on DR 41975 I realized a small issue in current 
> > rehash implementation that sometimes lead to load_factor being 
> > greater than max_load_factor. Here is a patch to fix that:
> Ok, good.
>
> I think we could as well have everywhere:
>
>     const unsigned long __p = *std::lower_bound...
>
> and then change the following *__p to __p. Isn't a tad cleaner?
>
> Thanks,
> Paolo.
Attached patch applied, I have integrated your remark Paolo.

2011-07-24  François Dumont <francois.cppdevs@free.fr>

        * include/bits/hashtable_policy.h (_Prime_rehash_policy): Use
        __builtin_floor rather than __builtin_ceil to compute next resize
        value.
        * testsuite/23_containers/unordered_set/hash_policy/load_factor.cc:
        New.

For info, I will submit a proposal for DR 41975 tomorrow or the day after.

Regards
Index: include/bits/hashtable_policy.h
===================================================================
--- include/bits/hashtable_policy.h	(revision 176581)
+++ include/bits/hashtable_policy.h	(working copy)
@@ -427,10 +427,10 @@
   _Prime_rehash_policy::
   _M_next_bkt(std::size_t __n) const
   {
-    const unsigned long* __p = std::lower_bound(__prime_list, __prime_list
+    const unsigned long __p = *std::lower_bound(__prime_list, __prime_list
 						+ _S_n_primes, __n);
     _M_next_resize =
-      static_cast<std::size_t>(__builtin_ceil(*__p * _M_max_load_factor));
+      static_cast<std::size_t>(__builtin_floor(__p * _M_max_load_factor));
     return *__p;
   }
 
@@ -441,10 +441,10 @@
   _M_bkt_for_elements(std::size_t __n) const
   {
     const float __min_bkts = __n / _M_max_load_factor;
-    const unsigned long* __p = std::lower_bound(__prime_list, __prime_list
+    const unsigned long __p = *std::lower_bound(__prime_list, __prime_list
 						+ _S_n_primes, __min_bkts);
     _M_next_resize =
-      static_cast<std::size_t>(__builtin_ceil(*__p * _M_max_load_factor));
+      static_cast<std::size_t>(__builtin_floor(__p * _M_max_load_factor));
     return *__p;
   }
 
@@ -469,17 +469,17 @@
 	if (__min_bkts > __n_bkt)
 	  {
 	    __min_bkts = std::max(__min_bkts, _M_growth_factor * __n_bkt);
-	    const unsigned long* __p =
-	      std::lower_bound(__prime_list, __prime_list + _S_n_primes,
-			       __min_bkts);
+	    const unsigned long __p =
+	      *std::lower_bound(__prime_list, __prime_list + _S_n_primes,
+				__min_bkts);
 	    _M_next_resize = static_cast<std::size_t>
-	      (__builtin_ceil(*__p * _M_max_load_factor));
+	      (__builtin_floor(__p * _M_max_load_factor));
 	    return std::make_pair(true, *__p);
 	  }
 	else
 	  {
 	    _M_next_resize = static_cast<std::size_t>
-	      (__builtin_ceil(__n_bkt * _M_max_load_factor));
+	      (__builtin_floor(__n_bkt * _M_max_load_factor));
 	    return std::make_pair(false, 0);
 	  }
       }
Index: testsuite/23_containers/unordered_set/hash_policy/load_factor.cc
===================================================================
--- testsuite/23_containers/unordered_set/hash_policy/load_factor.cc	(revision 0)
+++ testsuite/23_containers/unordered_set/hash_policy/load_factor.cc	(revision 0)
@@ -0,0 +1,58 @@
+// Copyright (C) 2011 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-options "-std=gnu++0x" }
+
+#include <unordered_set>
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  bool test __attribute__((unused)) = true;
+  {
+    std::unordered_set<int> us;
+    for (int i = 0; i != 100000; ++i)
+    {
+      us.insert(i);
+      VERIFY( us.load_factor() <= us.max_load_factor() );
+    }
+  }
+  {
+    std::unordered_set<int> us;
+    us.max_load_factor(3.f);
+    for (int i = 0; i != 100000; ++i)
+    {
+      us.insert(i);
+      VERIFY( us.load_factor() <= us.max_load_factor() );
+    }
+  }
+  {
+    std::unordered_set<int> us;
+    us.max_load_factor(.3f);
+    for (int i = 0; i != 100000; ++i)
+    {
+      us.insert(i);
+      VERIFY( us.load_factor() <= us.max_load_factor() );
+    }
+  }
+}
+
+int main()
+{
+  test01();
+  return 0;
+}