Limit Debug mode impact: overload __niter_base

[François Dumont]

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.

>
>
>> 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.

>
> 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 ?

François