Re: [patch] No allocation for empty unordered containers

2014-09-09 Thread Jonathan Wakely

On 14/08/14 21:22 +0200, François Dumont wrote:
I am preparing a patch for profile mode so I will submit modification 
for this mode with this big patch.


btw, François, for profile mode I think we should just do something
like this patch.

I feel quite strongly that if using Debug Mode or Profile Mode makes
your program run out of memory where it wouldn't usually fail, then
terminating is reasonable. The point of Profile Mode is not to test
abnormal execution of your program because that won't give you useful
profile information for the normal case.

It's more important for the noexcept specification to be consistent
across normal/debug/profile modes than for profile mode to fail
gracefully via bad_alloc in out-of-memory scenarios.

diff --git a/libstdc++-v3/include/profile/unordered_base.h 
b/libstdc++-v3/include/profile/unordered_base.h
index 283f87c..cd9db7e 100644
--- a/libstdc++-v3/include/profile/unordered_base.h
+++ b/libstdc++-v3/include/profile/unordered_base.h
@@ -154,7 +154,7 @@ namespace __profile
   using __unique_keys = std::integral_constantbool, _Unique_keys;
 
 protected:
-  _Unordered_profile()
+  _Unordered_profile() noexcept
   {
auto __uc = _M_conjure();
__profcxx_hashtable_construct(__uc, __uc.bucket_count());
@@ -162,10 +162,10 @@ namespace __profile
   }
   _Unordered_profile(const _Unordered_profile)
: _Unordered_profile() { }
-  _Unordered_profile(_Unordered_profile)
+  _Unordered_profile(_Unordered_profile) noexcept
: _Unordered_profile() { }
 
-  ~_Unordered_profile() noexcept
+  ~_Unordered_profile()
   {
auto __uc = _M_conjure();
__profcxx_hashtable_destruct(__uc, __uc.bucket_count(), __uc.size());


Re: [patch] No allocation for empty unordered containers

2014-09-09 Thread François Dumont

On 09/09/2014 19:29, Jonathan Wakely wrote:

On 14/08/14 21:22 +0200, François Dumont wrote:
I am preparing a patch for profile mode so I will submit modification 
for this mode with this big patch.


btw, François, for profile mode I think we should just do something
like this patch.

I feel quite strongly that if using Debug Mode or Profile Mode makes
your program run out of memory where it wouldn't usually fail, then
terminating is reasonable. The point of Profile Mode is not to test
abnormal execution of your program because that won't give you useful
profile information for the normal case.

It's more important for the noexcept specification to be consistent
across normal/debug/profile modes than for profile mode to fail
gracefully via bad_alloc in out-of-memory scenarios.

Sure, no problem. In the patch I am preparing for profile mode 
failure in allocation will just mean that the involved container won't 
be profiled so that I can add noexcept wherever it is needed for 
consistency with normal mode. I hope to be able to submit this patch in 
a week or two.


François


Re: [patch] No allocation for empty unordered containers

2014-09-09 Thread Jonathan Wakely

On 09/09/14 23:03 +0200, François Dumont wrote:

On 09/09/2014 19:29, Jonathan Wakely wrote:

On 14/08/14 21:22 +0200, François Dumont wrote:
I am preparing a patch for profile mode so I will submit 
modification for this mode with this big patch.


btw, François, for profile mode I think we should just do something
like this patch.

I feel quite strongly that if using Debug Mode or Profile Mode makes
your program run out of memory where it wouldn't usually fail, then
terminating is reasonable. The point of Profile Mode is not to test
abnormal execution of your program because that won't give you useful
profile information for the normal case.

It's more important for the noexcept specification to be consistent
across normal/debug/profile modes than for profile mode to fail
gracefully via bad_alloc in out-of-memory scenarios.

   Sure, no problem. In the patch I am preparing for profile mode 
failure in allocation will just mean that the involved container won't 
be profiled so that I can add noexcept wherever it is needed for 
consistency with normal mode. I hope to be able to submit this patch 
in a week or two.


Great - thanks for working on it.


Re: [patch] No allocation for empty unordered containers

2014-09-05 Thread Jonathan Wakely

On 14/08/14 21:22 +0200, François Dumont wrote:
   Then here is the patch to introduce default constructor with 
compiler computed noexcept qualification. Note that I also made 
allocator aware default constructor allocation free however noexcept 
qualification has to be manually written which I find quite a burden. 
Do you think we shall do so now ?


I don't really care about that constructor, no need to do it now.

The patch is OK to commit, thanks.


Re: [patch] No allocation for empty unordered containers

2014-09-02 Thread Jonathan Wakely

On 30/08/14 20:03 +0200, François Dumont wrote:

Any news for my patch proposals ?

Regarding documentation of default minimum number of buckets, I don't 
know where it has been documented but why do we need to document it 
separately ? Could it be taken care by Doxygen ? Can't it get the 
default value from the code itself ? If not we could document it 
ourself next to the code rather than in a distinct file.


It's OK to document it with a Doxygen comment, although I think it
would be better in doc/xml/manual/containers.xml.

I'm reviewing the rest of the patch today, thanks for you patience.



Re: [patch] No allocation for empty unordered containers

2014-08-30 Thread François Dumont

Any news for my patch proposals ?

Regarding documentation of default minimum number of buckets, I don't 
know where it has been documented but why do we need to document it 
separately ? Could it be taken care by Doxygen ? Can't it get the 
default value from the code itself ? If not we could document it ourself 
next to the code rather than in a distinct file.


François

On 14/08/2014 21:22, François Dumont wrote:

On 13/08/2014 11:50, Jonathan Wakely wrote:


Yes you can, it's conforming to replace a (non-virtual) member function
with default arguments by two or more member functions. We do it all
the time.

See 17.6.5.5 [member.functions] p2.


You should have told it sooner ! But of course no-one is supposed 
to ignore the Standard :-).


