diff mbox

profile mode fix

Message ID 52E95F57.5090500@gmail.com
State New
Headers show

Commit Message

François Dumont Jan. 29, 2014, 8:06 p.m. UTC
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 ?

Tested under Linux x86_64 normal and profile mode.

2014-01-29  François Dumont <fdumont@gcc.gnu.org>

     PR libstdc++/55083
     * testsuite/23_containers/unordered_multimap/55043.cc: Add
     dg-require-normal-mode because not supported in profile mode.
     * testsuite/23_containers/unordered_set/55043.cc: Likewise.
     * testsuite/23_containers/unordered_multiset/55043.cc: Likewise.
     * testsuite/23_containers/unordered_map/55043.cc: Likewise.

Ok to commit ?

François


On 01/27/2014 11:18 PM, François Dumont wrote:
> 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
>
>

Comments

Jonathan Wakely Jan. 29, 2014, 8:18 p.m. UTC | #1
On 29 January 2014 20:06, François Dumont <frs.dumont@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.
François Dumont Jan. 30, 2014, 9:05 p.m. UTC | #2
On 01/29/2014 09:18 PM, Jonathan Wakely wrote:
> On 29 January 2014 20:06, François Dumont <frs.dumont@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
diff mbox

Patch

Index: testsuite/23_containers/unordered_multimap/55043.cc
===================================================================
--- testsuite/23_containers/unordered_multimap/55043.cc	(revision 207207)
+++ testsuite/23_containers/unordered_multimap/55043.cc	(working copy)
@@ -1,5 +1,6 @@ 
 // { dg-options "-std=gnu++0x" }
 // { dg-do compile }
+// { dg-require-normal-mode "" }
 
 // Copyright (C) 2013-2014 Free Software Foundation, Inc.
 //
Index: testsuite/23_containers/unordered_set/55043.cc
===================================================================
--- testsuite/23_containers/unordered_set/55043.cc	(revision 207207)
+++ testsuite/23_containers/unordered_set/55043.cc	(working copy)
@@ -1,5 +1,6 @@ 
 // { dg-options "-std=gnu++0x" }
 // { dg-do compile }
+// { dg-require-normal-mode "" }
 
 // Copyright (C) 2013-2014 Free Software Foundation, Inc.
 //
Index: testsuite/23_containers/unordered_multiset/55043.cc
===================================================================
--- testsuite/23_containers/unordered_multiset/55043.cc	(revision 207207)
+++ testsuite/23_containers/unordered_multiset/55043.cc	(working copy)
@@ -1,5 +1,6 @@ 
 // { dg-options "-std=gnu++0x" }
 // { dg-do compile }
+// { dg-require-normal-mode "" }
 
 // Copyright (C) 2013-2014 Free Software Foundation, Inc.
 //
Index: testsuite/23_containers/unordered_map/55043.cc
===================================================================
--- testsuite/23_containers/unordered_map/55043.cc	(revision 207207)
+++ testsuite/23_containers/unordered_map/55043.cc	(working copy)
@@ -1,5 +1,6 @@ 
 // { dg-options "-std=gnu++0x" }
 // { dg-do compile }
+// { dg-require-normal-mode "" }
 
 // Copyright (C) 2013-2014 Free Software Foundation, Inc.
 //