Re: profile mode fix

2014-01-30 Thread François Dumont

On 01/29/2014 09:18 PM, Jonathan Wakely wrote:

On 29 January 2014 20:06, François Dumont frs.dum...@gmail.com wrote:

 Here is the patch that simply consider 55083 as not supported except in
normal mode. This is a temporary workaround for 4.9 release so I prefer not
to introduce a dg-profile-mode-unsupported or something like that. Those
tests will simply appear as not supported for debug and parallel mode even
if they are, not a big deal, no ?

But with that change we don't find out if those tests regress in debug mode  :-(

I prefer to just add noexcept to the profile mode move constructor,
and if it throws then the program terminates.  If you run out of
memory when using profile mode then terminating seems reasonable to
me;  I don't think people are using profile mode to test how their
programs handle std::bad_alloc.

Put another way, if your program runs out of memory *because* of
profile mode, then the results of the profiling will not give you
useful data about how your program usually behaves. Using profile mode
has altered the behaviour of the program.  So in that situation simply
calling std::terminate() makes sense.


So I let you apply your patch with your comment.

Do you think then that using a special allocator implementation 
with a one element cache to make sure that a std::unordered_map insert 
preceded by an erase won't throw could be interested ? If not I will 
simply remove it from my TODOs list.


François



Re: profile mode fix

2014-01-29 Thread Jonathan Wakely
On 29 January 2014 20:06, François Dumont frs.dum...@gmail.com wrote:
 Here is the patch that simply consider 55083 as not supported except in
 normal mode. This is a temporary workaround for 4.9 release so I prefer not
 to introduce a dg-profile-mode-unsupported or something like that. Those
 tests will simply appear as not supported for debug and parallel mode even
 if they are, not a big deal, no ?

But with that change we don't find out if those tests regress in debug mode  :-(

I prefer to just add noexcept to the profile mode move constructor,
and if it throws then the program terminates.  If you run out of
memory when using profile mode then terminating seems reasonable to
me;  I don't think people are using profile mode to test how their
programs handle std::bad_alloc.

Put another way, if your program runs out of memory *because* of
profile mode, then the results of the profiling will not give you
useful data about how your program usually behaves. Using profile mode
has altered the behaviour of the program.  So in that situation simply
calling std::terminate() makes sense.


Re: profile mode fix

2014-01-27 Thread François Dumont
Indeed, default constructor and copy constructor shall not be noexcept 
qualified.


IMO we should be able to make move constructor noexcept by using a 
special allocator for the underlying unordered_map that would allow to 
replace an entry with an other one without requiring a 
deallocate/allocate. It would be the same kind of allocator keeping a 
released instance in cache that you already talk about to enhance 
std::deque behavior especially when used in a std::queue.


For 4.9 we could consider that this test is not supported in profile 
mode and I will work on it for next version.


François


On 01/26/2014 11:38 AM, Jonathan Wakely wrote:

On 26 January 2014 09:43, François Dumont wrote:

Hi

 This is a patch to fix PR 55033 in profile mode. Like in debug mode it
was missing noexcept qualifier on move constructor.

But don't those functions allocate memory? So they can throw.

I agree we want the move constructor to be noexcept anyway, and maybe
the default constructor, but why would we want to lie about the copy
constructor?

I have this patch in my tree that I'm trying to decide whether it
should be committed, but if we make the change we should have a
comment like this:

--- a/libstdc++-v3/include/profile/unordered_base.h
+++ b/libstdc++-v3/include/profile/unordered_base.h
@@ -160,9 +160,14 @@ namespace __profile
  __profcxx_hashtable_construct(__uc, __uc.bucket_count());
 __profcxx_hashtable_construct2(__uc);
}
+
_Unordered_profile(const _Unordered_profile)
 : _Unordered_profile() { }
-  _Unordered_profile(_Unordered_profile)
+
+  // This might actually throw, but for consistency with normal mode
+  // unordered containers we want the noexcept specification, and will
+  // std::terminate() if an exception is thrown.
+  _Unordered_profile(_Unordered_profile) noexcept
 : _Unordered_profile() { }

~_Unordered_profile() noexcept





profile mode fix

2014-01-26 Thread François Dumont

Hi

This is a patch to fix PR 55033 in profile mode. Like in debug mode 
it was missing noexcept qualifier on move constructor.


2014-01-26  François Dumont  fdum...@gcc.gnu.org

PR libstdc++/55083
* include/profile/unordered_base.h (_Unordered_profile()): Add
noexcept qualifier.
(_Unordered_profile(const _Unordered_profile)): Likewise.
(_Unordered_profile(_Unordered_profile)): Likewise.

Tested under Linux x86_64 profile mode.

With this patch I have no more failure in profile mode.

Ok to commit ?

François
Index: include/profile/unordered_base.h
===
--- include/profile/unordered_base.h	(revision 207074)
+++ include/profile/unordered_base.h	(working copy)
@@ -154,15 +154,15 @@
   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());
 	__profcxx_hashtable_construct2(__uc);
   }
