From: khizmax Date: Sat, 25 Jul 2015 09:45:32 +0000 (+0300) Subject: SkipList: X-Git-Tag: v2.1.0~173 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=9f4651a5d26ac3b768c57e54c643988fb4eab9ef;p=libcds.git SkipList: - fixed memory order - TSan checking - replaced ensure() function with update() - small optimizations --- diff --git a/cds/container/impl/skip_list_map.h b/cds/container/impl/skip_list_map.h index 29987199..80e1ccc5 100644 --- a/cds/container/impl/skip_list_map.h +++ b/cds/container/impl/skip_list_map.h @@ -295,49 +295,53 @@ namespace cds { namespace container { return false; } - /// Ensures that the \p key exists in the map + /// Updates data by \p key /** The operation performs inserting or changing data with lock-free manner. If the \p key not found in the map, then the new item created from \p key - is inserted into the map (note that in this case the \ref key_type should be - constructible from type \p K). - Otherwise, the functor \p func is called with item found. - The functor \p Func may be a function with signature: - \code - void func( bool bNew, value_type& item ); - \endcode - or a functor: + will be inserted into the map iff \p bInsert is \p true + (note that in this case the \ref key_type should be constructible from type \p K). + Otherwise, if \p key is found, the functor \p func is called with item found. + The functor \p Func signature: \code struct my_functor { void operator()( bool bNew, value_type& item ); }; \endcode - - with arguments: + where: - \p bNew - \p true if the item has been inserted, \p false otherwise - - \p item - item of the list + - \p item - item of the map The functor may change any fields of the \p item.second that is \ref value_type. - Returns std::pair where \p first is true if operation is successfull, - \p second is true if new item has been added or \p false if the item with \p key - already is in the list. + Returns std::pair where \p first is \p true if operation is successfull, + \p second is \p true if new item has been added or \p false if \p key already exists. @warning See \ref cds_intrusive_item_creating "insert item troubleshooting" */ template - std::pair ensure( K const& key, Func func ) + std::pair update( K const& key, Func func, bool bInsert = true ) { scoped_node_ptr pNode( node_allocator().New( random_level(), key )); - std::pair res = base_class::ensure( *pNode, - [&func](bool bNew, node_type& item, node_type const& ){ func( bNew, item.m_Value ); } + std::pair res = base_class::update( *pNode, + [&func](bool bNew, node_type& item, node_type const& ){ func( bNew, item.m_Value );}, + bInsert ); if ( res.first && res.second ) pNode.release(); return res; } + //@cond + // Deprecated, use update() + template + std::pair ensure( K const& key, Func func ) + { + return update( key, func, true ); + } + //@endcond + /// Delete \p key from the map /** \anchor cds_nonintrusive_SkipListMap_erase_val diff --git a/cds/container/impl/skip_list_set.h b/cds/container/impl/skip_list_set.h index 0935780b..a310d780 100644 --- a/cds/container/impl/skip_list_set.h +++ b/cds/container/impl/skip_list_set.h @@ -250,48 +250,56 @@ namespace cds { namespace container { return false; } - /// Ensures that the item exists in the set + /// Updates the item /** The operation performs inserting or changing data with lock-free manner. If the \p val key not found in the set, then the new item created from \p val - is inserted into the set. Otherwise, the functor \p func is called with the item found. - The functor \p Func should be a function with signature: - \code - void func( bool bNew, value_type& item, const Q& val ); - \endcode - or a functor: + will be inserted into the set iff \p bInsert is \p true. + Otherwise, if \p val is found, the functor \p func will be called with the item found. + + The functor \p Func signature: \code struct my_functor { void operator()( bool bNew, value_type& item, const Q& val ); }; \endcode - - with arguments: + where: - \p bNew - \p true if the item has been inserted, \p false otherwise - \p item - item of the set - - \p val - argument \p key passed into the \p %ensure() function + - \p val - argument \p key passed into the \p %update() function The functor may change non-key fields of the \p item; however, \p func must guarantee that during changing no any other modifications could be made on this item by concurrent threads. - Returns std::pair where \p first is true if operation is successfull, - \p second is true if new item has been added or \p false if the item with \p key - already is in the set. + Returns std::pair where \p first is \p true if operation is successfull, + i.e. the item has been inserted or updated, + \p second is \p true if new item has been added or \p false if the item with key equal to \p val + already exists. @warning See \ref cds_intrusive_item_creating "insert item troubleshooting" */ template - std::pair ensure( const Q& val, Func func ) + std::pair update( const Q& val, Func func, bool bInsert = true ) { scoped_node_ptr sp( node_allocator().New( random_level(), val )); - std::pair bRes = base_class::ensure( *sp, - [&func, &val](bool bNew, node_type& node, node_type&){ func( bNew, node.m_Value, val ); }); + std::pair bRes = base_class::update( *sp, + [&func, &val](bool bNew, node_type& node, node_type&){ func( bNew, node.m_Value, val ); }, + bInsert ); if ( bRes.first && bRes.second ) sp.release(); return bRes; } + //@cond + // Deprecated, use update() + template + std::pair ensure( const Q& val, Func func ) + { + return update( val, func, true ); + } + //@endcond + /// Inserts data of type \p value_type created in-place from std::forward(args)... /** Returns \p true if inserting successful, \p false otherwise. diff --git a/cds/container/skip_list_map_nogc.h b/cds/container/skip_list_map_nogc.h index 57e1c5ad..932fbea8 100644 --- a/cds/container/skip_list_map_nogc.h +++ b/cds/container/skip_list_map_nogc.h @@ -235,21 +235,30 @@ namespace cds { namespace container { return base_class::emplace( std::forward(key), std::move(mapped_type(std::forward(args)...))); } - /// Ensures that the key \p key exists in the map + /// UPdates data by \p key /** - The operation inserts new item if the key \p key is not found in the map. - Otherwise, the function returns an iterator that points to item found. + The operation inserts new item if \p key is not found in the map and \p bInsert is \p true. + Otherwise, if \p key is found, the function returns an iterator that points to item found. Returns std::pair where \p first is an iterator pointing to - item found or inserted, \p second is true if new item has been added or \p false if the item - already is in the list. + item found or inserted or \p end() if \p key is not found and insertion is not allowed (\p bInsert is \p false), + \p second is \p true if new item has been added or \p false if the item already exists. */ template - std::pair ensure( K const& key ) + std::pair update( K const& key, bool bInsert = true ) { //TODO: pass arguments by reference (make_pair makes copy) - return base_class::ensure( std::make_pair( key, mapped_type() )); + return base_class::update( std::make_pair( key, mapped_type() ), bInsert ); + } + + //@cond + // Deprecated, use update() + template + std::pair ensure( K const& key ) + { + return update( key, true ); } + //@endcond /// Finds the key \p key /** \anchor cds_nonintrusive_SkipListMap_nogc_find_val diff --git a/cds/container/skip_list_map_rcu.h b/cds/container/skip_list_map_rcu.h index 8b78ba81..70185d7f 100644 --- a/cds/container/skip_list_map_rcu.h +++ b/cds/container/skip_list_map_rcu.h @@ -326,46 +326,55 @@ namespace cds { namespace container { return false; } - /// Ensures that the \p key exists in the map + /// Updates data by \p key /** The operation performs inserting or changing data with lock-free manner. If the \p key not found in the map, then the new item created from \p key - is inserted into the map (note that in this case the \ref key_type should be - constructible from type \p K). - Otherwise, the functor \p func is called with item found. + is inserted into the map iff \p bInsert is \p true. + Otherwise, if \p key found, the functor \p func is called with item found. The functor \p Func interface is: \code struct my_functor { void operator()( bool bNew, value_type& item ); }; \endcode - with arguments: + where: - \p bNew - \p true if the item has been inserted, \p false otherwise - - \p item - item of the list + - \p item - item of the map The functor may change any fields of \p item.second. RCU \p synchronize() method can be called. RCU should not be locked. - Returns std::pair where \p first is true if operation is successfull, - \p second is true if new item has been added or \p false if the item with \p key - already is in the list. + Returns std::pair where \p first is \p true if operation is successfull, + \p second is \p true if new item has been added or \p false if the item with \p key + already exists. @warning See \ref cds_intrusive_item_creating "insert item troubleshooting" */ template - std::pair ensure( K const& key, Func func ) + std::pair update( K const& key, Func func, bool bInsert = true ) { scoped_node_ptr pNode( node_allocator().New( random_level(), key )); - std::pair res = base_class::ensure( *pNode, - [&func](bool bNew, node_type& item, node_type const& ){ func( bNew, item.m_Value ); } + std::pair res = base_class::update( *pNode, + [&func](bool bNew, node_type& item, node_type const& ){ func( bNew, item.m_Value );}, + bInsert ); if ( res.first && res.second ) pNode.release(); return res; } + //@cond + // Deprecated, use update() + template + std::pair ensure( K const& key, Func func ) + { + return update( key, func, true ); + } + //@endcond + /// Delete \p key from the map /**\anchor cds_nonintrusive_SkipListMap_rcu_erase_val diff --git a/cds/container/skip_list_set_nogc.h b/cds/container/skip_list_set_nogc.h index 24f8171b..0d6f8850 100644 --- a/cds/container/skip_list_set_nogc.h +++ b/cds/container/skip_list_set_nogc.h @@ -267,27 +267,39 @@ namespace cds { namespace container { return end(); } - /// Ensures that the item \p val exists in the set + /// Updates the item /** - The operation inserts new item if the key \p val is not found in the set. - Otherwise, the function returns an iterator that points to item found. + The operation inserts new item if \p val is not found in the set and \p bInsert is \p true. + Otherwise, if that key exists, the function returns an iterator that points to item found. - Returns std::pair where \p first is an iterator pointing to - item found or inserted, \p second is true if new item has been added or \p false if the item + Returns std::pair where \p first is an iterator pointing to + item found or inserted or \p end() if \p val is not found and \p bInsert is \p false, + \p second is \p true if new item has been added or \p false if the item already is in the set. */ template - std::pair ensure( const Q& val ) + std::pair update( const Q& val, bool bInsert = true ) { scoped_node_ptr sp( node_allocator().New( base_class::random_level(), val )); node_type * pNode; - std::pair bRes = base_class::ensure( *sp, [&pNode](bool, node_type& item, node_type&) { pNode = &item; } ); + std::pair bRes = base_class::update( *sp, [&pNode](bool, node_type& item, node_type&) { pNode = &item; }, bInsert ); if ( bRes.first && bRes.second ) sp.release(); + else if ( !bRes.first ) + return std::make_pair( end(), false ); assert( pNode ); return std::make_pair( node_to_iterator( pNode ), bRes.second ); } + //@cond + // Deprecated, use update() + template + std::pair ensure( const Q& val ) + { + return update( val, true ); + } + //@endcond + /// Searches \p key /** \anchor cds_nonintrusive_SkipListSet_nogc_find_val diff --git a/cds/container/skip_list_set_rcu.h b/cds/container/skip_list_set_rcu.h index bb53d747..32453bfa 100644 --- a/cds/container/skip_list_set_rcu.h +++ b/cds/container/skip_list_set_rcu.h @@ -330,27 +330,23 @@ namespace cds { namespace container { return false; } - /// Ensures that the item exists in the set + /// Updates the item /** The operation performs inserting or changing data with lock-free manner. - If the \p val key not found in the set, then the new item created from \p val - is inserted into the set. Otherwise, the functor \p func is called with the item found. - The functor \p Func should be a function with signature: - \code - void func( bool bNew, value_type& item, const Q& val ); - \endcode - or a functor: + If \p val not found in the set, then the new item created from \p val + is inserted into the set iff \p bInsert is \p true. + Otherwise, the functor \p func is called with the item found. + The functor \p Func signature: \code struct my_functor { void operator()( bool bNew, value_type& item, const Q& val ); }; \endcode - - with arguments: + where: - \p bNew - \p true if the item has been inserted, \p false otherwise - - \p item - item of the set - - \p val - argument \p key passed into the \p ensure function + - \p item - an item of the set + - \p val - argument \p val passed into the \p %update() function The functor may change non-key fields of the \p item; however, \p func must guarantee that during changing no any other modifications could be made on this item by concurrent threads. @@ -359,19 +355,28 @@ namespace cds { namespace container { Returns std::pair where \p first is true if operation is successfull, \p second is true if new item has been added or \p false if the item with \p key - already is in the set. + already exists. */ template - std::pair ensure( const Q& val, Func func ) + std::pair update( const Q& val, Func func, bool bInsert = true ) { scoped_node_ptr sp( node_allocator().New( random_level(), val )); - std::pair bRes = base_class::ensure( *sp, - [&func, &val](bool bNew, node_type& node, node_type&){ func( bNew, node.m_Value, val ); }); + std::pair bRes = base_class::update( *sp, + [&func, &val](bool bNew, node_type& node, node_type&){ func( bNew, node.m_Value, val );}, bInsert ); if ( bRes.first && bRes.second ) sp.release(); return bRes; } + //@cond + // Deprecated, use update() + template + std::pair ensure( const Q& val, Func func ) + { + return update( val, func, true ); + } + //@endcond + /// Inserts data of type \ref value_type constructed with std::forward(args)... /** Returns \p true if inserting successful, \p false otherwise. diff --git a/cds/intrusive/details/skip_list_base.h b/cds/intrusive/details/skip_list_base.h index be70fc1b..e24ae7a6 100644 --- a/cds/intrusive/details/skip_list_base.h +++ b/cds/intrusive/details/skip_list_base.h @@ -86,7 +86,15 @@ namespace cds { namespace intrusive { assert( nLevel < height() ); assert( nLevel == 0 || (nLevel > 0 && m_arrNext != nullptr) ); +# ifdef CDS_THREAD_SANITIZER_ENABLED + // TSan false positive: m_arrNext is read-only array + CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN; + atomic_marked_ptr& r = nLevel ? m_arrNext[ nLevel - 1] : m_pNext; + CDS_TSAN_ANNOTATE_IGNORE_READS_END; + return r; +# else return nLevel ? m_arrNext[ nLevel - 1] : m_pNext; +# endif } /// Access to element of next pointer array (const version) @@ -95,7 +103,15 @@ namespace cds { namespace intrusive { assert( nLevel < height() ); assert( nLevel == 0 || nLevel > 0 && m_arrNext != nullptr ); +# ifdef CDS_THREAD_SANITIZER_ENABLED + // TSan false positive: m_arrNext is read-only array + CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN; + atomic_marked_ptr const& r = nLevel ? m_arrNext[ nLevel - 1] : m_pNext; + CDS_TSAN_ANNOTATE_IGNORE_READS_END; + return r; +# else return nLevel ? m_arrNext[ nLevel - 1] : m_pNext; +# endif } /// Access to element of next pointer array (same as \ref next function) @@ -350,12 +366,13 @@ namespace cds { namespace intrusive { event_counter m_nInsertSuccess ; ///< Count of success insertion event_counter m_nInsertFailed ; ///< Count of failed insertion event_counter m_nInsertRetries ; ///< Count of unsuccessful retries of insertion - event_counter m_nEnsureExist ; ///< Count of \p ensure call for existed node - event_counter m_nEnsureNew ; ///< Count of \p ensure call for new node + event_counter m_nUpdateExist ; ///< Count of \p update() call for existed node + event_counter m_nUpdateNew ; ///< Count of \p update() call for new node event_counter m_nUnlinkSuccess ; ///< Count of successful call of \p unlink event_counter m_nUnlinkFailed ; ///< Count of failed call of \p unlink event_counter m_nEraseSuccess ; ///< Count of successful call of \p erase event_counter m_nEraseFailed ; ///< Count of failed call of \p erase + event_counter m_nEraseRetry ; ///< Count of retries while erasing node event_counter m_nFindFastSuccess ; ///< Count of successful call of \p find and all derivatives (via fast-path) event_counter m_nFindFastFailed ; ///< Count of failed call of \p find and all derivatives (via fast-path) event_counter m_nFindSlowSuccess ; ///< Count of successful call of \p find and all derivatives (via slow-path) @@ -394,12 +411,13 @@ namespace cds { namespace intrusive { void onInsertSuccess() { ++m_nInsertSuccess ; } void onInsertFailed() { ++m_nInsertFailed ; } void onInsertRetry() { ++m_nInsertRetries ; } - void onEnsureExist() { ++m_nEnsureExist ; } - void onEnsureNew() { ++m_nEnsureNew ; } + void onUpdateExist() { ++m_nUpdateExist ; } + void onUpdateNew() { ++m_nUpdateNew ; } void onUnlinkSuccess() { ++m_nUnlinkSuccess ; } void onUnlinkFailed() { ++m_nUnlinkFailed ; } void onEraseSuccess() { ++m_nEraseSuccess ; } void onEraseFailed() { ++m_nEraseFailed ; } + void onEraseRetry() { ++m_nEraseRetry; } void onFindFastSuccess() { ++m_nFindFastSuccess ; } void onFindFastFailed() { ++m_nFindFastFailed ; } void onFindSlowSuccess() { ++m_nFindSlowSuccess ; } @@ -434,12 +452,13 @@ namespace cds { namespace intrusive { void onInsertSuccess() const {} void onInsertFailed() const {} void onInsertRetry() const {} - void onEnsureExist() const {} - void onEnsureNew() const {} + void onUpdateExist() const {} + void onUpdateNew() const {} void onUnlinkSuccess() const {} void onUnlinkFailed() const {} void onEraseSuccess() const {} void onEraseFailed() const {} + void onEraseRetry() const {} void onFindFastSuccess() const {} void onFindFastFailed() const {} void onFindSlowSuccess() const {} diff --git a/cds/intrusive/impl/skip_list.h b/cds/intrusive/impl/skip_list.h index 9abad6f5..3574937a 100644 --- a/cds/intrusive/impl/skip_list.h +++ b/cds/intrusive/impl/skip_list.h @@ -391,7 +391,7 @@ namespace cds { namespace intrusive { node_type * pSucc[ c_nMaxHeight ]; typename gc::template GuardArray< c_nMaxHeight * 2 > guards; ///< Guards array for pPrev/pSucc - node_type * pCur; // guarded by guards; needed only for \p ensure() + node_type * pCur; // guarded by guards; needed only for \p update() }; //@endcond @@ -460,16 +460,16 @@ namespace cds { namespace intrusive { } // pSucc contains deletion mark for pCur - pSucc = pCur->next( nLevel ).load( memory_model::memory_order_relaxed ); + pSucc = pCur->next( nLevel ).load( memory_model::memory_order_acquire ); - if ( pPred->next( nLevel ).load( memory_model::memory_order_relaxed ).all() != pCur.ptr() ) + if ( pPred->next( nLevel ).load( memory_model::memory_order_acquire ).all() != pCur.ptr() ) goto retry; if ( pSucc.bits() ) { // pCur is marked, i.e. logically deleted. marked_node_ptr p( pCur.ptr() ); if ( pPred->next( nLevel ).compare_exchange_strong( p, marked_node_ptr( pSucc.ptr() ), - memory_model::memory_order_release, atomics::memory_order_relaxed )) + memory_model::memory_order_acquire, atomics::memory_order_relaxed )) { if ( nLevel == 0 ) { gc::retire( node_traits::to_value_ptr( pCur.ptr() ), dispose_node ); @@ -528,19 +528,21 @@ namespace cds { namespace intrusive { if ( pCur.ptr() ) { // pSucc contains deletion mark for pCur - pSucc = pCur->next( nLevel ).load( memory_model::memory_order_relaxed ); + pSucc = pCur->next( nLevel ).load( memory_model::memory_order_acquire ); - if ( pPred->next( nLevel ).load( memory_model::memory_order_relaxed ).all() != pCur.ptr() ) + if ( pPred->next( nLevel ).load( memory_model::memory_order_acquire ).all() != pCur.ptr() ) goto retry; if ( pSucc.bits() ) { // pCur is marked, i.e. logically deleted. marked_node_ptr p( pCur.ptr() ); if ( pPred->next( nLevel ).compare_exchange_strong( p, marked_node_ptr( pSucc.ptr() ), - memory_model::memory_order_release, atomics::memory_order_relaxed )) + memory_model::memory_order_acquire, atomics::memory_order_relaxed )) { - if ( nLevel == 0 ) + if ( nLevel == 0 ) { gc::retire( node_traits::to_value_ptr( pCur.ptr() ), dispose_node ); + m_Stat.onEraseWhileFind(); + } } goto retry; } @@ -582,19 +584,21 @@ namespace cds { namespace intrusive { } // pSucc contains deletion mark for pCur - pSucc = pCur->next( nLevel ).load( memory_model::memory_order_relaxed ); + pSucc = pCur->next( nLevel ).load( memory_model::memory_order_acquire ); - if ( pPred->next( nLevel ).load( memory_model::memory_order_relaxed ).all() != pCur.ptr() ) + if ( pPred->next( nLevel ).load( memory_model::memory_order_acquire ).all() != pCur.ptr() ) goto retry; if ( pSucc.bits() ) { // pCur is marked, i.e. logically deleted. marked_node_ptr p( pCur.ptr() ); if ( pPred->next( nLevel ).compare_exchange_strong( p, marked_node_ptr( pSucc.ptr() ), - memory_model::memory_order_release, atomics::memory_order_relaxed )) + memory_model::memory_order_acquire, atomics::memory_order_relaxed )) { - if ( nLevel == 0 ) + if ( nLevel == 0 ) { gc::retire( node_traits::to_value_ptr( pCur.ptr() ), dispose_node ); + m_Stat.onEraseWhileFind(); + } } goto retry; } @@ -624,15 +628,17 @@ namespace cds { namespace intrusive { for ( unsigned int nLevel = 1; nLevel < nHeight; ++nLevel ) pNode->next(nLevel).store( marked_node_ptr(), memory_model::memory_order_relaxed ); + // Insert at level 0 { marked_node_ptr p( pos.pSucc[0] ); pNode->next( 0 ).store( p, memory_model::memory_order_release ); - if ( !pos.pPrev[0]->next(0).compare_exchange_strong( p, marked_node_ptr(pNode), memory_model::memory_order_release, atomics::memory_order_relaxed ) ) { + if ( !pos.pPrev[0]->next(0).compare_exchange_strong( p, marked_node_ptr(pNode), memory_model::memory_order_release, atomics::memory_order_relaxed )) return false; - } + f( val ); } + // Insert at level 1..max for ( unsigned int nLevel = 1; nLevel < nHeight; ++nLevel ) { marked_node_ptr p; while ( true ) { @@ -645,7 +651,7 @@ namespace cds { namespace intrusive { return true; } p = q; - if ( pos.pPrev[nLevel]->next(nLevel).compare_exchange_strong( q, marked_node_ptr( pNode ), memory_model::memory_order_release, atomics::memory_order_relaxed ) ) + if ( pos.pPrev[nLevel]->next(nLevel).compare_exchange_strong( q, marked_node_ptr( pNode ), memory_model::memory_order_release, atomics::memory_order_relaxed )) break; // Renew insert position @@ -666,14 +672,13 @@ namespace cds { namespace intrusive { assert( pDel != nullptr ); marked_node_ptr pSucc; - typename gc::Guard gSucc; // logical deletion (marking) for ( unsigned int nLevel = pDel->height() - 1; nLevel > 0; --nLevel ) { while ( true ) { - pSucc = gSucc.protect( pDel->next(nLevel), gc_protect ); + pSucc = pDel->next(nLevel); if ( pSucc.bits() || pDel->next(nLevel).compare_exchange_weak( pSucc, pSucc | 1, - memory_model::memory_order_acquire, atomics::memory_order_relaxed )) + memory_model::memory_order_release, atomics::memory_order_relaxed )) { break; } @@ -681,10 +686,8 @@ namespace cds { namespace intrusive { } while ( true ) { - pSucc = gSucc.protect( pDel->next(0), gc_protect ); - marked_node_ptr p( pSucc.ptr() ); - if ( pDel->next(0).compare_exchange_strong( p, marked_node_ptr(p.ptr(), 1), - memory_model::memory_order_acquire, atomics::memory_order_relaxed )) + marked_node_ptr p( pDel->next(0).load(memory_model::memory_order_relaxed).ptr() ); + if ( pDel->next(0).compare_exchange_strong( p, p | 1, memory_model::memory_order_release, atomics::memory_order_relaxed )) { f( *node_traits::to_value_ptr( pDel )); @@ -692,9 +695,9 @@ namespace cds { namespace intrusive { // try fast erase p = pDel; for ( int nLevel = static_cast( pDel->height() - 1 ); nLevel >= 0; --nLevel ) { - pSucc = gSucc.protect( pDel->next(nLevel), gc_protect ); + pSucc = pDel->next(nLevel).load(memory_model::memory_order_relaxed); if ( !pos.pPrev[nLevel]->next(nLevel).compare_exchange_strong( p, marked_node_ptr(pSucc.ptr()), - memory_model::memory_order_release, atomics::memory_order_relaxed) ) + memory_model::memory_order_acquire, atomics::memory_order_relaxed) ) { // Make slow erase find_position( *node_traits::to_value_ptr( pDel ), pos, key_comparator(), false ); @@ -710,9 +713,11 @@ namespace cds { namespace intrusive { } else { if ( p.bits() ) { + // Another thread is deleting pDel right now return false; } } + m_Stat.onEraseRetry(); } } @@ -1045,8 +1050,7 @@ namespace cds { namespace intrusive { position pos; while ( true ) { - bool bFound = find_position( val, pos, key_comparator(), true ); - if ( bFound ) { + if ( find_position( val, pos, key_comparator(), true )) { // scoped_node_ptr deletes the node tower if we create it if ( !bTowerMade ) scp.release(); @@ -1076,31 +1080,33 @@ namespace cds { namespace intrusive { } } - /// Ensures that the \p val exists in the set + /// Updates the node /** The operation performs inserting or changing data with lock-free manner. - If the item \p val is not found in the set, then \p val is inserted into the set. + If the item \p val is not found in the set, then \p val is inserted into the set + iff \p bInsert is \p true. Otherwise, the functor \p func is called with item found. - The functor signature is: + The functor \p func signature is: \code void func( bool bNew, value_type& item, value_type& val ); \endcode with arguments: - \p bNew - \p true if the item has been inserted, \p false otherwise - \p item - item of the set - - \p val - argument \p val passed into the \p ensure function + - \p val - argument \p val passed into the \p %update() function If new item has been inserted (i.e. \p bNew is \p true) then \p item and \p val arguments refer to the same thing. Returns std::pair where \p first is \p true if operation is successfull, + i.e. the node has been inserted or updated, \p second is \p true if new item has been added or \p false if the item with \p key - already is in the set. + already exists. @warning See \ref cds_intrusive_item_creating "insert item troubleshooting" */ template - std::pair ensure( value_type& val, Func func ) + std::pair update( value_type& val, Func func, bool bInsert = true ) { typename gc::Guard gNew; gNew.assign( &val ); @@ -1121,10 +1127,15 @@ namespace cds { namespace intrusive { scp.release(); func( false, *node_traits::to_value_ptr(pos.pCur), val ); - m_Stat.onEnsureExist(); + m_Stat.onUpdateExist(); return std::make_pair( true, false ); } + if ( !bInsert ) { + scp.release(); + return std::make_pair( false, false ); + } + if ( !bTowerOk ) { build_node( pNode ); nHeight = pNode->height(); @@ -1141,11 +1152,20 @@ namespace cds { namespace intrusive { ++m_ItemCounter; scp.release(); m_Stat.onAddNode( nHeight ); - m_Stat.onEnsureNew(); + m_Stat.onUpdateNew(); return std::make_pair( true, true ); } } + //@cond + // Deprecated, use update() instead + template + std::pair ensure( value_type& val, Func func ) + { + return update( val, func, true ); + } + //@endcond + /// Unlinks the item \p val from the set /** The function searches the item \p val in the set and unlink it from the set diff --git a/cds/intrusive/skip_list_nogc.h b/cds/intrusive/skip_list_nogc.h index 6a8f4c3f..1d4503f4 100644 --- a/cds/intrusive/skip_list_nogc.h +++ b/cds/intrusive/skip_list_nogc.h @@ -679,11 +679,12 @@ namespace cds { namespace intrusive { } } - /// Ensures that the \p val exists in the set + /// Updates the node /** The operation performs inserting or changing data with lock-free manner. - If the item \p val is not found in the set, then \p val is inserted into the set. + If the item \p val is not found in the set, then \p val is inserted into the set + iff \p bInsert is \p true. Otherwise, the functor \p func is called with item found. The functor signature is: \code @@ -692,7 +693,7 @@ namespace cds { namespace intrusive { with arguments: - \p bNew - \p true if the item has been inserted, \p false otherwise - \p item - item of the set - - \p val - argument \p val passed into the \p ensure function + - \p val - argument \p val passed into the \p %update() function If new item has been inserted (i.e. \p bNew is \p true) then \p item and \p val arguments refer to the same thing. @@ -706,7 +707,7 @@ namespace cds { namespace intrusive { @warning See \ref cds_intrusive_item_creating "insert item troubleshooting" */ template - std::pair ensure( value_type& val, Func func ) + std::pair update( value_type& val, Func func, bool bInsert = true ) { node_type * pNode = node_traits::to_node_ptr( val ); scoped_node_ptr scp( pNode ); @@ -724,10 +725,15 @@ namespace cds { namespace intrusive { scp.release(); func( false, *node_traits::to_value_ptr(pos.pCur), val ); - m_Stat.onEnsureExist(); + m_Stat.onUpdateExist(); return std::make_pair( true, false ); } + if ( !bInsert ) { + scp.release(); + return std::make_pair( false, false ); + } + if ( !bTowerOk ) { build_node( pNode ); nHeight = pNode->height(); @@ -744,11 +750,20 @@ namespace cds { namespace intrusive { ++m_ItemCounter; scp.release(); m_Stat.onAddNode( nHeight ); - m_Stat.onEnsureNew(); + m_Stat.onUpdateNew(); return std::make_pair( true, true ); } } + //@cond + // Deprecated, use update() instead + template + std::pair ensure( value_type& val, Func func ) + { + return update( val, func, true ); + } + //@endcond + /// Finds \p key /** \anchor cds_intrusive_SkipListSet_nogc_find_func The function searches the item with key equal to \p key and calls the functor \p f for item found. diff --git a/cds/intrusive/skip_list_rcu.h b/cds/intrusive/skip_list_rcu.h index 58ac7eda..95d9f592 100644 --- a/cds/intrusive/skip_list_rcu.h +++ b/cds/intrusive/skip_list_rcu.h @@ -101,7 +101,15 @@ namespace cds { namespace intrusive { assert( nLevel < height() ); assert( nLevel == 0 || (nLevel > 0 && m_arrNext != nullptr) ); - return nLevel ? m_arrNext[nLevel - 1] : m_pNext; +# ifdef CDS_THREAD_SANITIZER_ENABLED + // TSan false positive: m_arrNext is read-only array + CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN; + atomic_marked_ptr& r = nLevel ? m_arrNext[ nLevel - 1] : m_pNext; + CDS_TSAN_ANNOTATE_IGNORE_READS_END; + return r; +# else + return nLevel ? m_arrNext[ nLevel - 1] : m_pNext; +# endif } /// Access to element of next pointer array (const version) @@ -110,7 +118,15 @@ namespace cds { namespace intrusive { assert( nLevel < height() ); assert( nLevel == 0 || nLevel > 0 && m_arrNext != nullptr ); - return nLevel ? m_arrNext[nLevel - 1] : m_pNext; +# ifdef CDS_THREAD_SANITIZER_ENABLED + // TSan false positive: m_arrNext is read-only array + CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN; + atomic_marked_ptr& r = nLevel ? m_arrNext[ nLevel - 1] : m_pNext; + CDS_TSAN_ANNOTATE_IGNORE_READS_END; + return r; +# else + return nLevel ? m_arrNext[ nLevel - 1] : m_pNext; +# endif } /// Access to element of next pointer array (same as \ref next function) @@ -682,7 +698,7 @@ namespace cds { namespace intrusive { for ( int nLevel = static_cast(c_nMaxHeight - 1); nLevel >= 0; --nLevel ) { while ( true ) { - pCur = pPred->next( nLevel ).load( memory_model::memory_order_relaxed ); + pCur = pPred->next( nLevel ).load( memory_model::memory_order_acquire ); if ( pCur.bits() ) { // pCur.bits() means that pPred is logically deleted goto retry; @@ -694,9 +710,9 @@ namespace cds { namespace intrusive { } // pSucc contains deletion mark for pCur - pSucc = pCur->next( nLevel ).load( memory_model::memory_order_relaxed ); + pSucc = pCur->next( nLevel ).load( memory_model::memory_order_acquire ); - if ( pPred->next( nLevel ).load( memory_model::memory_order_relaxed ).all() != pCur.ptr() ) + if ( pPred->next( nLevel ).load( memory_model::memory_order_acquire ).all() != pCur.ptr() ) goto retry; if ( pSucc.bits() ) { @@ -760,7 +776,7 @@ namespace cds { namespace intrusive { for ( int nLevel = static_cast(c_nMaxHeight - 1); nLevel >= 0; --nLevel ) { - pCur = pPred->next( nLevel ).load( memory_model::memory_order_relaxed ); + pCur = pPred->next( nLevel ).load( memory_model::memory_order_acquire ); // pCur.bits() means that pPred is logically deleted // head cannot be deleted assert( pCur.bits() == 0 ); @@ -768,9 +784,9 @@ namespace cds { namespace intrusive { if ( pCur.ptr() ) { // pSucc contains deletion mark for pCur - pSucc = pCur->next( nLevel ).load( memory_model::memory_order_relaxed ); + pSucc = pCur->next( nLevel ).load( memory_model::memory_order_acquire ); - if ( pPred->next( nLevel ).load( memory_model::memory_order_relaxed ).all() != pCur.ptr() ) + if ( pPred->next( nLevel ).load( memory_model::memory_order_acquire ).all() != pCur.ptr() ) goto retry; if ( pSucc.bits() ) { @@ -820,7 +836,7 @@ namespace cds { namespace intrusive { for ( int nLevel = static_cast(c_nMaxHeight - 1); nLevel >= 0; --nLevel ) { while ( true ) { - pCur = pPred->next( nLevel ).load( memory_model::memory_order_relaxed ); + pCur = pPred->next( nLevel ).load( memory_model::memory_order_acquire ); if ( pCur.bits() ) { // pCur.bits() means that pPred is logically deleted goto retry; @@ -832,9 +848,9 @@ namespace cds { namespace intrusive { } // pSucc contains deletion mark for pCur - pSucc = pCur->next( nLevel ).load( memory_model::memory_order_relaxed ); + pSucc = pCur->next( nLevel ).load( memory_model::memory_order_acquire ); - if ( pPred->next( nLevel ).load( memory_model::memory_order_relaxed ).all() != pCur.ptr() ) + if ( pPred->next( nLevel ).load( memory_model::memory_order_acquire ).all() != pCur.ptr() ) goto retry; if ( pSucc.bits() ) { @@ -986,9 +1002,9 @@ namespace cds { namespace intrusive { } else m_Stat.onFastExtract(); - return true; } + m_Stat.onEraseRetry(); } } @@ -1423,11 +1439,12 @@ namespace cds { namespace intrusive { return bRet; } - /// Ensures that the \p val exists in the set + /// Updates the node /** The operation performs inserting or changing data with lock-free manner. - If the item \p val is not found in the set, then \p val is inserted into the set. + If the item \p val is not found in the set, then \p val is inserted into the set + iff \p bInsert is \p true. Otherwise, the functor \p func is called with item found. The functor signature is: \code @@ -1436,7 +1453,7 @@ namespace cds { namespace intrusive { with arguments: - \p bNew - \p true if the item has been inserted, \p false otherwise - \p item - item of the set - - \p val - argument \p val passed into the \p %ensure() function + - \p val - argument \p val passed into the \p %update() function If new item has been inserted (i.e. \p bNew is \p true) then \p item and \p val arguments refer to the same thing. @@ -1446,13 +1463,14 @@ namespace cds { namespace intrusive { RCU \p synchronize method can be called. RCU should not be locked. Returns std::pair where \p first is \p true if operation is successfull, + i.e. the node has been inserted or updated, \p second is \p true if new item has been added or \p false if the item with \p key - already is in the set. + already exists. @warning See \ref cds_intrusive_item_creating "insert item troubleshooting" */ template - std::pair ensure( value_type& val, Func func ) + std::pair update( value_type& val, Func func, bool bInsert = true ) { check_deadlock_policy::check(); @@ -1476,7 +1494,13 @@ namespace cds { namespace intrusive { scp.release(); func( false, *node_traits::to_value_ptr(pos.pCur), val ); - m_Stat.onEnsureExist(); + m_Stat.onUpdateExist(); + break; + } + + if ( !bInsert ) { + scp.release(); + bRet.first = false; break; } @@ -1496,7 +1520,7 @@ namespace cds { namespace intrusive { ++m_ItemCounter; scp.release(); m_Stat.onAddNode( nHeight ); - m_Stat.onEnsureNew(); + m_Stat.onUpdateNew(); bRet.second = true; break; } @@ -1505,6 +1529,15 @@ namespace cds { namespace intrusive { return bRet; } + //@cond + // Deprecated, use update(). + template + std::pair ensure( value_type& val, Func func ) + { + return update( val, func, true ); + } + //@endcond + /// Unlinks the item \p val from the set /** The function searches the item \p val in the set and unlink it from the set diff --git a/tests/test-hdr/set/hdr_intrusive_skiplist_set.h b/tests/test-hdr/set/hdr_intrusive_skiplist_set.h index 6d882bde..04b0ddce 100644 --- a/tests/test-hdr/set/hdr_intrusive_skiplist_set.h +++ b/tests/test-hdr/set/hdr_intrusive_skiplist_set.h @@ -397,21 +397,28 @@ namespace set { //CPPUNIT_MSG( PrintStat()(s, "Insert test") ); ensure_functor f; - std::pair ret = s.ensure( v1, f ); + std::pair ret = s.update( v1, f, true ); CPPUNIT_ASSERT( ret.first ); CPPUNIT_ASSERT( ret.second ); CPPUNIT_ASSERT( v1.nEnsureNewCount == 1 ); CPPUNIT_ASSERT( v1.nEnsureCount == 0 ); CPPUNIT_ASSERT( check_size( s, 1 )); - ret = s.ensure( v2, f ); + ret = s.update( v2, f, false ); + CPPUNIT_ASSERT( !ret.first ); + CPPUNIT_ASSERT( !ret.second ); + CPPUNIT_ASSERT( v2.nEnsureNewCount == 0 ); + CPPUNIT_ASSERT( v2.nEnsureCount == 0 ); + CPPUNIT_ASSERT( check_size( s, 1 )); + + ret = s.update( v2, f ); CPPUNIT_ASSERT( ret.first ); CPPUNIT_ASSERT( ret.second ); CPPUNIT_ASSERT( v2.nEnsureNewCount == 1 ); CPPUNIT_ASSERT( v2.nEnsureCount == 0 ); CPPUNIT_ASSERT( check_size( s, 2 )); - ret = s.ensure( v3, f ); + ret = s.update( v3, f, true ); CPPUNIT_ASSERT( ret.first ); CPPUNIT_ASSERT( ret.second ); CPPUNIT_ASSERT( v3.nEnsureNewCount == 1 ); @@ -422,21 +429,21 @@ namespace set { CPPUNIT_ASSERT( s.find_with( v2, base_class::less() ) == &v2 ); CPPUNIT_ASSERT( s.find( v3 ) == &v3 ); - ret = s.ensure( v1, f ); + ret = s.update( v1, f, true ); CPPUNIT_ASSERT( ret.first ); CPPUNIT_ASSERT( !ret.second ); CPPUNIT_ASSERT( v1.nEnsureNewCount == 1 ); CPPUNIT_ASSERT( v1.nEnsureCount == 1 ); CPPUNIT_ASSERT( check_size( s, 3 )); - ret = s.ensure( v2, f ); + ret = s.update( v2, f, false ); CPPUNIT_ASSERT( ret.first ); CPPUNIT_ASSERT( !ret.second ); CPPUNIT_ASSERT( v2.nEnsureNewCount == 1 ); CPPUNIT_ASSERT( v2.nEnsureCount == 1 ); CPPUNIT_ASSERT( check_size( s, 3 )); - ret = s.ensure( v3, f ); + ret = s.update( v3, f ); CPPUNIT_ASSERT( ret.first ); CPPUNIT_ASSERT( !ret.second ); CPPUNIT_ASSERT( v3.nEnsureNewCount == 1 ); diff --git a/tests/unit/print_skip_list_stat.h b/tests/unit/print_skip_list_stat.h index 7633a753..615cffc3 100644 --- a/tests/unit/print_skip_list_stat.h +++ b/tests/unit/print_skip_list_stat.h @@ -22,8 +22,8 @@ namespace std { << "\t\t m_nInsertSuccess: " << s.m_nInsertSuccess.get() << "\n" << "\t\t m_nInsertFailed: " << s.m_nInsertFailed.get() << "\n" << "\t\t m_nInsertRetries: " << s.m_nInsertRetries.get() << "\n" - << "\t\t m_nEnsureExist: " << s.m_nEnsureExist.get() << "\n" - << "\t\t m_nEnsureNew: " << s.m_nEnsureNew.get() << "\n" + << "\t\t m_nUpdateExist: " << s.m_nUpdateExist.get() << "\n" + << "\t\t m_nUpdateNew: " << s.m_nUpdateNew.get() << "\n" << "\t\t m_nUnlinkSuccess: " << s.m_nUnlinkSuccess.get() << "\n" << "\t\t m_nUnlinkFailed: " << s.m_nUnlinkFailed.get() << "\n" << "\t\t m_nExtractSuccess: " << s.m_nExtractSuccess.get() << "\n" @@ -37,6 +37,7 @@ namespace std { << "\t\t m_nExtractMaxRetries: " << s.m_nExtractMaxRetries.get() << "\n" << "\t\t m_nEraseSuccess: " << s.m_nEraseSuccess.get() << "\n" << "\t\t m_nEraseFailed: " << s.m_nEraseFailed.get() << "\n" + << "\t\t m_nEraseRetry: " << s.m_nEraseRetry.get() << "\n" << "\t\t m_nFindFastSuccess: " << s.m_nFindFastSuccess.get() << "\n" << "\t\t m_nFindFastFailed: " << s.m_nFindFastFailed.get() << "\n" << "\t\t m_nFindSlowSuccess: " << s.m_nFindSlowSuccess.get() << "\n"