Is this the most efficient way of move the contents of one std::vector to the end of another in C++11?

Question

I was thinking that the vector::insert() and std::copy() commands require extra allocation. However, if I push_back() a newly created element an then swap() it I think this would reduce any allocations so long as the contained type is not allocating with the default constructor.

My question is actually specifically for std::vectors of type std::string, but should work for other contained types as stated here:

template <typename T>
void appendMove(std::vector<T>& dst, std::vector<T>& src)
{
    dst.reserve(dst.size() + src.size())
    for(std::vector<T>::iterator it = src.begin(); it != src.end(); ++it)
    {
        dst.push_back(std::vector<T>());
        std::swap(dst.end()[-1], *it);
    }
}

Am I correct? Am I missing anything else? Maybe there's some better way of doing this?

Answer 1

Performance disclaimer: Use profiling.

Performance considerations:

• push_back has to check for each call if the capacity of the vector is sufficient to insert the element. It is unlikely that the compiler is smart enough to avoid that check within the loop, that is, for every loop iteration it'll have to check, which can also prohibit further optimizations.
• If there's no call to reserve before, push_back has to adjust the capacity of the vector on the go, maybe multiple times within the loop, which would lead to moving the already stored elements.
• swap is slightly different from move: move has less strict guarantees on the moved objects, which could allow optimizations
• As GManNickG pointed out in the comments, vector::insert could reserve the necessary memory before inserting, as it gets the whole range to be inserted. This would probably require a specialization on random access iterators because std::difference for them is in O(1) (it could be applied to all bidirectional iterators, but this could be slower - two loop iterations - than not reserving).

The most efficient way I can think of is to reserve the necessary capacity and then insert the elements (either via push_back or via insert) without capacity checks.

A smart Standard Library implementation could do the call to reserve inside insert and not check for the capacity during insertion. I'm not entirely sure this would comply to the Standard, though.

If your library is smart enough, Andy Prowl's version (see comments) is sufficient:

dst.insert( dst.end(),
            std::make_move_iterator(src.begin()),
            std::make_move_iterator(src.end())    );

Else, you can write the call to reserve manually before invoking insert, but you cannot (AFAIK) insert/append an element w/o internal capacity checks:

template < typename T, typename FwdIt >
void append(FwdIt src_begin, FwdIt src_end, std::vector<T>& dst)
{
    dst.reserve( dst.size() + std::distance(src_begin, src_end) );
    // capacity checks might slow the loop inside `insert` down
    dst.insert(dst.end(), src_begin, src_end);
}

Example:

int main()
{
    std::vector<int> dst = { 0, 1, 2 };
    std::vector<int> src = { 3, 42 };

    append( std::make_move_iterator(src.begin()),
            std::make_move_iterator(src.end()),
            dst );
}

It might be better to implement append for different iterator types:

template < typename T, typename FwdIt >
void append(FwdIt src_begin, FwdIt src_end, std::vector<T>& dst,
            std::forward_iterator_tag)
{
    // let the vector handle resizing
    dst.insert(dst.end(), src_begin, src_end);
}

template < typename T, typename RAIt >
void append(RAIt src_begin, RAIt src_end, std::vector<T>& dst,
            std::random_access_iterator_tag)
{
    dst.reserve( dst.size() + (src_end - src_begin) );
    dst.insert(dst.end(), src_begin, src_end);
}

template < typename T, typename FwdIt >
void append(FwdIt src_begin, FwdIt src_end, std::vector<T>& dst)
{
    append( src_begin, src_end, dst,
            typename std::iterator_traits<FwdIt>::iterator_category() );
}

---

If a performance problem occurs because of the capacity checks inside the loop, you could try to default-construct the required additional elements first. When they exist (i.e. have been constructed) you can use the non-checked operator[] or simple iterators to move the src objects to their destination:

template < typename T, typename RAIt >
void append(RAIt src_begin, RAIt src_end, std::vector<T>& dst,
            std::random_access_iterator_tag)
{
    auto src_size = src_end - src_begin;

    dst.resize( dst.size() + src_size );

    // copy is not required to invoke capacity checks
    std::copy( src_begin, src_end, dst.end() - src_size );
    // ^this^ should move with the example provided above
}

---

Convenience wrapper:

template < typename T, typename FwdIt >
void append_move(FwdIt src_begin, FwdIt src_end, std::vector<T>& dst)
{
    append( std::make_move_iterator(src_begin),
            std::make_move_iterator(src_end),
            dst );
}