Limit Debug mode impact: overload __niter_base
Jonathan Wakely
jwakely@redhat.com
Wed Jun 27 19:17:00 GMT 2018
On 27/06/18 21:03 +0200, François Dumont wrote:
>On 26/06/2018 18:03, Jonathan Wakely wrote:
>>On 18/06/18 23:01 +0200, François Dumont wrote:
>>>Hi
>>>
>>>Â Â Â I abandon the idea of providing Debug algos, it would be too
>>>much code to add and maintain. However I haven't quit on reducing
>>>Debug mode performance impact.
>>>
>>>Â Â Â So this patch make use of the existing std::__niter_base to
>>>get rid of the debug layer around __gnu_debug::vector<>::iterator
>>>so that __builtin_memmove replacement can take place.
>>>
>>>Â Â Â As _Safe_iterator<> do not have a constructor taking a pointer
>>>I change algos implementation so that we do not try to instantiate
>>>the iterator type ourselves but rather rely on its operators + or
>>>-.
>>>
>>>Â Â Â The small drawback is that for std::equal algo where we can't
>>>use the __glibcxx_can_increment we need to keep the debug layer to
>>>make sure we don't reach past-the-end iterator. So I had to remove
>>>usage of __niter_base when in Debug mode, doing so it also disable
>>>potential usage of __builtin_memcmp when calling std::equal on
>>>std::__cxx1998::vector<> iterators. A rather rare situation I
>>>think.
>>>
>>>Â Â Â Note that I don't know how to test that __builtin_memmove has
>>>been indeed used. So I've been through some debug sessions to
>>>check that.
>>
>>The attached patch (not fully tested) seems to be a much simpler way
>>to achieve the same thing. Instead of modifying all the helper
>>structs, just define a new function to re-wrap the result into the
>>desired iterator type.
>
>Through my proposal I tried to limit the presence of Debug code in the
>normal mode code. If you prefer to preserve existing code it is indeed
>a simpler approach.
But to avoid any dependency on _GLIBCXX_DEBUG you had to make more
changes in more places, and complicate all the helper structs.
If we really want to avoid the #ifdef _GLIBCXX_DEBUG code in
<bits/stl_algobase.h> we could move the debug mode overloads into
<debug/stl_iterator.h>. I don't feel strongly about that, I think it's
OK in <bits/stl_algobase.h> and would be OK in the debug header too
(assuming it still works).
>>>diff --git a/libstdc++-v3/include/debug/stl_iterator.h
>>>b/libstdc++-v3/include/debug/stl_iterator.h
>>>index a6a2a76..eca7203 100644
>>>--- a/libstdc++-v3/include/debug/stl_iterator.h
>>>+++ b/libstdc++-v3/include/debug/stl_iterator.h
>>>@@ -120,4 +120,19 @@ namespace __gnu_debug
>>>#endif
>>>}
>>>
>>>+#if __cplusplus >= 201103L
>>>+namespace std
>>>+{
>>>+_GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>+
>>>+template<typename _Iterator, typename _Container, typename _Sequence>
>>>+Â Â Â _Iterator
>>>+Â Â Â __niter_base(const __gnu_debug::_Safe_iterator<
>>>+Â Â Â Â Â Â Â Â __gnu_cxx::__normal_iterator<_Iterator, _Container>,
>>>+Â Â Â Â Â Â Â Â _Sequence>&);
>>>+
>>>+_GLIBCXX_END_NAMESPACE_VERSION
>>>+}
>>>+#endif
>>
>>Why is this overload only defined for C++11 and later? I defined it
>>unconditionally in the attached patch.
>
>I initially wrote something like:
>template<typename _Iterator, typename _Sequence>
>Â Â Â inline auto
>Â Â Â __niter_base(const __gnu_debug::_Safe_iterator<_Iterator,
>_Container>& __it)
>Â Â Â -> decltype(__niter_base(__it.base()))
>Â Â Â { return __niter_base(__it.base()); }
>
>which require C++11
>
>I forgot to remove the __cplusplus check when I eventually use a more
>specific overload.
Ah OK, I wondered if there was some reason I had missed.
>>
>>What do you think?
>>
>>
>Yes, that is a fine approach. Do you wat to apply it or do you want me
>to work on it a little and re-submit the patch ?
I sent a follow-up with a complete patch that passes the testsuite.
I'm happy with that version of the patch, but do you have any
suggestions to improve it?
More information about the Gcc-patches
mailing list