Then here is the patch to introduce default constructor with 
compiler computed noexcept qualification. Note that I also made 
allocator aware default constructor allocation free however noexcept 
qualification has to be manually written which I find quite a burden. 
Do you think we shall do so now ?


2014-08-14  François Dumont fdum...@gcc.gnu.org

* include/bits/hashtable_policy.h (_Prime_rehash_policy): Qualify 
constructor

noexcept.
(_Hash_code_base): All specialization default constructible if
possible.
(_Hashtable_base): Likewise.
* include/bits/hashtable.h (_Hashtable()): Implementation 
defaulted.
* include/bits/unordered_map.h (unordered_map::unordered_map()): 
New,

implementation defaulted.
(unordered_multimap::unordered_multimap()): Likewise.
* include/bits/unordered_set.h
(unordered_set::unordered_set()): Likewise.
(unordered_multiset::unordered_multiset()): Likewise.
* include/debug/unordered_map: Likewise.
* include/debug/unordered_set: Likewise.
* testsuite/23_containers/unordered_map/allocator/noexcept.cc
(test04()): New.
* testsuite/23_containers/unordered_multimap/allocator/noexcept.cc
(test04()): New.
* testsuite/23_containers/unordered_set/allocator/noexcept.cc
(test04()): New.
* testsuite/23_containers/unordered_multiset/allocator/noexcept.cc
(test04()): New.

I am preparing a patch for profile mode so I will submit modification 
for this mode with this big patch.


Tested under Linux x86_64.

Ok to commit ?

François




Re: [patch] No allocation for empty unordered containers

2014-08-14 Thread François Dumont

On 13/08/2014 11:50, Jonathan Wakely wrote:


Yes you can, it's conforming to replace a (non-virtual) member function
with default arguments by two or more member functions. We do it all
the time.

See 17.6.5.5 [member.functions] p2.


You should have told it sooner ! But of course no-one is supposed 
to ignore the Standard :-).


Then here is the patch to introduce default constructor with 
compiler computed noexcept qualification. Note that I also made 
allocator aware default constructor allocation free however noexcept 
qualification has to be manually written which I find quite a burden. Do 
you think we shall do so now ?


2014-08-14  François Dumont fdum...@gcc.gnu.org

* include/bits/hashtable_policy.h (_Prime_rehash_policy): Qualify 
constructor

noexcept.
(_Hash_code_base): All specialization default constructible if
possible.
(_Hashtable_base): Likewise.
* include/bits/hashtable.h (_Hashtable()): Implementation defaulted.
* include/bits/unordered_map.h (unordered_map::unordered_map()): New,
implementation defaulted.
(unordered_multimap::unordered_multimap()): Likewise.
* include/bits/unordered_set.h
(unordered_set::unordered_set()): Likewise.
(unordered_multiset::unordered_multiset()): Likewise.
* include/debug/unordered_map: Likewise.
* include/debug/unordered_set: Likewise.
* testsuite/23_containers/unordered_map/allocator/noexcept.cc
(test04()): New.
* testsuite/23_containers/unordered_multimap/allocator/noexcept.cc
(test04()): New.
* testsuite/23_containers/unordered_set/allocator/noexcept.cc
(test04()): New.
* testsuite/23_containers/unordered_multiset/allocator/noexcept.cc
(test04()): New.

I am preparing a patch for profile mode so I will submit modification 
for this mode with this big patch.


Tested under Linux x86_64.

Ok to commit ?

