From 21e673c697fd5a434a3a9f53636218e1d4978ba0 Mon Sep 17 00:00:00 2001 From: Christian Mazakas Date: Thu, 9 Feb 2023 15:59:23 -0800 Subject: [PATCH] Begin cleanup of node_handle implementation --- include/boost/unordered/detail/foa.hpp | 96 ++++++++----------- .../boost/unordered/unordered_node_map.hpp | 30 +++--- .../boost/unordered/unordered_node_set.hpp | 27 +++--- test/unordered/node_handle_tests.cpp | 1 - 4 files changed, 75 insertions(+), 79 deletions(-) diff --git a/include/boost/unordered/detail/foa.hpp b/include/boost/unordered/detail/foa.hpp index 710d2324..2f101c8b 100644 --- a/include/boost/unordered/detail/foa.hpp +++ b/include/boost/unordered/detail/foa.hpp @@ -92,32 +92,32 @@ struct insert_return_type NodeType node; }; -template +template struct node_handle_base { protected: - using type_policy=NodeTypes; + using type_policy=TypePolicy; + using value_type=typename type_policy::value_type; using element_type=typename type_policy::element_type; public: using allocator_type = Allocator; private: - alignas(element_type) unsigned char x_[sizeof(element_type)]; - alignas(Allocator) unsigned char a_[sizeof(Allocator)]; - bool empty_=true; + value_type* p_=nullptr; + alignas(Allocator) unsigned char a_[sizeof(Allocator)]={0}; protected: - element_type& element()noexcept + value_type& element()noexcept { BOOST_ASSERT(!empty()); - return *reinterpret_cast(x_); + return *p_; } - element_type const& element()const noexcept + value_type const& element()const noexcept { BOOST_ASSERT(!empty()); - return *reinterpret_cast(x_); + return *p_; } Allocator& al()noexcept @@ -132,13 +132,23 @@ struct node_handle_base return *reinterpret_cast(a_); } - void emplace(element_type x,Allocator a) + void emplace(value_type* p,Allocator a) { BOOST_ASSERT(empty()); - - new(x_)element_type(std::move(x)); + p_=p; new(a_)Allocator(a); - empty_=false; + } + + void emplace(element_type&& x,Allocator a) + { + emplace(x.p,a); + x.p=nullptr; + } + + void clear() + { + al().~Allocator(); + p_=nullptr; } public: @@ -147,16 +157,8 @@ struct node_handle_base node_handle_base(node_handle_base&& nh) noexcept { if (!nh.empty()){ - // neither of these move constructors are allowed to throw exceptions - // so we can get away with rote placement new - // - new(a_)Allocator(std::move(nh.al())); - new(x_)element_type(std::move(nh.element())); - empty_=false; - - reinterpret_cast(nh.x_)->~element_type(); - reinterpret_cast(nh.a_)->~Allocator(); - nh.empty_ = true; + emplace(nh.p_,nh.al()); + nh.clear(); } } @@ -167,22 +169,19 @@ struct node_handle_base Allocator>::type::value; if(!empty()){ - type_policy::destroy(al(),reinterpret_cast(x_)); - reinterpret_cast(x_)->~element_type(); + type_policy::destroy(al(),p_); if (pocma&&!nh.empty()){al()=std::move(nh.al());} } if(!nh.empty()){ - new(x_)element_type(std::move(nh.element())); if(empty()){new(a_)Allocator(std::move(nh.al()));} - empty_=false; + p_=nh.p_; - reinterpret_cast(nh.x_)->~element_type(); + nh.p_=nullptr; reinterpret_cast(nh.a_)->~Allocator(); - nh.empty_=true; }else if (!empty()){ reinterpret_cast(a_)->~Allocator(); - empty_=true; + p_=nullptr; } return *this; @@ -191,16 +190,14 @@ struct node_handle_base ~node_handle_base() { if(!empty()){ - type_policy::destroy(al(),reinterpret_cast(x_)); - reinterpret_cast(x_)->~element_type(); + type_policy::destroy(al(),p_); reinterpret_cast(a_)->~Allocator(); - empty_=true; } } allocator_type get_allocator()const noexcept{return al();} explicit operator bool()const noexcept{ return !empty();} - BOOST_ATTRIBUTE_NODISCARD bool empty()const noexcept{return empty_;} + BOOST_ATTRIBUTE_NODISCARD bool empty()const noexcept{return p_==nullptr;} void swap(node_handle_base& nh) noexcept( boost::allocator_is_always_equal::type::value|| @@ -214,7 +211,9 @@ struct node_handle_base if (!empty()&&!nh.empty()){ BOOST_ASSERT(pocs || al()==nh.al()); - element().swap(nh.element()); + value_type *p=p_; + p_=nh.p_; + nh.p_=p; if(pocs){ swap(al(),nh.al()); @@ -226,21 +225,11 @@ struct node_handle_base if (empty()&&nh.empty()){return;} if (empty()){ - new(x_)element_type(std::move(nh.element())); - new(a_)Allocator(nh.al()); - empty_=false; - - reinterpret_cast(nh.x_)->~element_type(); - reinterpret_cast(nh.a_)->~Allocator(); - nh.empty_=true; + emplace(nh.p_,nh.al()); + nh.clear(); }else{ - new(nh.x_)element_type(std::move(element())); - new(nh.a_)Allocator(al()); - nh.empty_=false; - - reinterpret_cast(x_)->~element_type(); - reinterpret_cast(a_)->~Allocator(); - empty_=true; + nh.emplace(p_,al()); + clear(); } } @@ -1668,13 +1657,12 @@ public: } } - element_type extract(const_iterator pos)noexcept + element_type extract(const_iterator pos) { BOOST_ASSERT(pos!=end()); - element_type x=std::move(*pos.p); - destroy_element(pos.p); - recover_slot(pos.pc); - return x; + erase_on_exit e{*this,iterator{const_iterator_cast_tag{},pos}}; + (void)e; + return std::move(*pos.p); } // TODO: should we accept different allocator too? diff --git a/include/boost/unordered/unordered_node_map.hpp b/include/boost/unordered/unordered_node_map.hpp index 271fbd2c..9a0f293e 100644 --- a/include/boost/unordered/unordered_node_map.hpp +++ b/include/boost/unordered/unordered_node_map.hpp @@ -60,10 +60,6 @@ namespace boost { p = rhs.p; rhs.p = nullptr; } - - void swap(element_type& rhs) noexcept { - std::swap(p, rhs.p); - } }; static value_type& value_from(element_type const& x) { return *(x.p); } @@ -118,14 +114,19 @@ namespace boost { BOOST_CATCH_END } + template static void destroy(A& al, value_type* p) noexcept + { + boost::allocator_destroy(al, p); + boost::allocator_deallocate(al, + boost::pointer_traits< + typename boost::allocator_pointer::type>::pointer_to(*p), + 1); + } + template static void destroy(A& al, element_type* p) noexcept { if (p->p) { - boost::allocator_destroy(al, p->p); - boost::allocator_deallocate(al, - boost::pointer_traits< - typename boost::allocator_pointer::type>::pointer_to(*p->p), - 1); + destroy(al,p->p); } } }; @@ -159,14 +160,13 @@ namespace boost { key_type& key() const { BOOST_ASSERT(!empty()); - return const_cast(type_policy::extract(element())); + return const_cast(element().first); } mapped_type& mapped() const { BOOST_ASSERT(!empty()); - return const_cast( - type_policy::value_from(element()).second); + return const_cast(element().second); } }; } // namespace detail @@ -405,8 +405,12 @@ namespace boost { BOOST_ASSERT(get_allocator() == nh.get_allocator()); - auto itp = table_.emplace_impl(map_types::move(nh.element())); + typename map_types::element_type x; + x.p=std::addressof(nh.element()); + + auto itp = table_.emplace_impl(std::move(x)); if (itp.second) { + nh.clear(); return {itp.first, true, node_type{}}; } else { return {itp.first, false, std::move(nh)}; diff --git a/include/boost/unordered/unordered_node_set.hpp b/include/boost/unordered/unordered_node_set.hpp index bf97c85e..5adf0f37 100644 --- a/include/boost/unordered/unordered_node_set.hpp +++ b/include/boost/unordered/unordered_node_set.hpp @@ -56,10 +56,6 @@ namespace boost { p = rhs.p; rhs.p = nullptr; } - - void swap(element_type& rhs) noexcept { - std::swap(p, rhs.p); - } }; static value_type& value_from(element_type const& x) { return *x.p; } @@ -100,14 +96,19 @@ namespace boost { BOOST_CATCH_END } + template static void destroy(A& al, value_type* p) noexcept + { + boost::allocator_destroy(al, p); + boost::allocator_deallocate(al, + boost::pointer_traits< + typename boost::allocator_pointer::type>::pointer_to(*p), + 1); + } + template static void destroy(A& al, element_type* p) noexcept { if (p->p) { - boost::allocator_destroy(al, p->p); - boost::allocator_deallocate(al, - boost::pointer_traits< - typename boost::allocator_pointer::type>::pointer_to(*p->p), - 1); + destroy(al, p->p); } } }; @@ -139,7 +140,7 @@ namespace boost { value_type& value() const { BOOST_ASSERT(!empty()); - return const_cast(type_policy::extract(element())); + return const_cast(element()); } }; } // namespace detail @@ -392,8 +393,12 @@ namespace boost { BOOST_ASSERT(get_allocator() == nh.get_allocator()); - auto itp = table_.emplace_impl(set_types::move(nh.element())); + typename set_types::element_type x; + x.p=std::addressof(nh.element()); + + auto itp = table_.emplace_impl(std::move(x)); if (itp.second) { + nh.clear(); return {itp.first, true, node_type{}}; } else { return {itp.first, false, std::move(nh)}; diff --git a/test/unordered/node_handle_tests.cpp b/test/unordered/node_handle_tests.cpp index ad0aa1e1..9787d83a 100644 --- a/test/unordered/node_handle_tests.cpp +++ b/test/unordered/node_handle_tests.cpp @@ -119,7 +119,6 @@ static void failed_insertion_with_hint() typename Set::node_type nh = src.extract(10); - std::cout << "performing relevant test now" << std::endl; BOOST_TEST(dst.insert(dst.find(10), boost::move(nh)) == dst.find(10)); BOOST_TEST(nh); BOOST_TEST(!nh.empty());