-  _Unordered_profile(const _Unordered_profile)
+  _Unordered_profile(const _Unordered_profile) noexcept
 	: _Unordered_profile() { }
-  _Unordered_profile(_Unordered_profile)
+  _Unordered_profile(_Unordered_profile) noexcept
 	: _Unordered_profile() { }
 
   ~_Unordered_profile() noexcept


Re: profile mode fix

2014-01-26 Thread Jonathan Wakely
On 26 January 2014 09:43, François Dumont wrote:
 Hi

 This is a patch to fix PR 55033 in profile mode. Like in debug mode it
 was missing noexcept qualifier on move constructor.

But don't those functions allocate memory? So they can throw.

I agree we want the move constructor to be noexcept anyway, and maybe
the default constructor, but why would we want to lie about the copy
constructor?

I have this patch in my tree that I'm trying to decide whether it
should be committed, but if we make the change we should have a
comment like this:

--- a/libstdc++-v3/include/profile/unordered_base.h
+++ b/libstdc++-v3/include/profile/unordered_base.h
@@ -160,9 +160,14 @@ namespace __profile
 __profcxx_hashtable_construct(__uc, __uc.bucket_count());
__profcxx_hashtable_construct2(__uc);
   }
+
   _Unordered_profile(const _Unordered_profile)
: _Unordered_profile() { }
-  _Unordered_profile(_Unordered_profile)
+
+  // This might actually throw, but for consistency with normal mode
+  // unordered containers we want the noexcept specification, and will
+  // std::terminate() if an exception is thrown.
+  _Unordered_profile(_Unordered_profile) noexcept
: _Unordered_profile() { }

   ~_Unordered_profile() noexcept


Tree containers profile mode fix

2014-01-15 Thread François Dumont

Hi

Here is a patch to fix profile mode compilation errors. It makes 
tree based containers C++11 allocator aware in profile mode as they are 
in normal mode. I also try to use default implementation as much as 
possible to benefit from the normal mode noexcept qualifications on the 
defaulted functions.


Note that for some containers having no profiling instrumentation I 
try to replace the empty wrapper with template alias like:


template typename _Key, typename _Compare = std::less_Key,
 typename _Alloc = std::allocator_Key
using set = _GLIBCXX_STD_C::set_Key, _Compare, _Alloc;

But some explicit instantiations failed then in the tests. Is it a 
gcc limitation or a Standard requirement to refuse explicit 
instantiation of a template alias ? If It could be interpreted as an 
explicit instantiation of the underlying type it would be really useful.


2014-01-15  François Dumont fdum...@gcc.gnu.org

* include/profile/set.h (set): Implement C++11 allocator-aware
container requirements.
* include/profile/map.h (map): Likewise.
* include/profile/multiset.h (multiset): Likewise.
* include/profile/multimap.h (multimap): Likewise.
* include/profile/set.h
(set::operator=(const set)): Define as default in C++11 mode.
(set::operator=(set)): Likewise.
* include/profile/map.h
(map::operator=(const map)): Likewise.
(map::operator=(map)): Likewise.
* include/profile/multiset.h
(multiset::operator=(const multiset)): Likewise.
(multiset::operator=(multiset)): Likewise.
* include/profile/multimap.h
(multimap::operator=(const multimap)): Likewise.
(multimap::operator=(multimap)): Likewise.
* include/profile/set.h (set::operator=(std::initializer_list)):
Rely on the same operator from normal mode.
* include/profile/map.h (map::operator=(std::initializer_list)):
Likewise.
* include/profile/multiset.h
(multiset::operator=(std::initializer_list)): Likewise.
* include/debug/multimap.h
(multimap::operator=(std::initializer_list)): Likewise.
* include/debug/set.h (set::swap(set)): Add noexcept
specification.
* include/debug/map.h (map::swap(map)): Likewise.
* include/debug/multiset.h (multiset::swap(multiset)): Likewise.
* include/debug/multimap.h (multimap::swap(multimap)): Likewise.

Tested under Linux x86_64 normal and profile modes.

François

Index: include/profile/multiset.h
===
--- include/profile/multiset.h	(revision 206587)
+++ include/profile/multiset.h	(working copy)
@@ -43,6 +43,10 @@
 {
   typedef _GLIBCXX_STD_C::multiset_Key, _Compare, _Allocator _Base;
 
+#if __cplusplus = 201103L
+  typedef __gnu_cxx::__alloc_traits_Allocator _Alloc_traits;
+#endif
+
 public:
   // types:
   typedef _Key key_type;
@@ -79,49 +83,62 @@
 		 const _Allocator __a = _Allocator())
 	: _Base(__first, __last, __comp, __a) { }
 
+#if __cplusplus  201103L
   multiset(const multiset __x)
   : _Base(__x) { }