François
Index: include/bits/hashtable_policy.h
===
--- include/bits/hashtable_policy.h	(revision 213780)
+++ include/bits/hashtable_policy.h	(working copy)
@@ -460,7 +460,7 @@
   /// smallest prime that keeps the load factor small enough.
   struct _Prime_rehash_policy
   {
-_Prime_rehash_policy(float __z = 1.0)
+_Prime_rehash_policy(float __z = 1.0) noexcept
 : _M_max_load_factor(__z), _M_next_resize(0) { }
 
 float
@@ -1071,7 +1071,8 @@
   typedef void* 	__hash_code;
   typedef _Hash_node_Value, false			__node_type;
 
-  // We need the default constructor for the local iterators.
+  // We need the default constructor for the local iterators and _Hashtable
+  // default constructor.
   _Hash_code_base() = default;
 
   _Hash_code_base(const _ExtractKey __ex, const _H1, const _H2,
@@ -1161,7 +1162,8 @@
   typedef std::size_t __hash_code;
   typedef _Hash_node_Value, false			__node_type;
 
-  // We need the default constructor for the local iterators.
+  // We need the default constructor for the local iterators and _Hashtable
+  // default constructor.
   _Hash_code_base() = default;
 
   _Hash_code_base(const _ExtractKey __ex,
@@ -1250,6 +1252,8 @@
   typedef std::size_t __hash_code;
   typedef _Hash_node_Value, true			__node_type;
 
+  // We need the default constructor for _Hashtable default constructor.
+  _Hash_code_base() = default;
   _Hash_code_base(const _ExtractKey __ex,
 		  const _H1 __h1, const _H2 __h2,
 		  const _Default_ranged_hash)
@@ -1694,6 +1698,7 @@
 	__hash_code, __hash_cached::value;
 
   protected:
+_Hashtable_base() = default;
 _Hashtable_base(const _ExtractKey __ex, const _H1 __h1, const _H2 __h2,
 		const _Hash __hash, const _Equal __eq)
 : __hash_code_base(__ex, __h1, __h2, __hash), _EqualEBO(__eq)
@@ -1906,6 +1911,7 @@
 	__alloc_rebind__node_alloc_type, __bucket_type;
   using __bucket_alloc_traits = std::allocator_traits__bucket_alloc_type;
 
+  _Hashtable_alloc() = default;
   _Hashtable_alloc(const _Hashtable_alloc) = default;
   _Hashtable_alloc(_Hashtable_alloc) = default;
 
Index: include/bits/hashtable.h
===
--- include/bits/hashtable.h	(revision 213780)
+++ include/bits/hashtable.h	(working copy)
@@ -310,10 +310,10 @@
    const_local_iterator;
 
 private:
-  __bucket_type*		_M_buckets;
-  size_type			_M_bucket_count;
+  __bucket_type*		_M_buckets		= _M_single_bucket;
+  size_type			_M_bucket_count		= 1;
   __node_base		_M_before_begin;
-  size_type			_M_element_count;
+  size_type			_M_element_count	= 0;
   _RehashPolicy		_M_rehash_policy;
 
   // A single bucket used when only need for 1 bucket. Especially
@@ -322,7 +322,7 @@
   // qualified.
   // Note that we can't leave hashtable with 0 bucket without adding
   // numerous checks in the code to avoid 0 modulus.
-  __bucket_type		

Re: [patch] No allocation for empty unordered containers

2014-08-13 Thread Jonathan Wakely

On 12/08/14 21:53 +0200, François Dumont wrote:

On 12/08/2014 21:39, Marc Glisse wrote:

On Tue, 12 Aug 2014, François Dumont wrote:

  Based on your feedbacks I think we should stay with just 
targeting good QoI by not allocating on default construction. As I 
said the noexcept qualification would need to not conform strictly 
to the Standard.


The standard explicitly says that you may add noexcept wherever you 
like. It is constexpr that we can only add in GNU mode.


   That's not what I meant. For unordered containers there is no real 
default constructor. it is in fact a constructor with parameters which 
have all default values. This is why you can only say that it won't 
throw at runtime and so you can't qualify it noexcept.


Yes you can, it's conforming to replace a (non-virtual) member function
with default arguments by two or more member functions. We do it all
the time.

See 17.6.5.5 [member.functions] p2.



Re: [patch] No allocation for empty unordered containers

2014-08-12 Thread François Dumont

On 05/08/2014 22:23, Paolo Carlini wrote:

Hi,

On 08/05/2014 10:10 PM, Jonathan Wakely wrote:

It doesn't have to be noexcept, but IMHO there is no point changing
the containers to avoid allocation unless that allows us to mark it
noexcept.  If it can throw anyway, we might as well allocate the
initial buckets.
I have been following this discussion on and off. In general, I must 
say that AFAIK a default constructor which doesn't allocate memory is 
normally considered superior from the QoI point of view, even if isn't 
noexcept. As probably I have already mentioned, a well known C++ 
library implementer used to repeat it all the time and used also to 
say that our std::list implementation was very good exactly because of 
that feature, *well* before the invention of noexcept. That said, 
marking the constructor noexcept is of course the obvious next step, I 
don't have much to say about the general development plans we have got 
here.


Paolo.


Hi

Based on your feedbacks I think we should stay with just targeting 
good QoI by not allocating on default construction. As I said the 
noexcept qualification would need to not conform strictly to the Standard.


So here is the patch again even if a little bit simplified. 
Regarding the doc I try to put information in Doxygen comments this way 
we do not need to maintain another page for that.


2014-08-12  François Dumont fdum...@gcc.gnu.org

* include/bits/hashtable.h: Make use of the internal single bucket to
limit allocation as long as container remains empty.
* include/bits/unordered_map.h: Set default bucket hint to 0 in
constructors to avoid allocation.
* include/bits/unordered_set.h: Likewise.
* include/debug/unordered_map: Likewise.
* include/debug/unordered_set: Likewise.
* src/c++11/hashtable_c++0x.cc (_Prime_rehash_policy::_M_next_bkt):
Returns 1 for hint 0.
* testsuite/23_containers/unordered_map/allocator/
empty_instantiation.cc:New.
* testsuite/23_containers/unordered_multimap/allocator/
empty_instantiation.cc:New.
* testsuite/23_containers/unordered_set/allocator/
empty_instantiation.cc: New.
* testsuite/23_containers/unordered_multiset/allocator/
empty_instantiation.cc: New.

Tested under Linux x86_64.

François

Index: include/bits/hashtable.h
===
--- include/bits/hashtable.h	(revision 213780)
+++ include/bits/hashtable.h	(working copy)
@@ -382,6 +382,17 @@
   void
   _M_reset() noexcept;
 
+  _Hashtable(const _H1 __h1, const _H2 __h2, const _Hash __h,
+		 const _Equal __eq, const _ExtractKey __exk,
+		 const allocator_type __a)
+	: __hashtable_base(__exk, __h1, __h2, __h, __eq),
+	  __hashtable_alloc(__node_alloc_type(__a)),
+	  _M_buckets(_M_single_bucket),
+	  _M_bucket_count(1),
+	  _M_element_count(0),
+	  _M_single_bucket(nullptr)
+  { }
+
 public:
   // Constructor, destructor, assignment, swap
   _Hashtable(size_type __bucket_hint,
@@ -407,12 +418,12 @@
   // Use delegating constructors.
   explicit
   _Hashtable(const allocator_type __a)
-  : _Hashtable(10, _H1(), _H2(), _Hash(), key_equal(),
+  : _Hashtable(_H1(), _H2(), _Hash(), key_equal(),
 		   __key_extract(), __a)
   { }
 
   explicit
-  _Hashtable(size_type __n = 10,
+  _Hashtable(size_type __n = 0,
 		 const _H1 __hf = _H1(),
 		 const key_equal __eql = key_equal(),
 		 const allocator_type __a = allocator_type())
@@ -791,15 +802,14 @@
 	   const _H1 __h1, const _H2 __h2, const _Hash __h,
 	   const _Equal __eq, const _ExtractKey __exk,
 	   const allocator_type __a)
-: __hashtable_base(__exk, __h1, __h2, __h, __eq),
-  __map_base(),
-  __rehash_base(),
-  __hashtable_alloc(__node_alloc_type(__a)),
-  _M_element_count(0),
-  _M_rehash_policy()
+  : _Hashtable(__h1, __h2, __h, __eq, __exk, __a)
 {
-  _M_bucket_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
-  _M_buckets = _M_allocate_buckets(_M_bucket_count);
+  auto __bkt = _M_rehash_policy._M_next_bkt(__bucket_hint);
+  if (__bkt  _M_bucket_count)
+	{
+	  _M_buckets = _M_allocate_buckets(__bkt);
+	  _M_bucket_count = __bkt;
+	}
 }
 
   templatetypename _Key, typename _Value,
@@ -814,20 +824,20 @@
 		 const _H1 __h1, const _H2 __h2, const _Hash __h,
 		 const _Equal __eq, const _ExtractKey __exk,
 		 const allocator_type __a)
-  : __hashtable_base(__exk, __h1, __h2, __h, __eq),
-	__map_base(),
-	__rehash_base(),
-	__hashtable_alloc(__node_alloc_type(__a)),
-	_M_element_count(0),
-	_M_rehash_policy()
+	: _Hashtable(__h1, __h2, __h, __eq, __exk, __a)
   {
 	auto __nb_elems = __detail::__distance_fw(__f, __l);
-	_M_bucket_count =
+	auto __bkt_count =
 	  _M_rehash_policy._M_next_bkt(
 	std::max(_M_rehash_policy._M_bkt_for_elements(__nb_elems),
 		 __bucket_hint));
 
-	_M_buckets = _M_allocate_buckets(_M_bucket_count);
+	if 

Re: [patch] No allocation for empty unordered containers

2014-08-12 Thread Marc Glisse

On Tue, 12 Aug 2014, François Dumont wrote:

   Based on your feedbacks I think we should stay with just targeting good 
QoI by not allocating on default construction. As I said the noexcept 
qualification would need to not conform strictly to the Standard.


The standard explicitly says that you may add noexcept wherever you like. 
It is constexpr that we can only add in GNU mode.


--
Marc Glisse


Re: [patch] No allocation for empty unordered containers

2014-08-12 Thread François Dumont

On 12/08/2014 21:39, Marc Glisse wrote:

On Tue, 12 Aug 2014, François Dumont wrote:

   Based on your feedbacks I think we should stay with just targeting 
good QoI by not allocating on default construction. As I said the 
noexcept qualification would need to not conform strictly to the 
Standard.


The standard explicitly says that you may add noexcept wherever you 
like. It is constexpr that we can only add in GNU mode.


That's not what I meant. For unordered containers there is no real 
default constructor. it is in fact a constructor with parameters which 
have all default values. This is why you can only say that it won't 
throw at runtime and so you can't qualify it noexcept.


François


Re: [patch] No allocation for empty unordered containers

2014-08-04 Thread Jonathan Wakely

On 25/07/14 22:53 +0200, François Dumont wrote:

Hi

   I think I never get feedback regarding this patch proposal. Note 


I've been trying to weigh up the pros and cons and am unsure what's
best, but I think my preference is to have a noexcept default
constructor. Unless you hear any objections in the next 48 hours
please go ahead and commit this to trunk, thanks

that if accepted the doc will have to be updated regarding the default 
hint value.


Good point.


Thanks


On 03/06/2014 22:44, François Dumont wrote:

Hi

   Thanks to the single bucket introduced to make move semantic 
noexcept we can also avoid some over allocations. Here is a patch to 
avoid any allocation on default instantiation, on range constructor 
when range is empty and on construction from an initialization list 
when this list is empty too. I had to make all default hint value to 
0 so that if this value is used the rehash policy next bucket 
returns 1 bucket.


   I don't know if you had in mind to noexcept qualify the default 
constructor but it would mean to have a real default constructor and 
another to deal with the hint which wouldn't match the Standard so 
no noexcept qualification at the moment.


Tested under Linux x86_64.normal debug and profile modes.


2014-06-03  François Dumont fdum...@gcc.gnu.org

   * include/bits/hashtable.h: Make use of the internal single bucket to
   limit allocation as long as container remains empty.
   * include/bits/unordered_map.h: Set default bucket hint to 0 in
   constructors to avoid allocation.
   * include/bits/unordered_set.h: Likewise.
   * include/debug/unordered_map: Likewise.
   * include/debug/unordered_set: Likewise.
   * include/profile/unordered_map: Likewise.
   * include/profile/unordered_set: Likewise.
   * src/c++11/hashtable_c++0x.cc (_Prime_rehash_policy::_M_next_bkt):
   Returns 1 for hint 0.
   * testsuite/23_containers/unordered_map/allocator/
   empty_instantiation.cc:New.
   * testsuite/23_containers/unordered_multimap/allocator/
   empty_instantiation.cc:New.
   * testsuite/23_containers/unordered_set/allocator/
   empty_instantiation.cc: New.
   * testsuite/23_containers/unordered_multiset/allocator/
   empty_instantiation.cc: New.

Ok to commit ?

François







Index: include/bits/hashtable.h
===
--- include/bits/hashtable.h(revision 211144)
+++ include/bits/hashtable.h(working copy)
@@ -407,12 +407,12 @@
  // Use delegating constructors.
  explicit
  _Hashtable(const allocator_type __a)
-  : _Hashtable(10, _H1(), _H2(), _Hash(), key_equal(),
+  : _Hashtable(0, _H1(), _H2(), _Hash(), key_equal(),
   __key_extract(), __a)
  { }

  explicit
-  _Hashtable(size_type __n = 10,
+  _Hashtable(size_type __n = 0,
 const _H1 __hf = _H1(),
 const key_equal __eql = key_equal(),
 const allocator_type __a = allocator_type())
@@ -792,14 +792,18 @@
   const _Equal __eq, const _ExtractKey __exk,
   const allocator_type __a)
: __hashtable_base(__exk, __h1, __h2, __h, __eq),
-  __map_base(),
-  __rehash_base(),
  __hashtable_alloc(__node_alloc_type(__a)),
+  _M_buckets(_M_single_bucket),
+  _M_bucket_count(1),
  _M_element_count(0),
-  _M_rehash_policy()
+  _M_single_bucket(nullptr)
{
-  _M_bucket_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
-  _M_buckets = _M_allocate_buckets(_M_bucket_count);
+  auto __bkt_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
+  if (_M_bucket_count != __bkt_count)
+   {
+ _M_bucket_count = __bkt_count;
+ _M_buckets = _M_allocate_buckets(_M_bucket_count);
+   }
}

  templatetypename _Key, typename _Value,
@@ -815,19 +819,24 @@
 const _Equal __eq, const _ExtractKey __exk,
 const allocator_type __a)
  : __hashtable_base(__exk, __h1, __h2, __h, __eq),
-   __map_base(),
-   __rehash_base(),
__hashtable_alloc(__node_alloc_type(__a)),
+   _M_buckets(_M_single_bucket),
+   _M_bucket_count(1),
_M_element_count(0),
-   _M_rehash_policy()
+   _M_single_bucket(nullptr)
  {
auto __nb_elems = __detail::__distance_fw(__f, __l);
-   _M_bucket_count =
+   auto __bkt_count =
  _M_rehash_policy._M_next_bkt(
std::max(_M_rehash_policy._M_bkt_for_elements(__nb_elems),
 __bucket_hint));
+ 
+	if (_M_bucket_count != __bkt_count)

+ {
+   _M_bucket_count = __bkt_count;
+   _M_buckets = _M_allocate_buckets(_M_bucket_count);
+ }

-   _M_buckets = _M_allocate_buckets(_M_bucket_count);
__try
  {
for (; __f != __l; ++__f)
@@ -864,14 +873,15 @@
  {
// Replacement allocator cannot free existing storage.
this-_M_deallocate_nodes(_M_begin());
- 

Re: [patch] No allocation for empty unordered containers

2014-08-04 Thread Jonathan Wakely

On 04/08/14 12:49 +0100, Jonathan Wakely wrote:

On 25/07/14 22:53 +0200, François Dumont wrote:

Hi

  I think I never get feedback regarding this patch proposal. Note


I've been trying to weigh up the pros and cons and am unsure what's
best, but I think my preference is to have a noexcept default
constructor. Unless you hear any objections in the next 48 hours
please go ahead and commit this to trunk, thanks


I hit send too soon, I meant to say that I think this change is also
needed:

  I don't know if you had in mind to noexcept qualify the default 
constructor but it would mean to have a real default constructor 
and another to deal with the hint which wouldn't match the 
Standard so no noexcept qualification at the moment.


If we don't make it noexcept then I see no point in avoiding
allocation.

(And if the functors and allocator can throw then the noexcept might
have to be condition.)

So please make sure we get that change as well during stage 1.



Re: [patch] No allocation for empty unordered containers

2014-07-25 Thread François Dumont

Hi

I think I never get feedback regarding this patch proposal. Note 
that if accepted the doc will have to be updated regarding the default 
hint value.


Thanks


On 03/06/2014 22:44, François Dumont wrote:

Hi

Thanks to the single bucket introduced to make move semantic 
noexcept we can also avoid some over allocations. Here is a patch to 
avoid any allocation on default instantiation, on range constructor 
when range is empty and on construction from an initialization list 
when this list is empty too. I had to make all default hint value to 0 
so that if this value is used the rehash policy next bucket returns 1 
bucket.


I don't know if you had in mind to noexcept qualify the default 
constructor but it would mean to have a real default constructor and 
another to deal with the hint which wouldn't match the Standard so no 
noexcept qualification at the moment.


Tested under Linux x86_64.normal debug and profile modes.


2014-06-03  François Dumont fdum...@gcc.gnu.org

* include/bits/hashtable.h: Make use of the internal single bucket to
limit allocation as long as container remains empty.
* include/bits/unordered_map.h: Set default bucket hint to 0 in
constructors to avoid allocation.
* include/bits/unordered_set.h: Likewise.
* include/debug/unordered_map: Likewise.
* include/debug/unordered_set: Likewise.
* include/profile/unordered_map: Likewise.
* include/profile/unordered_set: Likewise.
* src/c++11/hashtable_c++0x.cc (_Prime_rehash_policy::_M_next_bkt):
Returns 1 for hint 0.
* testsuite/23_containers/unordered_map/allocator/
empty_instantiation.cc:New.
* testsuite/23_containers/unordered_multimap/allocator/
empty_instantiation.cc:New.
* testsuite/23_containers/unordered_set/allocator/
empty_instantiation.cc: New.
* testsuite/23_containers/unordered_multiset/allocator/
empty_instantiation.cc: New.

Ok to commit ?

François




Index: include/bits/hashtable.h
===
--- include/bits/hashtable.h	(revision 211144)
+++ include/bits/hashtable.h	(working copy)
@@ -407,12 +407,12 @@
   // Use delegating constructors.
   explicit
   _Hashtable(const allocator_type __a)
-  : _Hashtable(10, _H1(), _H2(), _Hash(), key_equal(),
+  : _Hashtable(0, _H1(), _H2(), _Hash(), key_equal(),
 		   __key_extract(), __a)
   { }
 
   explicit
-  _Hashtable(size_type __n = 10,
+  _Hashtable(size_type __n = 0,
 		 const _H1 __hf = _H1(),
 		 const key_equal __eql = key_equal(),
 		 const allocator_type __a = allocator_type())
@@ -792,14 +792,18 @@
 	   const _Equal __eq, const _ExtractKey __exk,
 	   const allocator_type __a)
 : __hashtable_base(__exk, __h1, __h2, __h, __eq),
-  __map_base(),
-  __rehash_base(),
   __hashtable_alloc(__node_alloc_type(__a)),
+  _M_buckets(_M_single_bucket),
+  _M_bucket_count(1),
   _M_element_count(0),
-  _M_rehash_policy()
+  _M_single_bucket(nullptr)
 {
-  _M_bucket_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
-  _M_buckets = _M_allocate_buckets(_M_bucket_count);
+  auto __bkt_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
+  if (_M_bucket_count != __bkt_count)
+	{
+	  _M_bucket_count = __bkt_count;
+	  _M_buckets = _M_allocate_buckets(_M_bucket_count);
+	}
 }
 
   templatetypename _Key, typename _Value,
@@ -815,19 +819,24 @@
 		 const _Equal __eq, const _ExtractKey __exk,
 		 const allocator_type __a)
   : __hashtable_base(__exk, __h1, __h2, __h, __eq),
-	__map_base(),
-	__rehash_base(),
 	__hashtable_alloc(__node_alloc_type(__a)),
+	_M_buckets(_M_single_bucket),
+	_M_bucket_count(1),
 	_M_element_count(0),
-	_M_rehash_policy()
+	_M_single_bucket(nullptr)
   {
 	auto __nb_elems = __detail::__distance_fw(__f, __l);
-	_M_bucket_count =
+	auto __bkt_count =
 	  _M_rehash_policy._M_next_bkt(
 	std::max(_M_rehash_policy._M_bkt_for_elements(__nb_elems),
 		 __bucket_hint));
+ 
+	if (_M_bucket_count != __bkt_count)
+	  {
+	_M_bucket_count = __bkt_count;
+	_M_buckets = _M_allocate_buckets(_M_bucket_count);
+	  }
 
-	_M_buckets = _M_allocate_buckets(_M_bucket_count);
 	__try
 	  {
 	for (; __f != __l; ++__f)
@@ -864,14 +873,15 @@
 	  {
 		// Replacement allocator cannot free existing storage.
 		this-_M_deallocate_nodes(_M_begin());
-		_M_before_begin._M_nxt = nullptr;
 		_M_deallocate_buckets();
-		_M_buckets = nullptr;
+		__hashtable_base::operator=(__ht);
 		std::__alloc_on_copy(__this_alloc, __that_alloc);
-		__hashtable_base::operator=(__ht);
-		_M_bucket_count = __ht._M_bucket_count;
+		_M_buckets = _M_single_bucket;
+		_M_bucket_count = 1;
+		_M_before_begin._M_nxt = nullptr;
 		_M_element_count = __ht._M_element_count;
 		_M_rehash_policy = __ht._M_rehash_policy;
+		_M_single_bucket = nullptr;
 		__try
 		  {
 		_M_assign(__ht,
@@ -946,8 +956,14 @@
   _M_assign(const 

[patch] No allocation for empty unordered containers

2014-06-03 Thread François Dumont

Hi

Thanks to the single bucket introduced to make move semantic 
noexcept we can also avoid some over allocations. Here is a patch to 
avoid any allocation on default instantiation, on range constructor when 
range is empty and on construction from an initialization list when this 
list is empty too. I had to make all default hint value to 0 so that if 
this value is used the rehash policy next bucket returns 1 bucket.


I don't know if you had in mind to noexcept qualify the default 
constructor but it would mean to have a real default constructor and 
another to deal with the hint which wouldn't match the Standard so no 
noexcept qualification at the moment.


Tested under Linux x86_64.normal debug and profile modes.


2014-06-03  François Dumont fdum...@gcc.gnu.org

* include/bits/hashtable.h: Make use of the internal single bucket to
limit allocation as long as container remains empty.
* include/bits/unordered_map.h: Set default bucket hint to 0 in
constructors to avoid allocation.
* include/bits/unordered_set.h: Likewise.
* include/debug/unordered_map: Likewise.
* include/debug/unordered_set: Likewise.
* include/profile/unordered_map: Likewise.
* include/profile/unordered_set: Likewise.
* src/c++11/hashtable_c++0x.cc (_Prime_rehash_policy::_M_next_bkt):
Returns 1 for hint 0.
* testsuite/23_containers/unordered_map/allocator/
empty_instantiation.cc:New.
* testsuite/23_containers/unordered_multimap/allocator/
empty_instantiation.cc:New.
* testsuite/23_containers/unordered_set/allocator/
empty_instantiation.cc: New.
* testsuite/23_containers/unordered_multiset/allocator/
empty_instantiation.cc: New.

Ok to commit ?

François


Index: include/bits/hashtable.h
===
--- include/bits/hashtable.h	(revision 211144)
+++ include/bits/hashtable.h	(working copy)
@@ -407,12 +407,12 @@
   // Use delegating constructors.
   explicit
   _Hashtable(const allocator_type __a)
-  : _Hashtable(10, _H1(), _H2(), _Hash(), key_equal(),
+  : _Hashtable(0, _H1(), _H2(), _Hash(), key_equal(),
 		   __key_extract(), __a)
   { }
 
   explicit
-  _Hashtable(size_type __n = 10,
+  _Hashtable(size_type __n = 0,
 		 const _H1 __hf = _H1(),
 		 const key_equal __eql = key_equal(),
 		 const allocator_type __a = allocator_type())
@@ -792,14 +792,18 @@
 	   const _Equal __eq, const _ExtractKey __exk,
 	   const allocator_type __a)
 : __hashtable_base(__exk, __h1, __h2, __h, __eq),
-  __map_base(),
-  __rehash_base(),
   __hashtable_alloc(__node_alloc_type(__a)),
+  _M_buckets(_M_single_bucket),
+  _M_bucket_count(1),
   _M_element_count(0),
-  _M_rehash_policy()
+  _M_single_bucket(nullptr)
 {
-  _M_bucket_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
-  _M_buckets = _M_allocate_buckets(_M_bucket_count);
+  auto __bkt_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
+  if (_M_bucket_count != __bkt_count)
+	{
+	  _M_bucket_count = __bkt_count;
+	  _M_buckets = _M_allocate_buckets(_M_bucket_count);
+	}
 }
 
   templatetypename _Key, typename _Value,
@@ -815,19 +819,24 @@
 		 const _Equal __eq, const _ExtractKey __exk,
 		 const allocator_type __a)
   : __hashtable_base(__exk, __h1, __h2, __h, __eq),
-	__map_base(),
-	__rehash_base(),
 	__hashtable_alloc(__node_alloc_type(__a)),
+	_M_buckets(_M_single_bucket),
+	_M_bucket_count(1),
 	_M_element_count(0),
-	_M_rehash_policy()
+	_M_single_bucket(nullptr)
   {
 	auto __nb_elems = __detail::__distance_fw(__f, __l);
-	_M_bucket_count =
+	auto __bkt_count =
 	  _M_rehash_policy._M_next_bkt(
 	std::max(_M_rehash_policy._M_bkt_for_elements(__nb_elems),
 		 __bucket_hint));
+ 
+	if (_M_bucket_count != __bkt_count)
+	  {
+	_M_bucket_count = __bkt_count;
+	_M_buckets = _M_allocate_buckets(_M_bucket_count);
+	  }
 
-	_M_buckets = _M_allocate_buckets(_M_bucket_count);
 	__try
 	  {
 	for (; __f != __l; ++__f)
@@ -864,14 +873,15 @@
 	  {
 		// Replacement allocator cannot free existing storage.
 		this-_M_deallocate_nodes(_M_begin());
-		_M_before_begin._M_nxt = nullptr;
 		_M_deallocate_buckets();
-		_M_buckets = nullptr;
+		__hashtable_base::operator=(__ht);
 		std::__alloc_on_copy(__this_alloc, __that_alloc);
-		__hashtable_base::operator=(__ht);
-		_M_bucket_count = __ht._M_bucket_count;
+		_M_buckets = _M_single_bucket;
+		_M_bucket_count = 1;
+		_M_before_begin._M_nxt = nullptr;
 		_M_element_count = __ht._M_element_count;
 		_M_rehash_policy = __ht._M_rehash_policy;
+		_M_single_bucket = nullptr;
 		__try
 		  {
 		_M_assign(__ht,
@@ -946,8 +956,14 @@
   _M_assign(const _Hashtable __ht, const _NodeGenerator __node_gen)
   {
 	__bucket_type* __buckets = nullptr;
-	if (!_M_buckets)
-	  _M_buckets = __buckets = _M_allocate_buckets(_M_bucket_count);
+	if (_M_bucket_count !=