Boost logo

Boost-Commit :

Subject: [Boost-commit] svn:boost r58403 - in trunk: boost/unordered boost/unordered/detail libs/unordered/doc libs/unordered/test/unordered
From: daniel_james_at_[hidden]
Date: 2009-12-15 17:52:53


Author: danieljames
Date: 2009-12-15 17:52:52 EST (Tue, 15 Dec 2009)
New Revision: 58403
URL: http://svn.boost.org/trac/boost/changeset/58403

Log:
Implement an alternative erase function that doesn't return an iterator.
Ref #3693
Text files modified:
   trunk/boost/unordered/detail/fwd.hpp | 3
   trunk/boost/unordered/detail/table.hpp | 12 +++
   trunk/boost/unordered/unordered_map.hpp | 14 ++++
   trunk/boost/unordered/unordered_set.hpp | 14 ++++
   trunk/libs/unordered/doc/changes.qbk | 4 +
   trunk/libs/unordered/doc/ref.xml | 120 ++++++++++++++++++++++++++++++++++++++++
   trunk/libs/unordered/test/unordered/erase_tests.cpp | 49 ++++++++++++++++
   7 files changed, 210 insertions(+), 6 deletions(-)

Modified: trunk/boost/unordered/detail/fwd.hpp
==============================================================================
--- trunk/boost/unordered/detail/fwd.hpp (original)
+++ trunk/boost/unordered/detail/fwd.hpp 2009-12-15 17:52:52 EST (Tue, 15 Dec 2009)
@@ -532,7 +532,8 @@
 
         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);
 

Modified: trunk/boost/unordered/detail/table.hpp
==============================================================================
--- trunk/boost/unordered/detail/table.hpp (original)
+++ trunk/boost/unordered/detail/table.hpp 2009-12-15 17:52:52 EST (Tue, 15 Dec 2009)
@@ -652,10 +652,20 @@
         return *it ? this->erase_group(it, bucket) : 0;
     }
 
+ template <class T>
+ void hash_table<T>::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 <class T>
     BOOST_DEDUCED_TYPENAME T::iterator_base
- hash_table<T>::erase(iterator_base r)
+ hash_table<T>::erase_return_iterator(iterator_base r)
     {
         BOOST_ASSERT(r.node_);
         iterator_base next = r;

Modified: trunk/boost/unordered/unordered_map.hpp
==============================================================================
--- trunk/boost/unordered/unordered_map.hpp (original)
+++ trunk/boost/unordered/unordered_map.hpp 2009-12-15 17:52:52 EST (Tue, 15 Dec 2009)
@@ -355,7 +355,7 @@
 
         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 @@
             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 @@
 
         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 @@
             return iterator(table_.erase_range(get(first), get(last)));
         }
 
+ void erase_return_void(const_iterator position)
+ {
+ table_.erase(get(position));
+ }
+
         void clear()
         {
             table_.clear();

Modified: trunk/boost/unordered/unordered_set.hpp
==============================================================================
--- trunk/boost/unordered/unordered_set.hpp (original)
+++ trunk/boost/unordered/unordered_set.hpp 2009-12-15 17:52:52 EST (Tue, 15 Dec 2009)
@@ -348,7 +348,7 @@
 
         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 @@
             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 @@
 
         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 @@
             return iterator(table_.erase_range(get(first), get(last)));
         }
 
