On 27/06/18 21:25 +0200, François Dumont wrote:
On 27/06/2018 02:13, Jonathan Wakely wrote:
On 26/06/18 17:03 +0100, 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.
We don't need to give up all checks in std::equal, we can do this:
@@ -1044,7 +1085,13 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
typename iterator_traits<_II1>::value_type,
typename iterator_traits<_II2>::value_type>)
__glibcxx_requires_valid_range(__first1, __last1);
-
+#ifdef _GLIBCXX_DEBUG
+ typedef typename iterator_traits<_II1>::iterator_category _Cat1;
+ typedef typename iterator_traits<_II2>::iterator_category _Cat2;
+ if (!__are_same<_Cat1, input_iterator_tag>::__value
+ && __are_same<_Cat2, random_access_iterator_tag>::__value)
+ __glibcxx_requires_can_increment_range(__first1, __last1,
__first2);
+#endif
return std::__equal_aux(std::__niter_base(__first1),
std::__niter_base(__last1),
std::__niter_base(__first2));
We don't give up any check.
We do for my version of the patch, because with the new __niter_base
overload the debug layer gets removed. Your patch kept the checking by
not removing the debug layer, but loses the memcmp optimization.
My suggestion above removes the debug layer so uses the optimization,
but adds a separate range check, so we still get checking as well.
We only give up on using the __builtin_memcmp when std::equal is
being used while Debug mode is active and std::equal is called
directly with std::__cxx1998::vector.
Moreover this check is wrong and will introduce regression when
running testsuite in Debug mode (this is why I know). It will assert
in the following case:
vector<int> v1 { 0, 0, 0 };
vector<int> v2 { 0, 1 };
equal(v1.begin(), v1.end(), v2.begin());
We should assert for that test, it's undefined behaviour.