+#else
+  multiset(const multiset) = default;
+  multiset(multiset) = default;
 
-  multiset(const _Base __x)
-  : _Base(__x) { }
-
-#if __cplusplus = 201103L
-  multiset(multiset __x)
-  noexcept(is_nothrow_copy_constructible_Compare::value)
-  : _Base(std::move(__x))
-  { }
-
   multiset(initializer_listvalue_type __l,
 	   const _Compare __comp = _Compare(),
 	   const allocator_type __a = allocator_type())
   : _Base(__l, __comp, __a) { }
+
+  explicit
+  multiset(const allocator_type __a)
+	: _Base(__a) { }
+
+  multiset(const multiset __x, const allocator_type __a)
+  : _Base(__x, __a) { }
+
+  multiset(multiset __x, const allocator_type __a)
+  noexcept(is_nothrow_copy_constructible_Compare::value
+	_Alloc_traits::_S_always_equal())
+  : _Base(std::move(__x), __a) { }
+
+  multiset(initializer_listvalue_type __l, const allocator_type __a)
+  : _Base(__l, __a) { }
+
+  templatetypename _InputIterator
+multiset(_InputIterator __first, _InputIterator __last,
+	const allocator_type __a)
+	  : _Base(__first, __last, __a) { }
 #endif
 
+  multiset(const _Base __x)
+  : _Base(__x) { }
+
   ~multiset() _GLIBCXX_NOEXCEPT { }
 
+#if __cplusplus  201103L
   multiset
   operator=(const multiset __x)
   {
-	*static_cast_Base*(this) = __x;
+	_M_base() = __x;
 	return *this;
   }
+#else
+  multiset
+  operator=(const multiset) = default;
 
-#if __cplusplus = 201103L
   multiset
-  operator=(multiset __x)
-  {
-	// NB: DR 1204.
-	// NB: DR 675.
-	this-clear();
-	this-swap(__x);
-	return *this;
-  }
+  operator=(multiset) = default;
 
   multiset
   operator=(initializer_listvalue_type __l)
   {
-	this-clear();
-	this-insert(__l);
+	_M_base() = __l;
 	return *this;
   }
 #endif
@@ -272,6 +289,9 @@
 
   void
   

Re: Tree containers profile mode fix

2014-01-15 Thread Jonathan Wakely
On 15 January 2014 16:59, François Dumont wrote:
 Hi

 Here is a patch to fix profile mode compilation errors. It makes tree
 based containers C++11 allocator aware in profile mode as they are in normal
 mode. I also try to use default implementation as much as possible to
 benefit from the normal mode noexcept qualifications on the defaulted
 functions.

 Note that for some containers having no profiling instrumentation I try
 to replace the empty wrapper with template alias like:

 template typename _Key, typename _Compare = std::less_Key,
  typename _Alloc = std::allocator_Key
 using set = _GLIBCXX_STD_C::set_Key, _Compare, _Alloc;

 But some explicit instantiations failed then in the tests. Is it a gcc
 limitation or a Standard requirement to refuse explicit instantiation of a
 template alias ? If It could be interpreted as an explicit instantiation of
 the underlying type it would be really useful.

I'm pretty sure you cannot instantiate an alias template. An alias
template's underlying type might not be a template at all.


 2014-01-15  François Dumont fdum...@gcc.gnu.org

 * include/profile/set.h (set): Implement C++11 allocator-aware
 container requirements.
 * include/profile/map.h (map): Likewise.
 * include/profile/multiset.h (multiset): Likewise.
 * include/profile/multimap.h (multimap): Likewise.
 * include/profile/set.h
 (set::operator=(const set)): Define as default in C++11 mode.
 (set::operator=(set)): Likewise.
 * include/profile/map.h
 (map::operator=(const map)): Likewise.
 (map::operator=(map)): Likewise.
 * include/profile/multiset.h
 (multiset::operator=(const multiset)): Likewise.
 (multiset::operator=(multiset)): Likewise.
 * include/profile/multimap.h
 (multimap::operator=(const multimap)): Likewise.
 (multimap::operator=(multimap)): Likewise.
 * include/profile/set.h (set::operator=(std::initializer_list)):
 Rely on the same operator from normal mode.
 * include/profile/map.h (map::operator=(std::initializer_list)):
 Likewise.
 * include/profile/multiset.h
 (multiset::operator=(std::initializer_list)): Likewise.
 * include/debug/multimap.h
 (multimap::operator=(std::initializer_list)): Likewise.
 * include/debug/set.h (set::swap(set)): Add noexcept
 specification.
 * include/debug/map.h (map::swap(map)): Likewise.
 * include/debug/multiset.h (multiset::swap(multiset)): Likewise.
 * include/debug/multimap.h (multimap::swap(multimap)): Likewise.

The profile mode parts are OK, but the ChangeLog entry seems to have
extra files listed that are not in the patch. I assume the ChangeLog
is wrong?