Improve insert/emplace robustness to self insertion

Jonathan Wakely jwakely@redhat.com
Wed Jun 29 21:32:00 GMT 2016


On 29/06/16 21:36 +0100, Jonathan Wakely wrote:
>On 29/06/16 21:43 +0200, François Dumont wrote:
>>As asked here is now a patch to only fix the robustness issue. The 
>>consequence is that it is reverting the latest optimization as, 
>>without smart algo, we always need to do a copy to protect against 
>>insertion of values contained in the vector as shown by new tests.
>
>I don't understand. There is no problem with insert(), only with
>emplace(), so why do both get worse?
>
>Also, the problem is with emplacing from an lvalue, so why do the
>number of operations change for test02 and test03, which are for
>xvalues and rvalues?
>
>I haven't analyzed your patch, but the results seem wrong. We should
>not have to do any more work to insert rvalues.
>
>What am I missing?

It seems to me that the minimal fix would be:

--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -312,7 +312,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
          }
        else
          _M_insert_aux(begin() + (__position - cbegin()),
-                       std::forward<_Args>(__args)...);
+                       _Tp(std::forward<_Args>(__args)...));
        return iterator(this->_M_impl._M_start + __n);
       }
 
This causes regressions in the insert_vs_emplace.cc test because we
insert rvalues using emplace:

      iterator
      insert(const_iterator __position, value_type&& __x)
      { return emplace(__position, std::move(__x)); }

That's suboptimal, since in the general case we need an extra
construction for emplacing, but we know that we don't need to do that
when inserting rvalues.

So the correct fix would be to implement inserting rvalues without
using emplace.

The attached patch is a smaller change, and doesn't change the number
of operations for insertions, only for emplacing lvalues.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 4019 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20160629/c854e2ff/attachment.bin>


More information about the Gcc-patches mailing list