diff --git a/doc/container.qbk b/doc/container.qbk index 501d582..1c2fd8f 100644 --- a/doc/container.qbk +++ b/doc/container.qbk @@ -1345,6 +1345,7 @@ use [*Boost.Container]? There are several reasons for that: * [@https://github.com/boostorg/container/issues/187 GitHub #187: ['"flat_map::erase and unique keys"]]. * [@https://github.com/boostorg/container/issues/188 GitHub #188: ['"Build fails when RTTI is disabled"]]. * [@https://github.com/boostorg/container/issues/192 GitHub #192: ['"basic_string::clear() has poor codegen compared to STL implementations"]]. + * [@https://github.com/boostorg/container/issues/197 GitHub #197: ['"small_vector::swap causes spurious allocations and suboptimal performance"]]. [endsect] diff --git a/include/boost/container/vector.hpp b/include/boost/container/vector.hpp index e9ff910..de4a56b 100644 --- a/include/boost/container/vector.hpp +++ b/include/boost/container/vector.hpp @@ -87,8 +87,8 @@ class vec_iterator //Defining element_type to make libstdc++'s std::pointer_traits well-formed leads to ambiguity //due to LWG3446. So we need to specialize std::pointer_traits. See - //https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96416 for details. /Many thanks to Jonathan Wakely - //for explaning the issue. + //https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96416 for details. Many thanks to Jonathan Wakely + //for explaining the issue. #ifndef BOOST_GNU_STDLIB //Define element_ typedef typename boost::intrusive::pointer_traits::element_type element_type; @@ -317,7 +317,8 @@ struct vector_alloc_holder (void)propagate_allocator; (void)p; (void)to_alloc; (void)from_alloc; const bool all_storage_propagable = !allocator_traits_type::is_partially_propagable::value || !allocator_traits_type::storage_is_unpropagable(from_alloc, p); - return all_storage_propagable && (propagate_allocator || allocator_traits_type::equal(from_alloc, to_alloc)); + return all_storage_propagable && + (propagate_allocator || allocator_traits_type::is_always_equal::value || allocator_traits_type::equal(from_alloc, to_alloc)); } BOOST_CONTAINER_FORCEINLINE @@ -2549,31 +2550,40 @@ private: void priv_swap(Vector &x, dtl::false_type) //version_N { const bool propagate_alloc = allocator_traits_type::propagate_on_container_swap::value; - if(are_swap_propagable( this->get_stored_allocator(), this->m_holder.start() - , x.get_stored_allocator(), x.m_holder.start(), propagate_alloc)){ + if (BOOST_UNLIKELY(&x == this)){ + return; + } + else if(are_swap_propagable( this->get_stored_allocator(), this->m_holder.start() + , x.get_stored_allocator(), x.m_holder.start(), propagate_alloc)){ //Just swap internals this->m_holder.swap_resources(x.m_holder); } else{ - if (BOOST_UNLIKELY(&x == this)) - return; - //Else swap element by element... bool const t_smaller = this->size() < x.size(); vector &sml = t_smaller ? *this : x; vector &big = t_smaller ? x : *this; - size_type const common_elements = sml.size(); - for(size_type i = 0; i != common_elements; ++i){ - boost::adl_move_swap(sml[i], big[i]); + //For empty containers, maybe storage can be moved from the other (just like in the move constructor) + if(sml.empty() && is_propagable_from(big.get_stored_allocator(), big.data(), sml.get_allocator(), propagate_alloc)){ + if(BOOST_LIKELY(0u != sml.capacity())) + sml.m_holder.deallocate(sml.m_holder.m_start, sml.m_holder.m_capacity); + sml.steal_resources(big); + } + else { + //Else swap element by element... + size_type const common_elements = sml.size(); + for(size_type i = 0; i != common_elements; ++i){ + boost::adl_move_swap(sml[i], big[i]); + } + //... and move-insert the remaining range + sml.insert( sml.cend() + , boost::make_move_iterator(boost::movelib::iterator_to_raw_pointer(big.nth(common_elements))) + , boost::make_move_iterator(boost::movelib::iterator_to_raw_pointer(big.end())) + ); + //Destroy remaining elements + big.erase(big.nth(common_elements), big.cend()); } - //... and move-insert the remaining range - sml.insert( sml.cend() - , boost::make_move_iterator(boost::movelib::iterator_to_raw_pointer(big.nth(common_elements))) - , boost::make_move_iterator(boost::movelib::iterator_to_raw_pointer(big.end())) - ); - //Destroy remaining elements - big.erase(big.nth(common_elements), big.cend()); } //And now swap the allocator dtl::swap_alloc(this->m_holder.alloc(), x.m_holder.alloc(), dtl::bool_()); diff --git a/test/vector_test.hpp b/test/vector_test.hpp index aa13d9a..8ca7201 100644 --- a/test/vector_test.hpp +++ b/test/vector_test.hpp @@ -43,7 +43,7 @@ namespace container { namespace test{ template -struct vector_hash_function_capacity +struct vector_has_function_capacity { typedef typename Vector::size_type size_type; template struct Check; @@ -57,32 +57,57 @@ struct vector_hash_function_capacity template bool vector_capacity_test(V1&, V2&, boost::container::dtl::false_type) { + //deque has no reserve return true; } template bool vector_capacity_test(MyBoostVector&boostvector, MyStdVector&stdvector, boost::container::dtl::true_type) { - //deque has no reserve - boostvector.reserve(boostvector.size()*2); - stdvector.reserve(stdvector.size()*2); - if(!test::CheckEqualContainers(boostvector, stdvector)) return false; + { + boostvector.reserve(boostvector.size()*2); + stdvector.reserve(stdvector.size()*2); + if(!test::CheckEqualContainers(boostvector, stdvector)) return false; - std::size_t cap = boostvector.capacity(); - boostvector.reserve(cap*2); - stdvector.reserve(cap*2); - if(!test::CheckEqualContainers(boostvector, stdvector)) return false; - boostvector.resize(0); - stdvector.resize(0); - if(!test::CheckEqualContainers(boostvector, stdvector)) return false; + std::size_t cap = boostvector.capacity(); + boostvector.reserve(cap*2); + stdvector.reserve(cap*2); + if(!test::CheckEqualContainers(boostvector, stdvector)) return false; + boostvector.resize(0); + stdvector.resize(0); + if(!test::CheckEqualContainers(boostvector, stdvector)) return false; - boostvector.resize(cap*2); - stdvector.resize(cap*2); - if(!test::CheckEqualContainers(boostvector, stdvector)) return false; + boostvector.resize(cap*2); + stdvector.resize(cap*2); + if(!test::CheckEqualContainers(boostvector, stdvector)) return false; - boostvector.resize(cap*2); - stdvector.resize(cap*2); - if(!test::CheckEqualContainers(boostvector, stdvector)) return false; + boostvector.resize(cap*2); + stdvector.resize(cap*2); + if(!test::CheckEqualContainers(boostvector, stdvector)) return false; + } + + //test capacity swapping in swap + { + MyBoostVector a, b; + a.resize(1000); + + const std::size_t sz = a.size(); + const std::size_t cap = a.capacity(); + + a.resize(1000); + a.swap(b); + if( !(b.capacity() == cap) ) return false; + if( !(b.size() == sz) ) return false; + if( !(a.capacity() != cap) ) return false; + if( !(a.empty()) ) return false; + + a.swap(b); + + if( !(a.capacity() == cap) ) return false; + if( !(a.size() == sz) ) return false; + if( !(b.capacity() != cap) ) return false; + if( !(b.empty()) ) return false; + } return true; } @@ -161,21 +186,21 @@ bool vector_copyable_only(MyBoostVector &boostvector, MyStdVector &stdvector, bo stdvector.clear(); boostvector.assign(v1.begin(), v1.end()); stdvector.assign(v2.begin(), v2.end()); - if(!test::CheckEqualContainers(boostvector, stdvector)) return 1; + if(!test::CheckEqualContainers(boostvector, stdvector)) return false; } { //Vector(n, T) ::boost::movelib::unique_ptr const stdvectorp = ::boost::movelib::make_unique(100u, int(5)); ::boost::movelib::unique_ptr const boostvectorp = ::boost::movelib::make_unique(100u, IntType(5)); - if(!test::CheckEqualContainers(*boostvectorp, *stdvectorp)) return 1; + if(!test::CheckEqualContainers(*boostvectorp, *stdvectorp)) return false; } { //Vector(n, T, alloc) ::boost::movelib::unique_ptr const stdvectorp = ::boost::movelib::make_unique(100u, int(5)); ::boost::movelib::unique_ptr const boostvectorp = ::boost::movelib::make_unique(100u, IntType(5), typename MyBoostVector::allocator_type()); - if(!test::CheckEqualContainers(*boostvectorp, *stdvectorp)) return 1; + if(!test::CheckEqualContainers(*boostvectorp, *stdvectorp)) return false; } { //Vector(It, It) ::boost::movelib::unique_ptr const stdvectorp = @@ -184,7 +209,7 @@ bool vector_copyable_only(MyBoostVector &boostvector, MyStdVector &stdvector, bo ::boost::movelib::make_unique(100u); ::boost::movelib::unique_ptr const boostvectorp2 = ::boost::movelib::make_unique(boostvectorp->begin(), boostvectorp->end()); - if(!test::CheckEqualContainers(*boostvectorp2, *stdvectorp)) return 1; + if(!test::CheckEqualContainers(*boostvectorp2, *stdvectorp)) return false; } { //Vector(It, It, alloc) ::boost::movelib::unique_ptr const stdvectorp = @@ -193,7 +218,7 @@ bool vector_copyable_only(MyBoostVector &boostvector, MyStdVector &stdvector, bo ::boost::movelib::make_unique(100u); ::boost::movelib::unique_ptr const boostvectorp2 = ::boost::movelib::make_unique(boostvectorp->begin(), boostvectorp->end(), typename MyBoostVector::allocator_type()); - if(!test::CheckEqualContainers(*boostvectorp2, *stdvectorp)) return 1; + if(!test::CheckEqualContainers(*boostvectorp2, *stdvectorp)) return false; } { //resize(n, T) ::boost::movelib::unique_ptr const stdvectorp = @@ -202,7 +227,7 @@ bool vector_copyable_only(MyBoostVector &boostvector, MyStdVector &stdvector, bo ::boost::movelib::make_unique(); stdvectorp->resize(100u, int(9)); boostvectorp->resize(100u, IntType(9)); - if(!test::CheckEqualContainers(*boostvectorp, *stdvectorp)) return 1; + if(!test::CheckEqualContainers(*boostvectorp, *stdvectorp)) return false; } //operator= { @@ -498,7 +523,7 @@ int vector_test() if(!test::CheckEqualContainers(boostvector, stdvector)) return 1; } - if(!vector_capacity_test(boostvector, stdvector, dtl::bool_::value>())) + if(!vector_capacity_test(boostvector, stdvector, dtl::bool_::value>())) return 1; boostvector.clear();