+ void erase_return_void(const_iterator position)
+ {
+ table_.erase(get(position));
+ }
+
         void clear()
         {
             table_.clear();

Modified: trunk/libs/unordered/doc/changes.qbk
==============================================================================
--- trunk/libs/unordered/doc/changes.qbk (original)
+++ trunk/libs/unordered/doc/changes.qbk 2009-12-15 17:52:52 EST (Tue, 15 Dec 2009)
@@ -107,6 +107,10 @@
 
 * 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`.
 

Modified: trunk/libs/unordered/doc/ref.xml
==============================================================================
--- trunk/libs/unordered/doc/ref.xml (original)
+++ trunk/libs/unordered/doc/ref.xml 2009-12-15 17:52:52 EST (Tue, 15 Dec 2009)
@@ -459,6 +459,15 @@
                 <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
                 <para>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.</para>
               </throws>
+ <notes>
+ <para>
+ 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
+ <methodname>erase_return_void</methodname> which will be faster.
+ </para>
+ </notes>
             </method>
             <method name="erase">
               <parameter name="k">
@@ -494,6 +503,27 @@
                 <para>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.</para>
               </throws>
             </method>
+ <method name="erase_return_void">
+ <parameter name="position">
+ <paramtype>const_iterator</paramtype>
+ </parameter>
+ <type>iterator</type>
+ <description>
+ <para>Erase the element pointed to by <code>position</code>.</para>
+ </description>
+ <throws>
+ <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
+ <para>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.</para>
+ </throws>
+ <notes>
+ <para>
+ This is a temporary workaround for the inefficient
+ <methodname>erase</methodname> method. Hopefully, in a future
+ version the signature of <methodname>erase</methodname> will
+ be changed and this will be deprecated.
+ </para>
+ </notes>
+ </method>
             <method name="clear">
               <type>void</type>
               <description>
@@ -1252,6 +1282,15 @@
                 <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
                 <para>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.</para>
               </throws>
+ <notes>
+ <para>
+ 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
+ <methodname>erase_return_void</methodname> which will be faster.
+ </para>
+ </notes>
             </method>
             <method name="erase">
               <parameter name="k">
@@ -1287,6 +1326,27 @@
                 <para>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.</para>
               </throws>
             </method>
+ <method name="erase_return_void">
+ <parameter name="position">
+ <paramtype>const_iterator</paramtype>
+ </parameter>
+ <type>iterator</type>
+ <description>
+ <para>Erase the element pointed to by <code>position</code>.</para>
+ </description>
+ <throws>
+ <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
+ <para>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.</para>
+ </throws>
+ <notes>
+ <para>
+ This is a temporary workaround for the inefficient
+ <methodname>erase</methodname> method. Hopefully, in a future
+ version the signature of <methodname>erase</methodname> will
+ be changed and this will be deprecated.
+ </para>
+ </notes>
+ </method>
             <method name="clear">
               <type>void</type>
               <description>
@@ -2059,6 +2119,15 @@
                 <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
                 <para>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.</para>
               </throws>
+ <notes>
+ <para>
+ 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
+ <methodname>erase_return_void</methodname> which will be faster.
+ </para>
+ </notes>
             </method>
             <method name="erase">
               <parameter name="k">
@@ -2094,6 +2163,27 @@
                 <para>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.</para>
               </throws>
             </method>
+ <method name="erase_return_void">
+ <parameter name="position">
+ <paramtype>const_iterator</paramtype>
+ </parameter>
+ <type>iterator</type>
+ <description>
+ <para>Erase the element pointed to by <code>position</code>.</para>
+ </description>
+ <throws>
+ <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
+ <para>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.</para>
+ </throws>
+ <notes>
+ <para>
+ This is a temporary workaround for the inefficient
+ <methodname>erase</methodname> method. Hopefully, in a future
+ version the signature of <methodname>erase</methodname> will
+ be changed and this will be deprecated.
+ </para>
+ </notes>
+ </method>
             <method name="clear">
               <type>void</type>
               <description>
@@ -2901,6 +2991,15 @@
                 <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
                 <para>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.</para>
               </throws>
+ <notes>
+ <para>
+ 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
+ <methodname>erase_return_void</methodname> which will be faster.
+ </para>
+ </notes>
             </method>
             <method name="erase">
               <parameter name="k">
@@ -2936,6 +3035,27 @@
                 <para>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.</para>
               </throws>
             </method>
+ <method name="erase_return_void">
+ <parameter name="position">
+ <paramtype>const_iterator</paramtype>
+ </parameter>
+ <type>iterator</type>
+ <description>
+ <para>Erase the element pointed to by <code>position</code>.</para>
+ </description>
+ <throws>
+ <para>Only throws an exception if it is thrown by <code>hasher</code> or <code>key_equal</code>.</para>
+ <para>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.</para>
+ </throws>
+ <notes>
+ <para>
+ This is a temporary workaround for the inefficient
+ <methodname>erase</methodname> method. Hopefully, in a future
+ version the signature of <methodname>erase</methodname> will
+ be changed and this will be deprecated.
+ </para>
+ </notes>
+ </method>
             <method name="clear">
               <type>void</type>
               <description>

Modified: trunk/libs/unordered/test/unordered/erase_tests.cpp
==============================================================================
--- trunk/libs/unordered/test/unordered/erase_tests.cpp (original)
+++ trunk/libs/unordered/test/unordered/erase_tests.cpp 2009-12-15 17:52:52 EST (Tue, 15 Dec 2009)
@@ -112,6 +112,55 @@
         BOOST_TEST(x.erase(x.begin(), x.end()) == x.begin());
     }
 
+ std::cerr<<"erase_return_void(begin()).\n";
+ {
+ test::random_values<Container> 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<Container>(*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<Container> 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<Container>(*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<Container> v(500, generator);


Boost-Commit list run by bdawes at acm.org, david.abrahams at rcn.com, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk