From cae72eec2f4fde469742b203bc08618e8c5bd8ea Mon Sep 17 00:00:00 2001 From: Daniel James Date: Wed, 17 Aug 2016 12:08:15 +0100 Subject: [PATCH] Insert/emplace with hint. --- doc/changes.qbk | 1 + include/boost/unordered/detail/equivalent.hpp | 63 +++++++++- include/boost/unordered/detail/unique.hpp | 73 ++++++++++- include/boost/unordered/unordered_map.hpp | 48 +++---- include/boost/unordered/unordered_set.hpp | 48 +++---- test/Jamfile.v2 | 1 + test/unordered/insert_hint_tests.cpp | 118 ++++++++++++++++++ test/unordered/insert_stable_tests.cpp | 11 +- 8 files changed, 307 insertions(+), 56 deletions(-) create mode 100644 test/unordered/insert_hint_tests.cpp diff --git a/doc/changes.qbk b/doc/changes.qbk index 54002878..7444c69b 100644 --- a/doc/changes.qbk +++ b/doc/changes.qbk @@ -278,5 +278,6 @@ C++11 support has resulted in some breaking changes: for `unordered_multiset` and `unordered_multimap`. Might be a little slower. * Stop using return value SFINAE which some older compilers have issues with. +* Check hint iterator in `insert`/`emplace_hint`. [endsect] diff --git a/include/boost/unordered/detail/equivalent.hpp b/include/boost/unordered/detail/equivalent.hpp index f3ddc47f..45cd026d 100644 --- a/include/boost/unordered/detail/equivalent.hpp +++ b/include/boost/unordered/detail/equivalent.hpp @@ -321,7 +321,10 @@ namespace boost { namespace unordered { namespace detail { // Emplace/Insert - static inline void add_after_node( + // Add node 'n' to the group containing 'pos'. + // If 'pos' is the first node in group, add to the end of the group, + // otherwise add before 'pos'. + static inline void add_to_node_group( node_pointer n, node_pointer pos) { @@ -338,7 +341,7 @@ namespace boost { namespace unordered { namespace detail { { n->hash_ = key_hash; if (pos.node_) { - this->add_after_node(n, pos.node_); + this->add_to_node_group(n, pos.node_); if (n->next_) { std::size_t next_bucket = this->hash_to_bucket( static_cast(n->next_)->hash_); @@ -375,6 +378,24 @@ namespace boost { namespace unordered { namespace detail { return iterator(n); } + inline iterator add_using_hint( + node_pointer n, + node_pointer hint) + { + n->hash_ = hint->hash_; + this->add_to_node_group(n, hint); + if (n->next_ != hint && n->next_) { + std::size_t next_bucket = this->hash_to_bucket( + static_cast(n->next_)->hash_); + if (next_bucket != this->hash_to_bucket(n->hash_)) { + this->get_bucket(next_bucket)->next_ = n; + } + } + ++this->size_; + return iterator(n); + } + + #if defined(BOOST_NO_CXX11_RVALUE_REFERENCES) # if defined(BOOST_NO_CXX11_VARIADIC_TEMPLATES) iterator emplace(boost::unordered::detail::emplace_args1< @@ -383,6 +404,13 @@ namespace boost { namespace unordered { namespace detail { BOOST_ASSERT(false); return iterator(); } + + iterator emplace_hint(c_iterator, boost::unordered::detail::emplace_args1< + boost::unordered::detail::please_ignore_this_overload> const&) + { + BOOST_ASSERT(false); + return iterator(); + } # else iterator emplace( boost::unordered::detail::please_ignore_this_overload const&) @@ -390,6 +418,13 @@ namespace boost { namespace unordered { namespace detail { BOOST_ASSERT(false); return iterator(); } + + iterator emplace_hint(c_iterator, + boost::unordered::detail::please_ignore_this_overload const&) + { + BOOST_ASSERT(false); + return iterator(); + } # endif #endif @@ -401,6 +436,14 @@ namespace boost { namespace unordered { namespace detail { this->node_alloc(), BOOST_UNORDERED_EMPLACE_FORWARD))); } + template + iterator emplace_hint(c_iterator hint, BOOST_UNORDERED_EMPLACE_ARGS) + { + return iterator(emplace_hint_impl(hint, + boost::unordered::detail::func::construct_value_generic( + this->node_alloc(), BOOST_UNORDERED_EMPLACE_FORWARD))); + } + iterator emplace_impl(node_pointer n) { node_tmp a(n, this->node_alloc()); @@ -411,6 +454,22 @@ namespace boost { namespace unordered { namespace detail { return this->add_node(a.release(), key_hash, position); } + iterator emplace_hint_impl(c_iterator hint, node_pointer n) + { + node_tmp a(n, this->node_alloc()); + key_type const& k = this->get_key(a.node_->value()); + if (hint.node_ && this->key_eq()(k, this->get_key(*hint))) { + this->reserve_for_insert(this->size_ + 1); + return this->add_using_hint(a.release(), hint.node_); + } + else { + std::size_t key_hash = this->hash(k); + iterator position = this->find_node(key_hash, k); + this->reserve_for_insert(this->size_ + 1); + return this->add_node(a.release(), key_hash, position); + } + } + void emplace_impl_no_rehash(node_pointer n) { node_tmp a(n, this->node_alloc()); diff --git a/include/boost/unordered/detail/unique.hpp b/include/boost/unordered/detail/unique.hpp index 07923981..db397980 100644 --- a/include/boost/unordered/detail/unique.hpp +++ b/include/boost/unordered/detail/unique.hpp @@ -316,6 +316,14 @@ namespace boost { namespace unordered { namespace detail { BOOST_ASSERT(false); return emplace_return(this->begin(), false); } + + iterator emplace_hint(c_iterator, + boost::unordered::detail::emplace_args1< + boost::unordered::detail::please_ignore_this_overload> const&) + { + BOOST_ASSERT(false); + return this->begin(); + } # else emplace_return emplace( boost::unordered::detail::please_ignore_this_overload const&) @@ -323,6 +331,13 @@ namespace boost { namespace unordered { namespace detail { BOOST_ASSERT(false); return emplace_return(this->begin(), false); } + + iterator emplace_hint(c_iterator, + boost::unordered::detail::please_ignore_this_overload const&) + { + BOOST_ASSERT(false); + return this->begin(); + } # endif #endif @@ -340,6 +355,21 @@ namespace boost { namespace unordered { namespace detail { #endif } + template + iterator emplace_hint(c_iterator hint, + BOOST_UNORDERED_EMPLACE_ARGS) + { +#if !defined(BOOST_NO_CXX11_VARIADIC_TEMPLATES) + return emplace_hint_impl(hint, + extractor::extract(BOOST_UNORDERED_EMPLACE_FORWARD), + BOOST_UNORDERED_EMPLACE_FORWARD); +#else + return emplace_hint_impl(hint, + extractor::extract(args.a0, args.a1), + BOOST_UNORDERED_EMPLACE_FORWARD); +#endif + } + #if defined(BOOST_NO_CXX11_VARIADIC_TEMPLATES) template emplace_return emplace( @@ -347,8 +377,27 @@ namespace boost { namespace unordered { namespace detail { { return emplace_impl(extractor::extract(args.a0), args); } + + template + iterator emplace_hint(c_iterator hint, + boost::unordered::detail::emplace_args1 const& args) + { + return emplace_hint_impl(hint, extractor::extract(args.a0), args); + } #endif + template + iterator emplace_hint_impl(c_iterator hint, key_type const& k, + BOOST_UNORDERED_EMPLACE_ARGS) + { + if (hint.node_ && this->key_eq()(k, this->get_key(*hint))) { + return iterator(hint.node_); + } + else { + return emplace_impl(k, BOOST_UNORDERED_EMPLACE_FORWARD).first; + } + } + template emplace_return emplace_impl(key_type const& k, BOOST_UNORDERED_EMPLACE_ARGS) @@ -368,11 +417,31 @@ namespace boost { namespace unordered { namespace detail { } } + template + iterator emplace_hint_impl(c_iterator hint, no_key, + BOOST_UNORDERED_EMPLACE_ARGS) + { + node_tmp b( + boost::unordered::detail::func::construct_value_generic( + this->node_alloc(), BOOST_UNORDERED_EMPLACE_FORWARD), + this->node_alloc()); + key_type const& k = this->get_key(b.node_->value()); + if (hint.node_ && this->key_eq()(k, this->get_key(*hint))) { + return iterator(hint.node_); + } + std::size_t key_hash = this->hash(k); + iterator pos = this->find_node(key_hash, k); + if (pos.node_) { + return pos; + } + else { + return this->resize_and_add_node(b.release(), key_hash); + } + } + template emplace_return emplace_impl(no_key, BOOST_UNORDERED_EMPLACE_ARGS) { - // Don't have a key, so construct the node first in order - // to be able to lookup the position. node_tmp b( boost::unordered::detail::func::construct_value_generic( this->node_alloc(), BOOST_UNORDERED_EMPLACE_FORWARD), diff --git a/include/boost/unordered/unordered_map.hpp b/include/boost/unordered/unordered_map.hpp index 3904e56e..fc6fbe0e 100644 --- a/include/boost/unordered/unordered_map.hpp +++ b/include/boost/unordered/unordered_map.hpp @@ -240,9 +240,9 @@ namespace unordered } template - iterator emplace_hint(const_iterator, BOOST_FWD_REF(Args)... args) + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(Args)... args) { - return table_.emplace(boost::forward(args)...).first; + return table_.emplace_hint(hint, boost::forward(args)...); } #else @@ -281,12 +281,12 @@ namespace unordered } template - iterator emplace_hint(const_iterator, BOOST_FWD_REF(A0) a0) + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0)) - ).first; + ); } template @@ -302,15 +302,15 @@ namespace unordered } template - iterator emplace_hint(const_iterator, + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0, BOOST_FWD_REF(A1) a1) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0), boost::forward(a1)) - ).first; + ); } template @@ -328,17 +328,17 @@ namespace unordered } template - iterator emplace_hint(const_iterator, + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0, BOOST_FWD_REF(A1) a1, BOOST_FWD_REF(A2) a2) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0), boost::forward(a1), boost::forward(a2)) - ).first; + ); } #define BOOST_UNORDERED_EMPLACE(z, n, _) \ @@ -360,15 +360,15 @@ namespace unordered BOOST_PP_ENUM_PARAMS_Z(z, n, typename A) \ > \ iterator emplace_hint( \ - const_iterator, \ + const_iterator hint, \ BOOST_PP_ENUM_##z(n, BOOST_UNORDERED_FWD_PARAM, a) \ ) \ { \ - return table_.emplace( \ + return table_.emplace_hint(hint, \ boost::unordered::detail::create_emplace_args( \ BOOST_PP_ENUM_##z(n, BOOST_UNORDERED_CALL_FORWARD, \ a) \ - )).first; \ + )); \ } BOOST_PP_REPEAT_FROM_TO(4, BOOST_UNORDERED_EMPLACE_LIMIT, @@ -724,9 +724,9 @@ namespace unordered } template - iterator emplace_hint(const_iterator, BOOST_FWD_REF(Args)... args) + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(Args)... args) { - return table_.emplace(boost::forward(args)...); + return table_.emplace_hint(hint, boost::forward(args)...); } #else @@ -765,9 +765,9 @@ namespace unordered } template - iterator emplace_hint(const_iterator, BOOST_FWD_REF(A0) a0) + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0)) ); @@ -786,11 +786,11 @@ namespace unordered } template - iterator emplace_hint(const_iterator, + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0, BOOST_FWD_REF(A1) a1) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0), boost::forward(a1)) @@ -812,12 +812,12 @@ namespace unordered } template - iterator emplace_hint(const_iterator, + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0, BOOST_FWD_REF(A1) a1, BOOST_FWD_REF(A2) a2) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0), boost::forward(a1), @@ -844,11 +844,11 @@ namespace unordered BOOST_PP_ENUM_PARAMS_Z(z, n, typename A) \ > \ iterator emplace_hint( \ - const_iterator, \ + const_iterator hint, \ BOOST_PP_ENUM_##z(n, BOOST_UNORDERED_FWD_PARAM, a) \ ) \ { \ - return table_.emplace( \ + return table_.emplace_hint(hint, \ boost::unordered::detail::create_emplace_args( \ BOOST_PP_ENUM_##z(n, BOOST_UNORDERED_CALL_FORWARD, \ a) \ diff --git a/include/boost/unordered/unordered_set.hpp b/include/boost/unordered/unordered_set.hpp index c8c06215..aa9911bc 100644 --- a/include/boost/unordered/unordered_set.hpp +++ b/include/boost/unordered/unordered_set.hpp @@ -237,9 +237,9 @@ namespace unordered } template - iterator emplace_hint(const_iterator, BOOST_FWD_REF(Args)... args) + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(Args)... args) { - return table_.emplace(boost::forward(args)...).first; + return table_.emplace_hint(hint, boost::forward(args)...); } #else @@ -278,12 +278,12 @@ namespace unordered } template - iterator emplace_hint(const_iterator, BOOST_FWD_REF(A0) a0) + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0)) - ).first; + ); } template @@ -299,15 +299,15 @@ namespace unordered } template - iterator emplace_hint(const_iterator, + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0, BOOST_FWD_REF(A1) a1) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0), boost::forward(a1)) - ).first; + ); } template @@ -325,17 +325,17 @@ namespace unordered } template - iterator emplace_hint(const_iterator, + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0, BOOST_FWD_REF(A1) a1, BOOST_FWD_REF(A2) a2) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0), boost::forward(a1), boost::forward(a2)) - ).first; + ); } #define BOOST_UNORDERED_EMPLACE(z, n, _) \ @@ -357,15 +357,15 @@ namespace unordered BOOST_PP_ENUM_PARAMS_Z(z, n, typename A) \ > \ iterator emplace_hint( \ - const_iterator, \ + const_iterator hint, \ BOOST_PP_ENUM_##z(n, BOOST_UNORDERED_FWD_PARAM, a) \ ) \ { \ - return table_.emplace( \ + return table_.emplace_hint(hint, \ boost::unordered::detail::create_emplace_args( \ BOOST_PP_ENUM_##z(n, BOOST_UNORDERED_CALL_FORWARD, \ a) \ - )).first; \ + )); \ } BOOST_PP_REPEAT_FROM_TO(4, BOOST_UNORDERED_EMPLACE_LIMIT, @@ -707,9 +707,9 @@ namespace unordered } template - iterator emplace_hint(const_iterator, BOOST_FWD_REF(Args)... args) + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(Args)... args) { - return table_.emplace(boost::forward(args)...); + return table_.emplace_hint(hint, boost::forward(args)...); } #else @@ -748,9 +748,9 @@ namespace unordered } template - iterator emplace_hint(const_iterator, BOOST_FWD_REF(A0) a0) + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0)) ); @@ -769,11 +769,11 @@ namespace unordered } template - iterator emplace_hint(const_iterator, + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0, BOOST_FWD_REF(A1) a1) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0), boost::forward(a1)) @@ -795,12 +795,12 @@ namespace unordered } template - iterator emplace_hint(const_iterator, + iterator emplace_hint(const_iterator hint, BOOST_FWD_REF(A0) a0, BOOST_FWD_REF(A1) a1, BOOST_FWD_REF(A2) a2) { - return table_.emplace( + return table_.emplace_hint(hint, boost::unordered::detail::create_emplace_args( boost::forward(a0), boost::forward(a1), @@ -827,11 +827,11 @@ namespace unordered BOOST_PP_ENUM_PARAMS_Z(z, n, typename A) \ > \ iterator emplace_hint( \ - const_iterator, \ + const_iterator hint, \ BOOST_PP_ENUM_##z(n, BOOST_UNORDERED_FWD_PARAM, a) \ ) \ { \ - return table_.emplace( \ + return table_.emplace_hint(hint, \ boost::unordered::detail::create_emplace_args( \ BOOST_PP_ENUM_##z(n, BOOST_UNORDERED_CALL_FORWARD, \ a) \ diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 6f844095..cef0f17e 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -37,6 +37,7 @@ test-suite unordered [ run unordered/assign_tests.cpp ] [ run unordered/insert_tests.cpp ] [ run unordered/insert_stable_tests.cpp ] + [ run unordered/insert_hint_tests.cpp ] [ run unordered/unnecessary_copy_tests.cpp ] [ run unordered/erase_tests.cpp ] [ run unordered/erase_equiv_tests.cpp ] diff --git a/test/unordered/insert_hint_tests.cpp b/test/unordered/insert_hint_tests.cpp new file mode 100644 index 00000000..3a07c7b8 --- /dev/null +++ b/test/unordered/insert_hint_tests.cpp @@ -0,0 +1,118 @@ + +// Copyright 2016 Daniel James. +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +#include "../helpers/prefix.hpp" +#include +#include +#include "../helpers/postfix.hpp" + +#include "../helpers/test.hpp" +#include "../helpers/invariants.hpp" + +#include +#include +#include + +namespace insert_hint +{ +UNORDERED_AUTO_TEST(insert_hint_empty) { + typedef boost::unordered_multiset container; + container x; + x.insert(x.cbegin(), 10); + BOOST_TEST_EQ(x.size(), 1); + BOOST_TEST_EQ(x.count(10), 1); + test::check_equivalent_keys(x); +} + +UNORDERED_AUTO_TEST(insert_hint_empty2) { + typedef boost::unordered_multimap container; + container x; + x.emplace_hint(x.cbegin(), "hello", 50); + BOOST_TEST_EQ(x.size(), 1); + BOOST_TEST_EQ(x.count("hello"), 1); + BOOST_TEST_EQ(x.find("hello")->second, 50); + test::check_equivalent_keys(x); +} + +UNORDERED_AUTO_TEST(insert_hint_single) { + typedef boost::unordered_multiset container; + container x; + x.insert("equal"); + x.insert(x.cbegin(), "equal"); + BOOST_TEST_EQ(x.size(), 2); + BOOST_TEST_EQ(x.count("equal"), 2); + test::check_equivalent_keys(x); +} + +UNORDERED_AUTO_TEST(insert_hint_single2) { + typedef boost::unordered_multimap container; + container x; + x.emplace(10, "one"); + x.emplace_hint(x.cbegin(), 10, "two"); + BOOST_TEST_EQ(x.size(), 2); + BOOST_TEST_EQ(x.count(10), 2); + + container::iterator it = x.find(10); + std::string v0 = (it++)->second; + std::string v1 = (it++)->second; + + BOOST_TEST(v0 == "one" || v0 == "two"); + BOOST_TEST(v1 == "one" || v1 == "two"); + BOOST_TEST(v0 != v1); + + test::check_equivalent_keys(x); +} + +UNORDERED_AUTO_TEST(insert_hint_multiple) { + for (unsigned int size = 0; size < 10; ++size) { + for (unsigned int offset = 0; offset <= size; ++offset) { + typedef boost::unordered_multiset container; + container x; + + for (unsigned int i = 0; i < size; ++i) { x.insert("multiple"); } + + BOOST_TEST_EQ(x.size(), size); + + container::const_iterator position = x.cbegin(); + for (unsigned int i = 0; i < offset; ++i) { ++position; } + + x.insert(position, "multiple"); + + BOOST_TEST_EQ(x.size(), size + 1); + BOOST_TEST_EQ(x.count("multiple"), size + 1); + test::check_equivalent_keys(x); + } + } +} + +UNORDERED_AUTO_TEST(insert_hint_unique) { + typedef boost::unordered_set container; + container x; + x.insert(x.cbegin(), 10); + BOOST_TEST_EQ(x.size(), 1); + BOOST_TEST_EQ(x.count(10), 1); + test::check_equivalent_keys(x); +} + +UNORDERED_AUTO_TEST(insert_hint_unique_single) { + typedef boost::unordered_set container; + container x; + x.insert(10); + + x.insert(x.cbegin(), 10); + BOOST_TEST_EQ(x.size(), 1); + BOOST_TEST(x.count(10) == 1); + test::check_equivalent_keys(x); + + x.insert(x.cbegin(), 20); + BOOST_TEST(x.size() == 2); + BOOST_TEST(x.count(10) == 1); + BOOST_TEST(x.count(20) == 1); + test::check_equivalent_keys(x); +} + +} + +RUN_TESTS() diff --git a/test/unordered/insert_stable_tests.cpp b/test/unordered/insert_stable_tests.cpp index f750e50f..b4482764 100644 --- a/test/unordered/insert_stable_tests.cpp +++ b/test/unordered/insert_stable_tests.cpp @@ -49,6 +49,8 @@ UNORDERED_AUTO_TEST(stable_insert_test1) { x.insert(insert_stable::member(1,2)); x.insert(insert_stable::member(1,3)); + BOOST_TEST(x.count(insert_stable::member(1,4)) == 3); + boost::unordered_multiset::const_iterator it = x.begin(), end = x.end(); BOOST_TEST(it != end); @@ -66,10 +68,11 @@ UNORDERED_AUTO_TEST(stable_insert_test2) { boost::unordered_multimap::const_iterator iterator; - iterator it - = x.insert(x.end(), std::make_pair(insert_stable::member(1,1), 1)); - it = x.insert(it, std::make_pair(insert_stable::member(1,2), 2)); - it = x.insert(it, std::make_pair(insert_stable::member(1,3), 3)); + iterator it = x.emplace(insert_stable::member(1,1), 1); + it = x.emplace(insert_stable::member(1,2), 2); + it = x.emplace(insert_stable::member(1,3), 3); + + BOOST_TEST(x.count(insert_stable::member(1,4)) == 3); it = x.begin(); iterator end = x.end();