This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] debug mode

Hello

Here is my new patch proposition:

- I have tested it on x86_64 linux with make check. make debug-check gives me the same failures with or without the patch, the failures looks normal ones to me. For instance the synopsis.cc tests are all failing because of ambiguity between std::vector and std::__debug::vector, other tests are failing because of tests involving the profile mode.

- 80 columns

Sorry about that I looked for the number of chars and not the number of columns, because of tabulations it is not equal. By the way, are tabulations ok ? Most of projects I work on was forbidding tabs, should I consider that tabs are 8 spaces long ? To fix this problem I replace many __first, __last by respectively __f, __l, I hope it is fine.

- Performance gain

This patch contains 4 new performance tests to challenge range constructors of different containers instanciated with int type:
deque_construct: deque instances built from a vector [begin, end[ range
list_construct1: list instances built from a vector [begin, end[ range
list_construct2: list instances built from a list [begin, end[ range
vector_construct: vector instances built from a vector [begin, end[ range

When run in release mode it gives the following results:
deque_construct.cc 11r 9u 1s 0mem 0pf
list_construct1.cc 343r 341u 0s 0mem 0pf
list_construct2.cc 346r 346u 0s 0mem 0pf
vector_construct.cc 8r 2u 5s 0mem 0pf

deque and vector build are much faster because thanks to internal optimizations it relies on memmove.

Once run with _GLIBCXX_DEBUG defined without the patch it gives:

deque_construct.cc 529r 523u 2s 0mem 0pf
list_construct1.cc 895r 890u 1s 0mem 0pf
list_construct2.cc 884r 879u 1s 0mem 0pf
vector_construct.cc 475r 467u 6s 0mem 0pf

All tests are taking much more time. list_construct1 and list_construct2 because we need more time to check safe iterators on each initialization, deque_construct and vector_construct because usage of safe iterators disable completely usage of memmove and so once again we have to check each safe iterators each time we increment the safe iterator.

With the proposed patch we have:

deque_construct.cc 11r 7u 2s 0mem 0pf
list_construct1.cc 341r 339u 0s 0mem 0pf
list_construct2.cc 865r 861u 1s 0mem 0pf
vector_construct.cc 9r 3u 4s 0mem 0pf

As soon as we detect random access safe iterator we check the whole range in a single operation and there is no more overhead on each iterator incrementation. Moreover for deque construct and vector construct we restore usage of memmove. Only list_construct2 is not impacted because we are not able to validate the iterator range in a single operation so we still have the overhead of checking it each time we increment the safe iterator internally.

- Modifications of functions.h

The only point that I am not fully comfortable with is that I had to introduce a declaration of __check_singular in formatter.h in order to have functions.h to depend on formatter.h and not the opposite as it used to be when formatter.h was including debug.h. If you do not like it I can introduce a new header to define __check_singular and associated __check_singular_aux that will be overloaded in safe_iterator.h. Except that I think modifications are correct and as a matter of fact the test suite did not show any problem.

Here is the updated change logs:

2010-08-18 François Dumont <francois.cppdevs@free.fr>

* include/bits/stl_algobase.h: _Iter_base struct moved
* include/bits/stl_iterator_base_types.h: here.
* include/debug/functions.h: Fix behavior of __check_valid_range,
__check_string, __check_sorted debug functions that used to depend on
_GLIBCXX_DEBUG macro. Include formatter.h and use formatting macros
for a consistent debug result.
* include/debug/formatter.h: Remove debug.h include, add __check_singular
declaration.
* include/debug/debug.h: Remove formatter.h include that is now
included by functions.h.
* include/debug/safe_iterator.h: _Safe_iterator::_Base_Iterator
renamed in interator_type to make it usable by _Iter_base helper
struct. __gnu_debug::__base introduced.
* include/debug/set.h: Use __gnu_debug::__base once iterator range
valided.
* include/debug/unordered_map: Same.
* include/debug/multiset.h: Same.
* include/debug/vector: Same.
* include/debug/unordered_set: Same.
* include/debug/deque: Same.
* include/debug/map.h: Same.
* include/debug/string: Same.
* include/debug/list: Same.
* include/debug/multimap.h: Same.
* testsuite/23_containers/util/debug/assign_neg.h: New test cases on
debug checks performed on container assign operation.
* testsuite/23_containers/util/debug/construct_neg.h: New test cases
on debug checks on constructors.
* testsuite/23_containers/util/debug/insert_neg.h: New test cases on
debug checks performed on container insert operations.
* testsuite/23_containers/unordered_map/debug/cont_traits.h,
debug_cont_traits.h, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
unordered_map.
* testsuite/23_containers/multimap/debug/cont_traits.h,
debug_cont_traits.h, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
multimap.
* testsuite/23_containers/set/debug/cont_traits.h,
debug_cont_traits.h, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
set.
* testsuite/23_containers/unordered_multimap/debug/cont_traits.h,
debug_cont_traits.h, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
unordered_multimap.
* testsuite/23_containers/unordered_set/debug/cont_traits.h,
debug_cont_traits.h, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
unordered_set.
* testsuite/23_containers/multiset/debug/cont_traits.h,
debug_cont_traits.h, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
multiset.
* testsuite/23_containers/unordered_multiset/debug/cont_traits.h,
debug_cont_traits.h, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
unordered_multiset.
* testsuite/23_containers/map/debug/cont_traits.h,
debug_cont_traits.h, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
map.
* testsuite/23_containers/vector/debug/cont_traits.h,
debug_cont_traits.h, assign1_neg.cc, assign2_neg.cc, assign3_neg.cc,
assign4_neg.cc, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
vector.
* testsuite/23_containers/deque/debug/cont_traits.h,
debug_cont_traits.h, assign1_neg.cc, assign2_neg.cc, assign3_neg.cc,
assign4_neg.cc, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
deque.
* testsuite/23_containers/list/debug/cont_traits.h,
debug_cont_traits.h, assign1_neg.cc, assign2_neg.cc, assign3_neg.cc,
assign4_neg.cc, construct1_neg.cc, construct2_neg.cc,
construct3_neg.cc, construct4_neg.cc, insert1_neg.cc, insert2_neg.cc,
insert3_neg.cc, insert4_neg.cc: New, debug test cases on
list.
* testsuite/performance/23_containers/range_construct/list_construct1.cc:
New, validate performance impact of the patch on the debug mode
* testsuite/performance/23_containers/range_construct/list_construct2.cc:
Idem
* testsuite/performance/23_containers/range_construct/vector_construct.cc:
Idem
* testsuite/performance/23_containers/range_construct/deque_construct.cc:
Idem

Bests


On 07/31/2010 07:33 PM, Paolo Carlini wrote:
On 07/31/2010 07:26 PM, Paolo Carlini wrote:
I think you should also consider removing _GLIBCXX_DEBUG_ASSERT
completely, because after your patch it becomes largely unused,

Nevermind this specific point. We are also using it outside the debug
directory, directly. Still, I'm a bit nervous about the changes to
functions.h, should be discussed in more detail.

Paolo.

Attachment: debug.patch
Description: Text document

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]