From 0a03aa599a71559c88792e9c758447ec545f1739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Thu, 14 Aug 2014 00:22:19 +0200 Subject: [PATCH] Reimplemented remove family for lists using new stable_partition algorithm. This allows less code duplication between lists with different options (constant_time_size, cache_list, safe/non-safe hooks, etc.). --- .../intrusive/circular_list_algorithms.hpp | 82 ++++++++++++++++--- .../detail/common_slist_algorithms.hpp | 75 +++++++++++++++++ .../boost/intrusive/detail/generic_hook.hpp | 7 +- include/boost/intrusive/detail/utilities.hpp | 5 ++ include/boost/intrusive/list.hpp | 31 ++++--- include/boost/intrusive/slist.hpp | 37 ++++++--- test/common_functors.hpp | 8 ++ test/itestvalue.hpp | 8 ++ test/list_test.cpp | 25 +++++- test/slist_test.cpp | 20 +++++ 10 files changed, 257 insertions(+), 41 deletions(-) diff --git a/include/boost/intrusive/circular_list_algorithms.hpp b/include/boost/intrusive/circular_list_algorithms.hpp index 9d6213a..3253f15 100644 --- a/include/boost/intrusive/circular_list_algorithms.hpp +++ b/include/boost/intrusive/circular_list_algorithms.hpp @@ -64,8 +64,9 @@ class circular_list_algorithms //! Throws: Nothing. static void init(const node_ptr &this_node) { - NodeTraits::set_next(this_node, node_ptr()); - NodeTraits::set_previous(this_node, node_ptr()); + const node_ptr null_node((node_ptr())); + NodeTraits::set_next(this_node, null_node); + NodeTraits::set_previous(this_node, null_node); } //! Effects: Returns true is "this_node" is in a non-used state @@ -135,15 +136,10 @@ class circular_list_algorithms static node_ptr unlink(const node_ptr &this_node) { node_ptr next(NodeTraits::get_next(this_node)); - if(next){ - node_ptr prev(NodeTraits::get_previous(this_node)); - NodeTraits::set_next(prev, next); - NodeTraits::set_previous(next, prev); - return next; - } - else{ - return this_node; - } + node_ptr prev(NodeTraits::get_previous(this_node)); + NodeTraits::set_next(prev, next); + NodeTraits::set_previous(next, prev); + return next; } //! Requires: b and e must be nodes of the same circular list or an empty range. @@ -404,6 +400,70 @@ class circular_list_algorithms } link_after(last, p); } + + struct stable_partition_info + { + std::size_t num_1st_partition; + std::size_t num_2nd_partition; + node_ptr beg_2st_partition; + }; + + template + static void stable_partition(node_ptr beg, const node_ptr &end, Pred pred, stable_partition_info &info) + { + node_ptr bcur = node_traits::get_previous(beg); + node_ptr cur = beg; + node_ptr new_f = end; + + std::size_t num1 = 0, num2 = 0; + while(cur != end){ + if(pred(cur)){ + ++num1; + bcur = cur; + cur = node_traits::get_next(cur); + } + else{ + ++num2; + node_ptr last_to_remove = bcur; + new_f = cur; + bcur = cur; + cur = node_traits::get_next(cur); + BOOST_TRY{ + //Main loop + while(cur != end){ + if(pred(cur)){ //Might throw + ++num1; + //Process current node + node_traits::set_next (last_to_remove, cur); + node_traits::set_previous(cur, last_to_remove); + last_to_remove = cur; + node_ptr nxt = node_traits::get_next(cur); + node_traits::set_next (bcur, nxt); + node_traits::set_previous(nxt, bcur); + cur = nxt; + } + else{ + ++num2; + bcur = cur; + cur = node_traits::get_next(cur); + } + } + } + BOOST_CATCH(...){ + node_traits::set_next (last_to_remove, new_f); + node_traits::set_previous(new_f, last_to_remove); + throw; + } + BOOST_CATCH_END + node_traits::set_next(last_to_remove, new_f); + node_traits::set_previous(new_f, last_to_remove); + break; + } + } + info.num_1st_partition = num1; + info.num_2nd_partition = num2; + info.beg_2st_partition = new_f; + } }; /// @cond diff --git a/include/boost/intrusive/detail/common_slist_algorithms.hpp b/include/boost/intrusive/detail/common_slist_algorithms.hpp index 0adbd50..316e5a1 100644 --- a/include/boost/intrusive/detail/common_slist_algorithms.hpp +++ b/include/boost/intrusive/detail/common_slist_algorithms.hpp @@ -16,6 +16,7 @@ #include #include #include +#include #include namespace boost { @@ -88,9 +89,83 @@ class common_slist_algorithms NodeTraits::set_next(bp, next_b); } } + + struct stable_partition_info + { + std::size_t num_1st_partition; + std::size_t num_2nd_partition; + node_ptr beg_2st_partition; + node_ptr new_last_node; + }; + + template + static void stable_partition(node_ptr before_beg, const node_ptr &end, Pred pred, stable_partition_info &info) + { + node_ptr bcur = before_beg; + node_ptr cur = node_traits::get_next(bcur); + node_ptr new_f = end; + + std::size_t num1 = 0, num2 = 0; + while(cur != end){ + if(pred(cur)){ + ++num1; + bcur = cur; + cur = node_traits::get_next(cur); + } + else{ + ++num2; + node_ptr last_to_remove = bcur; + new_f = cur; + bcur = cur; + cur = node_traits::get_next(cur); + BOOST_TRY{ + //Main loop + while(cur != end){ + if(pred(cur)){ //Might throw + ++num1; + //Process current node + node_traits::set_next(last_to_remove, cur); + last_to_remove = cur; + node_ptr nxt = node_traits::get_next(cur); + node_traits::set_next(bcur, nxt); + cur = nxt; + } + else{ + ++num2; + bcur = cur; + cur = node_traits::get_next(cur); + } + } + } + BOOST_CATCH(...){ + node_traits::set_next(last_to_remove, new_f); + throw; + } + BOOST_CATCH_END + node_traits::set_next(last_to_remove, new_f); + break; + } + } + info.num_1st_partition = num1; + info.num_2nd_partition = num2; + info.beg_2st_partition = new_f; + info.new_last_node = bcur; + } }; +/// @endcond + } //namespace detail + +/// @cond + +template +struct get_algo +{ + typedef detail::common_slist_algorithms type; +}; + + } //namespace intrusive } //namespace boost diff --git a/include/boost/intrusive/detail/generic_hook.hpp b/include/boost/intrusive/detail/generic_hook.hpp index 835a8c7..a827678 100644 --- a/include/boost/intrusive/detail/generic_hook.hpp +++ b/include/boost/intrusive/detail/generic_hook.hpp @@ -177,8 +177,11 @@ class generic_hook void unlink() { BOOST_STATIC_ASSERT(( (int)hooktags::link_mode == (int)auto_unlink )); - node_algorithms::unlink(this->this_ptr()); - node_algorithms::init(this->this_ptr()); + node_ptr n(this->this_ptr()); + if(!node_algorithms::inited(n)){ + node_algorithms::unlink(n); + node_algorithms::init(n); + } } }; diff --git a/include/boost/intrusive/detail/utilities.hpp b/include/boost/intrusive/detail/utilities.hpp index 01b84ba..124e2ab 100644 --- a/include/boost/intrusive/detail/utilities.hpp +++ b/include/boost/intrusive/detail/utilities.hpp @@ -40,6 +40,7 @@ enum algo_types CircularListAlgorithms, CircularSListAlgorithms, LinearSListAlgorithms, + CommonSListAlgorithms, BsTreeAlgorithms, RbTreeAlgorithms, AvlTreeAlgorithms, @@ -219,6 +220,10 @@ struct key_nodeptr_comp bool operator()(const KeyType &key1, const KeyType2 &key2) const { return base_t::get()(this->key_forward(key1), this->key_forward(key2)); } + template + bool operator()(const KeyType &key1) const + { return base_t::get()(this->key_forward(key1)); } + const ValueTraits *const traits_; }; diff --git a/include/boost/intrusive/list.hpp b/include/boost/intrusive/list.hpp index cea73fe..3271dbf 100644 --- a/include/boost/intrusive/list.hpp +++ b/include/boost/intrusive/list.hpp @@ -1081,7 +1081,17 @@ class list_impl //! and iterators to elements that are not removed remain valid. template void remove_if(Pred pred) - { this->remove_and_dispose_if(pred, detail::null_disposer()); } + { + const node_ptr root_node = this->get_root_node(); + typename node_algorithms::stable_partition_info info; + node_algorithms::stable_partition + (node_traits::get_next(root_node), root_node, detail::key_nodeptr_comp(pred, &this->priv_value_traits()), info); + //Invariants preserved by stable_partition so erase can be safely called + //The first element might have changed so calculate it again + this->erase( const_iterator(node_traits::get_next(root_node), this->priv_value_traits_ptr()) + , const_iterator(info.beg_2st_partition, this->priv_value_traits_ptr()) + , info.num_1st_partition); + } //! Requires: Disposer::operator()(pointer) shouldn't throw. //! @@ -1098,16 +1108,15 @@ class list_impl template void remove_and_dispose_if(Pred pred, Disposer disposer) { - const_iterator cur(this->cbegin()); - const_iterator last(this->cend()); - while(cur != last) { - if(pred(*cur)){ - cur = this->erase_and_dispose(cur, disposer); - } - else{ - ++cur; - } - } + const node_ptr root_node = this->get_root_node(); + typename node_algorithms::stable_partition_info info; + node_algorithms::stable_partition + (node_traits::get_next(root_node), root_node, detail::key_nodeptr_comp(pred, &this->priv_value_traits()), info); + //Invariants preserved by stable_partition so erase can be safely called + //The first element might have changed so calculate it again + this->erase_and_dispose( const_iterator(node_traits::get_next(root_node), this->priv_value_traits_ptr()) + , const_iterator(info.beg_2st_partition, this->priv_value_traits_ptr()) + , disposer); } //! Effects: Removes adjacent duplicate elements or adjacent diff --git a/include/boost/intrusive/slist.hpp b/include/boost/intrusive/slist.hpp index 39837f4..9755919 100644 --- a/include/boost/intrusive/slist.hpp +++ b/include/boost/intrusive/slist.hpp @@ -1538,7 +1538,20 @@ class slist_impl //! and iterators to elements that are not removed remain valid. template void remove_if(Pred pred) - { this->remove_and_dispose_if(pred, detail::null_disposer()); } + { + const node_ptr bbeg = this->get_root_node(); + typename node_algorithms::stable_partition_info info; + node_algorithms::stable_partition + (bbeg, this->get_end_node(), detail::key_nodeptr_comp(pred, &this->priv_value_traits()), info); + //After cache last is set, slist invariants are preserved... + if(cache_last){ + this->set_last_node(info.new_last_node); + } + //...so erase can be safely called + this->erase_after( const_iterator(bbeg, this->priv_value_traits_ptr()) + , const_iterator(info.beg_2st_partition, this->priv_value_traits_ptr()) + , info.num_1st_partition); + } //! Requires: Disposer::operator()(pointer) shouldn't throw. //! @@ -1555,20 +1568,18 @@ class slist_impl template void remove_and_dispose_if(Pred pred, Disposer disposer) { - const_iterator bcur(this->before_begin()), cur(this->begin()), e(this->end()); - - while(cur != e){ - if (pred(*cur)){ - cur = this->erase_after_and_dispose(bcur, disposer); - } - else{ - bcur = cur; - ++cur; - } - } + const node_ptr bbeg = this->get_root_node(); + typename node_algorithms::stable_partition_info info; + node_algorithms::stable_partition + (bbeg, this->get_end_node(), detail::key_nodeptr_comp(pred, &this->priv_value_traits()), info); + //After cache last is set, slist invariants are preserved... if(cache_last){ - this->set_last_node(bcur.pointed_node()); + this->set_last_node(info.new_last_node); } + //...so erase can be safely called + this->erase_after_and_dispose( const_iterator(bbeg, this->priv_value_traits_ptr()) + , const_iterator(info.beg_2st_partition, this->priv_value_traits_ptr()) + , disposer); } //! Effects: Removes adjacent duplicate elements or adjacent diff --git a/test/common_functors.hpp b/test/common_functors.hpp index a6af9fb..9500a52 100644 --- a/test/common_functors.hpp +++ b/test/common_functors.hpp @@ -50,6 +50,14 @@ class new_default_factory { return new T(); } }; +class empty_disposer +{ + public: + template + void operator()(const T &) + {} +}; + } //namespace test { } //namespace intrusive { } //namespace boost { diff --git a/test/itestvalue.hpp b/test/itestvalue.hpp index 427bfaf..7935040 100644 --- a/test/itestvalue.hpp +++ b/test/itestvalue.hpp @@ -163,6 +163,14 @@ struct is_even { return ((&v1)->value_ & 1) == 0; } }; +struct is_odd +{ + template + bool operator() + (const value_type& v1) const + { return ((&v1)->value_ & 1) != 0; } +}; + template struct Value_Container; diff --git a/test/list_test.cpp b/test/list_test.cpp index 397165c..b962bc8 100644 --- a/test/list_test.cpp +++ b/test/list_test.cpp @@ -21,7 +21,6 @@ #include #include "test_macros.hpp" #include "test_container.hpp" -#include #include using namespace boost::intrusive; @@ -38,8 +37,7 @@ struct hooks typedef list_member_hook< link_mode , void_pointer > auto_member_hook_type; typedef nonhook_node_member< list_node_traits< VoidPointer >, - circular_list_algorithms - > nonhook_node_member_type; + circular_list_algorithms > nonhook_node_member_type; }; @@ -145,7 +143,26 @@ void test_list< List_Type, Value_Container > TEST_INTRUSIVE_SEQUENCE( init_values, list.begin() ); } { - Value_Container values2(values); // NOTE: problematic copy of value container + list_type list(values.begin(), values.end()); + list.remove_if(is_odd()); + int init_values [] = { 2, 4 }; + TEST_INTRUSIVE_SEQUENCE( init_values, list.begin() ); + } + { + list_type list(values.begin(), values.end()); + list.remove_and_dispose_if(is_even(), test::empty_disposer()); + int init_values [] = { 1, 3, 5 }; + TEST_INTRUSIVE_SEQUENCE( init_values, list.begin() ); + } + { + list_type list(values.begin(), values.end()); + list.remove_and_dispose_if(is_odd(), test::empty_disposer()); + int init_values [] = { 2, 4 }; + typename list_type::iterator i = list.begin(), e = list.end(); + TEST_INTRUSIVE_SEQUENCE( init_values, list.begin() ); + } + { + Value_Container values2(values); list_type list(values.begin(), values.end()); list.insert(list.end(), values2.begin(), values2.end()); list.sort(); diff --git a/test/slist_test.cpp b/test/slist_test.cpp index f455243..228c3e0 100644 --- a/test/slist_test.cpp +++ b/test/slist_test.cpp @@ -159,6 +159,26 @@ void test_slist< List_Type, Value_Container > int init_values [] = { 1, 3, 5 }; TEST_INTRUSIVE_SEQUENCE( init_values, list.begin() ); } + { + list_type list(values.begin(), values.end()); + list.remove_if(is_odd()); + int init_values [] = { 2, 4 }; + TEST_INTRUSIVE_SEQUENCE( init_values, list.begin() ); + } + { + list_type list(values.begin(), values.end()); + list.remove_and_dispose_if(is_even(), test::empty_disposer()); + int init_values [] = { 1, 3, 5 }; + typename list_type::iterator i = list.begin(), e = list.end(); + TEST_INTRUSIVE_SEQUENCE( init_values, list.begin() ); + } + { + list_type list(values.begin(), values.end()); + list.remove_and_dispose_if(is_odd(), test::empty_disposer()); + int init_values [] = { 2, 4 }; + typename list_type::iterator i = list.begin(), e = list.end(); + TEST_INTRUSIVE_SEQUENCE( init_values, list.begin() ); + } { Value_Container values2(values); list_type list(values.begin(), values.end());