From 4e6292b439f7109f0f308411fd6728840eef3e60 Mon Sep 17 00:00:00 2001 From: Daniel James Date: Tue, 15 Dec 2009 22:52:52 +0000 Subject: [PATCH] Implement an alternative erase function that doesn't return an iterator. Ref #3693 [SVN r58403] --- doc/changes.qbk | 4 + doc/ref.xml | 120 ++++++++++++++++++++++ include/boost/unordered/detail/fwd.hpp | 3 +- include/boost/unordered/detail/table.hpp | 12 ++- include/boost/unordered/unordered_map.hpp | 14 ++- include/boost/unordered/unordered_set.hpp | 14 ++- test/unordered/erase_tests.cpp | 49 +++++++++ 7 files changed, 210 insertions(+), 6 deletions(-) diff --git a/doc/changes.qbk b/doc/changes.qbk index 790e3863..adddc366 100644 --- a/doc/changes.qbk +++ b/doc/changes.qbk @@ -107,6 +107,10 @@ First official release. * Support instantiating the containers with incomplete value types. * Reduced the number of warnings (mostly in tests). +* [@http://svn.boost.org/trac/boost/ticket/3693 Ticket 3693]: + Add `erase_return_void` as a temporary workaround for the current + `erase` which can be inefficient because it has to find the next + element to return an iterator. * [@http://svn.boost.org/trac/boost/ticket/3773 Ticket 3773]: Add missing `std` qualifier to `ptrdiff_t`. diff --git a/doc/ref.xml b/doc/ref.xml index 4ca51abf..b55b5492 100644 --- a/doc/ref.xml +++ b/doc/ref.xml @@ -459,6 +459,15 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) Only throws an exception if it is thrown by hasher or key_equal. In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + When the number of elements is a lot smaller than the number of buckets + this function can be very inefficient as it has to search through empty + buckets for the next element, in order to return the iterator. + As a temporary workaround, the container has the method + erase_return_void which will be faster. + + @@ -494,6 +503,27 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + const_iterator + + iterator + + Erase the element pointed to by position. + + + Only throws an exception if it is thrown by hasher or key_equal. + In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + + This is a temporary workaround for the inefficient + erase method. Hopefully, in a future + version the signature of erase will + be changed and this will be deprecated. + + + void @@ -1252,6 +1282,15 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) Only throws an exception if it is thrown by hasher or key_equal. In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + When the number of elements is a lot smaller than the number of buckets + this function can be very inefficient as it has to search through empty + buckets for the next element, in order to return the iterator. + As a temporary workaround, the container has the method + erase_return_void which will be faster. + + @@ -1287,6 +1326,27 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + const_iterator + + iterator + + Erase the element pointed to by position. + + + Only throws an exception if it is thrown by hasher or key_equal. + In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + + This is a temporary workaround for the inefficient + erase method. Hopefully, in a future + version the signature of erase will + be changed and this will be deprecated. + + + void @@ -2059,6 +2119,15 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) Only throws an exception if it is thrown by hasher or key_equal. In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + When the number of elements is a lot smaller than the number of buckets + this function can be very inefficient as it has to search through empty + buckets for the next element, in order to return the iterator. + As a temporary workaround, the container has the method + erase_return_void which will be faster. + + @@ -2094,6 +2163,27 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + const_iterator + + iterator + + Erase the element pointed to by position. + + + Only throws an exception if it is thrown by hasher or key_equal. + In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + + This is a temporary workaround for the inefficient + erase method. Hopefully, in a future + version the signature of erase will + be changed and this will be deprecated. + + + void @@ -2901,6 +2991,15 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) Only throws an exception if it is thrown by hasher or key_equal. In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + When the number of elements is a lot smaller than the number of buckets + this function can be very inefficient as it has to search through empty + buckets for the next element, in order to return the iterator. + As a temporary workaround, the container has the method + erase_return_void which will be faster. + + @@ -2936,6 +3035,27 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + const_iterator + + iterator + + Erase the element pointed to by position. + + + Only throws an exception if it is thrown by hasher or key_equal. + In this implementation, this overload doesn't call either function object's methods so it is no throw, but this might not be true in other implementations. + + + + This is a temporary workaround for the inefficient + erase method. Hopefully, in a future + version the signature of erase will + be changed and this will be deprecated. + + + void diff --git a/include/boost/unordered/detail/fwd.hpp b/include/boost/unordered/detail/fwd.hpp index ff3d63e9..7128dbe9 100644 --- a/include/boost/unordered/detail/fwd.hpp +++ b/include/boost/unordered/detail/fwd.hpp @@ -532,7 +532,8 @@ namespace boost { namespace unordered_detail { void clear(); std::size_t erase_key(key_type const& k); - iterator_base erase(iterator_base r); + iterator_base erase_return_iterator(iterator_base r); + void erase(iterator_base r); std::size_t erase_group(node_ptr* it, bucket_ptr bucket); iterator_base erase_range(iterator_base r1, iterator_base r2); diff --git a/include/boost/unordered/detail/table.hpp b/include/boost/unordered/detail/table.hpp index 2f4e5ebd..ab95f595 100644 --- a/include/boost/unordered/detail/table.hpp +++ b/include/boost/unordered/detail/table.hpp @@ -652,10 +652,20 @@ namespace boost { namespace unordered_detail { return *it ? this->erase_group(it, bucket) : 0; } + template + void hash_table::erase(iterator_base r) + { + BOOST_ASSERT(r.node_); + --this->size_; + node::unlink_node(*r.bucket_, r.node_); + this->delete_node(r.node_); + // r has been invalidated but its bucket is still valid + this->recompute_begin_bucket(r.bucket_); + } template BOOST_DEDUCED_TYPENAME T::iterator_base - hash_table::erase(iterator_base r) + hash_table::erase_return_iterator(iterator_base r) { BOOST_ASSERT(r.node_); iterator_base next = r; diff --git a/include/boost/unordered/unordered_map.hpp b/include/boost/unordered/unordered_map.hpp index ef649897..712f769c 100644 --- a/include/boost/unordered/unordered_map.hpp +++ b/include/boost/unordered/unordered_map.hpp @@ -355,7 +355,7 @@ namespace boost iterator erase(const_iterator position) { - return iterator(table_.erase(get(position))); + return iterator(table_.erase_return_iterator(get(position))); } size_type erase(const key_type& k) @@ -368,6 +368,11 @@ namespace boost return iterator(table_.erase_range(get(first), get(last))); } + void erase_return_void(const_iterator position) + { + table_.erase(get(position)); + } + void clear() { table_.clear(); @@ -868,7 +873,7 @@ namespace boost iterator erase(const_iterator position) { - return iterator(table_.erase(get(position))); + return iterator(table_.erase_return_iterator(get(position))); } size_type erase(const key_type& k) @@ -881,6 +886,11 @@ namespace boost return iterator(table_.erase_range(get(first), get(last))); } + void erase_return_void(const_iterator position) + { + table_.erase(get(position)); + } + void clear() { table_.clear(); diff --git a/include/boost/unordered/unordered_set.hpp b/include/boost/unordered/unordered_set.hpp index 59843fd6..6982cf93 100644 --- a/include/boost/unordered/unordered_set.hpp +++ b/include/boost/unordered/unordered_set.hpp @@ -348,7 +348,7 @@ namespace boost iterator erase(const_iterator position) { - return iterator(table_.erase(get(position))); + return iterator(table_.erase_return_iterator(get(position))); } size_type erase(const key_type& k) @@ -361,6 +361,11 @@ namespace boost return iterator(table_.erase_range(get(first), get(last))); } + void erase_return_void(const_iterator position) + { + table_.erase(get(position)); + } + void clear() { table_.clear(); @@ -822,7 +827,7 @@ namespace boost iterator erase(const_iterator position) { - return iterator(table_.erase(get(position))); + return iterator(table_.erase_return_iterator(get(position))); } size_type erase(const key_type& k) @@ -835,6 +840,11 @@ namespace boost return iterator(table_.erase_range(get(first), get(last))); } + void erase_return_void(const_iterator position) + { + table_.erase(get(position)); + } + void clear() { table_.clear(); diff --git a/test/unordered/erase_tests.cpp b/test/unordered/erase_tests.cpp index eab22f4c..1bc6e4c6 100644 --- a/test/unordered/erase_tests.cpp +++ b/test/unordered/erase_tests.cpp @@ -112,6 +112,55 @@ void erase_tests1(Container*, test::random_generator generator = test::default_g BOOST_TEST(x.erase(x.begin(), x.end()) == x.begin()); } + std::cerr<<"erase_return_void(begin()).\n"; + { + test::random_values v(1000, generator); + Container x(v.begin(), v.end()); + std::size_t size = x.size(); + while(size > 0 && !x.empty()) + { + BOOST_DEDUCED_TYPENAME Container::key_type key = test::get_key(*x.begin()); + std::size_t count = x.count(key); + x.erase_return_void(x.begin()); + --size; + BOOST_TEST(x.count(key) == count - 1); + BOOST_TEST(x.size() == size); + } + BOOST_TEST(x.empty()); + } + + std::cerr<<"erase_return_void(random position).\n"; + { + test::random_values v(1000, generator); + Container x(v.begin(), v.end()); + std::size_t size = x.size(); + while(size > 0 && !x.empty()) + { + using namespace std; + int index = rand() % (int) x.size(); + BOOST_DEDUCED_TYPENAME Container::const_iterator prev, pos, next; + if(index == 0) { + prev = pos = x.begin(); + } + else { + prev = boost::next(x.begin(), index - 1); + pos = boost::next(prev); + } + next = boost::next(pos); + BOOST_DEDUCED_TYPENAME Container::key_type key = test::get_key(*pos); + std::size_t count = x.count(key); + x.erase_return_void(pos); + --size; + if(size > 0) + BOOST_TEST(index == 0 ? next == x.begin() : + next == boost::next(prev)); + BOOST_TEST(x.count(key) == count - 1); + BOOST_TEST(x.size() == size); + } + BOOST_TEST(x.empty()); + } + + std::cerr<<"clear().\n"; { test::random_values v(500, generator);