From 028a03b5a92d0be6f9f09fe3c72c36790399f193 Mon Sep 17 00:00:00 2001 From: Danila Klimenko Date: Tue, 25 Aug 2015 18:10:10 +0200 Subject: [PATCH] Performance optimizations (UniqueHazardPointer local copy; Tree node leaf/sentinel flags) --- containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h | 10 ++++++++-- containers_cpp/include/embb/containers/internal/hazard_pointer.h | 2 ++ containers_cpp/include/embb/containers/internal/lock_free_chromatic_tree-inl.h | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------- containers_cpp/include/embb/containers/lock_free_chromatic_tree.h | 50 +++++++++++++++++++++++++------------------------- 4 files changed, 109 insertions(+), 77 deletions(-) diff --git a/containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h b/containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h index 32299b2..edb026e 100644 --- a/containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h +++ b/containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h @@ -422,12 +422,16 @@ const double embb::containers::internal::HazardPointer:: template UniqueHazardPointer:: UniqueHazardPointer() - : hazard_guard_(NULL), undefined_guard_(NULL), active_(false) {} + : hazard_guard_(NULL), + local_ptr_value_(NULL), + undefined_guard_(NULL), + active_(false) {} template UniqueHazardPointer:: UniqueHazardPointer(AtomicTypePtr& hazard_guard, Type* undefined_guard) : hazard_guard_(&hazard_guard), + local_ptr_value_(hazard_guard_->Load()), undefined_guard_(undefined_guard), active_(LoadGuardedPointer() == undefined_guard_) {} @@ -487,6 +491,7 @@ void UniqueHazardPointer::AdoptHazard(const UniqueHazardPointer& other) { template void UniqueHazardPointer::Swap(UniqueHazardPointer& other) { std::swap(hazard_guard_, other.hazard_guard_); + std::swap(local_ptr_value_, other.local_ptr_value_); std::swap(undefined_guard_, other.undefined_guard_); std::swap(active_, other.active_); } @@ -517,12 +522,13 @@ void UniqueHazardPointer::ClearHazard() { template Type* UniqueHazardPointer::LoadGuardedPointer() const { - return hazard_guard_->Load(); + return local_ptr_value_; } template void UniqueHazardPointer::StoreGuardedPointer(Type* ptr) { hazard_guard_->Store(ptr); + local_ptr_value_ = ptr; } template diff --git a/containers_cpp/include/embb/containers/internal/hazard_pointer.h b/containers_cpp/include/embb/containers/internal/hazard_pointer.h index 8ce0b20..ccd3fe3 100644 --- a/containers_cpp/include/embb/containers/internal/hazard_pointer.h +++ b/containers_cpp/include/embb/containers/internal/hazard_pointer.h @@ -692,6 +692,8 @@ class UniqueHazardPointer { * hazardous pointers */ AtomicTypePtr* hazard_guard_; + /** Local copy of the guarded pointer value (used for optimization) */ + Type* local_ptr_value_; /** Dummy value used to clear the hazard guard from any hazards */ Type* undefined_guard_; /** Flag set to true when the guard is protecting some hazardous pointer */ diff --git a/containers_cpp/include/embb/containers/internal/lock_free_chromatic_tree-inl.h b/containers_cpp/include/embb/containers/internal/lock_free_chromatic_tree-inl.h index 3142ac1..fd02b65 100644 --- a/containers_cpp/include/embb/containers/internal/lock_free_chromatic_tree-inl.h +++ b/containers_cpp/include/embb/containers/internal/lock_free_chromatic_tree-inl.h @@ -48,7 +48,9 @@ ChromaticTreeNode(const Key& key, const Value& value, int weight, Node* left, Node* right, Operation* operation) : key_(key), value_(value), - weight_(weight), + weight_(weight < 0 ? -weight : weight), + is_leaf_(left == NULL), + is_sentinel_(weight < 0), left_(left), right_(right), retired_(false), @@ -60,52 +62,64 @@ ChromaticTreeNode(const Key& key, const Value& value, int weight, Operation* operation) : key_(key), value_(value), - weight_(weight), + weight_(weight < 0 ? -weight : weight), + is_leaf_(true), + is_sentinel_(weight < 0), left_(NULL), right_(NULL), retired_(false), operation_(operation) {} template -const Key& ChromaticTreeNode::GetKey() const { +inline const Key& ChromaticTreeNode::GetKey() const { return key_; } template -const Value& ChromaticTreeNode::GetValue() const { +inline const Value& ChromaticTreeNode::GetValue() const { return value_; } template -int ChromaticTreeNode::GetWeight() const { +inline int ChromaticTreeNode::GetWeight() const { return weight_; } template -typename ChromaticTreeNode::AtomicNodePtr& +inline typename ChromaticTreeNode::AtomicNodePtr& ChromaticTreeNode::GetLeft() { return left_; } template -typename ChromaticTreeNode::Node* +inline typename ChromaticTreeNode::Node* ChromaticTreeNode::GetLeft() const { return left_.Load(); } template -typename ChromaticTreeNode::AtomicNodePtr& +inline typename ChromaticTreeNode::AtomicNodePtr& ChromaticTreeNode::GetRight() { return right_; } template -typename ChromaticTreeNode::Node* +inline typename ChromaticTreeNode::Node* ChromaticTreeNode::GetRight() const { return right_.Load(); } template +inline bool ChromaticTreeNode::IsLeaf() const { + return is_leaf_; +} + +template +inline bool ChromaticTreeNode::IsSentinel() const { + return is_sentinel_; +} + +template bool ChromaticTreeNode:: ReplaceChild(Node* old_child, Node* new_child) { bool replaced = false; @@ -120,17 +134,17 @@ ReplaceChild(Node* old_child, Node* new_child) { } template -void ChromaticTreeNode::Retire() { +inline void ChromaticTreeNode::Retire() { retired_ = true; } template -bool ChromaticTreeNode::IsRetired() const { +inline bool ChromaticTreeNode::IsRetired() const { return retired_; } template -typename ChromaticTreeNode::AtomicOperationPtr& +inline typename ChromaticTreeNode::AtomicOperationPtr& ChromaticTreeNode::GetOperation() { return operation_; } @@ -143,13 +157,16 @@ ChromaticTreeOperation::ChromaticTreeOperation() root_(NULL), root_operation_(NULL), num_old_nodes_(0), - old_nodes_(), - old_operations_(), new_child_(NULL) #ifdef EMBB_DEBUG , deleted_(false) #endif -{} +{ + for (size_t i = 0; i < MAX_NODES; ++i) { + old_nodes_[i] = NULL; + old_operations_[i] = NULL; + } +} template void ChromaticTreeOperation:: @@ -292,7 +309,7 @@ void ChromaticTreeOperation::HelpAbort(Node* node) { } template -bool ChromaticTreeOperation::IsAborted() { +inline bool ChromaticTreeOperation::IsAborted() { #ifdef EMBB_DEBUG assert(!deleted_); #endif @@ -300,7 +317,7 @@ bool ChromaticTreeOperation::IsAborted() { } template -bool ChromaticTreeOperation::IsInProgress() { +inline bool ChromaticTreeOperation::IsInProgress() { #ifdef EMBB_DEBUG assert(!deleted_); #endif @@ -309,7 +326,7 @@ bool ChromaticTreeOperation::IsInProgress() { } template -bool ChromaticTreeOperation::IsCommitted() { +inline bool ChromaticTreeOperation::IsCommitted() { #ifdef EMBB_DEBUG assert(!deleted_); #endif @@ -341,7 +358,14 @@ void ChromaticTreeOperation::SetDeleted() { template typename ChromaticTreeOperation::Operation* ChromaticTreeOperation::GetInitialDummmy() { +#ifdef EMBB_PLATFORM_COMPILER_MSVC +#pragma warning(push) +#pragma warning(disable:4640) +#endif static ChromaticTreeOperation initial_dummy; +#ifdef EMBB_PLATFORM_COMPILER_MSVC +#pragma warning(pop) +#endif initial_dummy.state_ = STATE_COMMITTED; @@ -351,7 +375,14 @@ ChromaticTreeOperation::GetInitialDummmy() { template typename ChromaticTreeOperation::Operation* ChromaticTreeOperation::GetRetiredDummmy() { +#ifdef EMBB_PLATFORM_COMPILER_MSVC +#pragma warning(push) +#pragma warning(disable:4640) +#endif static ChromaticTreeOperation retired_dummy; +#ifdef EMBB_PLATFORM_COMPILER_MSVC +#pragma warning(pop) +#endif retired_dummy.state_ = STATE_COMMITTED; @@ -513,10 +544,10 @@ ChromaticTree(size_t capacity, Key undefined_key, Value undefined_value, operation_pool_(2 + 5 + 2 * capacity_ + operation_hazard_manager_.GetRetiredListMaxSize() * embb::base::Thread::GetThreadsMaxCount()), - entry_(node_pool_.Allocate(undefined_key_, undefined_value_, 1, + entry_(node_pool_.Allocate(undefined_key_, undefined_value_, -1, node_pool_.Allocate(undefined_key_, undefined_value_, - 1, + -1, Operation::INITIAL_DUMMY), static_cast(NULL), Operation::INITIAL_DUMMY)) { @@ -539,7 +570,7 @@ Get(const Key& key, Value& value) { HazardNodePtr leaf(GetNodeGuard(HIDX_LEAF)); Search(key, leaf, parent, grandparent); - bool keys_are_equal = !IsSentinel(leaf) && + bool keys_are_equal = !leaf->IsSentinel() && !(compare_(key, leaf->GetKey()) || compare_(leaf->GetKey(), key)); @@ -582,7 +613,7 @@ TryInsert(const Key& key, const Value& value, Value& old_value) { HazardOperationPtr leaf_op(GetOperationGuard(HIDX_LEAF)); if (!WeakLLX(leaf, leaf_op)) continue; - bool keys_are_equal = !IsSentinel(leaf) && + bool keys_are_equal = !leaf->IsSentinel() && !(compare_(key, leaf->GetKey()) || compare_(leaf->GetKey(), key)); if (keys_are_equal) { @@ -603,8 +634,11 @@ TryInsert(const Key& key, const Value& value, Value& old_value) { Operation::INITIAL_DUMMY); if (new_sibling == NULL) break; - int new_weight = (IsSentinel(parent)) ? 1 : (leaf->GetWeight() - 1); - if (IsSentinel(leaf) || compare_(key, leaf->GetKey())) { + int new_weight = + leaf->IsSentinel() ? -1 : + parent->IsSentinel() ? 1 : + (leaf->GetWeight() - 1); + if (leaf->IsSentinel() || compare_(key, leaf->GetKey())) { new_parent = node_pool_.Allocate(leaf->GetKey(), undefined_value_, new_weight, new_leaf, new_sibling, Operation::INITIAL_DUMMY); @@ -685,7 +719,7 @@ TryDelete(const Key& key, Value& old_value) { Search(key, leaf, parent, grandparent); // Reached leaf has a different key - nothing to delete - if (IsSentinel(leaf) || (compare_(key, leaf->GetKey()) || + if (leaf->IsSentinel() || (compare_(key, leaf->GetKey()) || compare_(leaf->GetKey(), key))) { old_value = undefined_value_; deletion_succeeded = true; @@ -720,8 +754,10 @@ TryDelete(const Key& key, Value& old_value) { HazardOperationPtr leaf_op(GetOperationGuard(HIDX_LEAF)); if (!WeakLLX(leaf, leaf_op)) continue; - int new_weight = (IsSentinel(grandparent)) ? - 1 : (parent->GetWeight() + sibling->GetWeight()); + int new_weight = + parent->IsSentinel() ? -1 : + grandparent->IsSentinel() ? 1 : + (parent->GetWeight() + sibling->GetWeight()); new_leaf = node_pool_.Allocate( sibling->GetKey(), sibling->GetValue(), new_weight, @@ -788,9 +824,9 @@ GetUndefinedValue() const { } template -bool ChromaticTree:: +inline bool ChromaticTree:: IsEmpty() const { - return IsLeaf(entry_->GetLeft()); + return entry_->GetLeft()->IsLeaf(); } template @@ -804,13 +840,13 @@ Search(const Key& key, HazardNodePtr& leaf, HazardNodePtr& parent, parent.ProtectSafe(entry_); leaf.ProtectSafe(entry_); - reached_leaf = IsLeaf(leaf); + reached_leaf = leaf->IsLeaf(); while (!reached_leaf) { grandparent.AdoptHazard(parent); parent.AdoptHazard(leaf); AtomicNodePtr& next_leaf = - (IsSentinel(leaf) || compare_(key, leaf->GetKey())) ? + (leaf->IsSentinel() || compare_(key, leaf->GetKey())) ? leaf->GetLeft() : leaf->GetRight(); // Parent is protected, so we can tolerate a changing child pointer @@ -828,25 +864,13 @@ Search(const Key& key, HazardNodePtr& leaf, HazardNodePtr& parent, VERIFY_ADDRESS(static_cast(leaf)); - reached_leaf = IsLeaf(leaf); + reached_leaf = leaf->IsLeaf(); } } } template -bool ChromaticTree:: -IsLeaf(const Node* node) const { - return node->GetLeft() == NULL; -} - -template -bool ChromaticTree:: -IsSentinel(const Node* node) const { - return (node == entry_) || (node == entry_->GetLeft()); -} - -template -bool ChromaticTree:: +inline bool ChromaticTree:: HasChild(const Node* parent, const Node* child) const { return (parent->GetLeft() == child || parent->GetRight() == child); } @@ -854,7 +878,7 @@ HasChild(const Node* parent, const Node* child) const { template void ChromaticTree:: Destruct(Node* node) { - if (!IsLeaf(node)) { + if (!node->IsLeaf()) { Destruct(node->GetLeft()); Destruct(node->GetRight()); } @@ -884,7 +908,7 @@ IsBalanced(const Node* node) const { // Overweight violation bool has_violation = node->GetWeight() > 1; - if (!has_violation && !IsLeaf(node)) { + if (!has_violation && !node->IsLeaf()) { const Node* left = node->GetLeft(); const Node* right = node->GetRight(); @@ -994,14 +1018,14 @@ CleanUp(const Key& key) { parent.ProtectSafe(entry_); leaf.ProtectSafe(entry_); - reached_leaf = IsLeaf(leaf); + reached_leaf = leaf->IsLeaf(); while (!reached_leaf && !found_violation) { grandgrandparent.AdoptHazard(grandparent); grandparent.AdoptHazard(parent); parent.AdoptHazard(leaf); AtomicNodePtr& next_leaf = - (IsSentinel(leaf) || compare_(key, leaf->GetKey())) ? + (leaf->IsSentinel() || compare_(key, leaf->GetKey())) ? leaf->GetLeft() : leaf->GetRight(); // Parent is protected, so we can tolerate a changing child pointer @@ -1030,7 +1054,7 @@ CleanUp(const Key& key) { break; } - reached_leaf = IsLeaf(leaf); + reached_leaf = leaf->IsLeaf(); } } diff --git a/containers_cpp/include/embb/containers/lock_free_chromatic_tree.h b/containers_cpp/include/embb/containers/lock_free_chromatic_tree.h index c7052c3..871bd7f 100644 --- a/containers_cpp/include/embb/containers/lock_free_chromatic_tree.h +++ b/containers_cpp/include/embb/containers/lock_free_chromatic_tree.h @@ -33,6 +33,8 @@ #include #include #include +#include +#include namespace embb { namespace containers { @@ -124,6 +126,20 @@ class ChromaticTreeNode { Node* GetRight() const; /** + * Checks if the node is a leaf. + * + * @return \c true if node is a leaf, \c false otherwise + */ + bool IsLeaf() const; + + /** + * Checks if the node is a sentinel. + * + * @return \c true if node is a sentinel, \c false otherwise + */ + bool IsSentinel() const; + + /** * Tries to replace one of the child pointers that compares equal to * \c old_child with the \c new_child using an atomic compare-and-swap * operation. If neither left nor right child pointer is pointing to @@ -166,13 +182,15 @@ class ChromaticTreeNode { ChromaticTreeNode(const ChromaticTreeNode&); ChromaticTreeNode& operator=(const ChromaticTreeNode&); - const Key key_; /**< Stored key. */ - const Value value_; /**< Stored value. */ - const int weight_; /**< Weight of the node. */ - AtomicNodePtr left_; /**< Pointer to left child node. */ - AtomicNodePtr right_; /**< Pointer to right child node. */ - AtomicFlag retired_; /**< Retired (marked for deletion) flag. */ - AtomicOperationPtr operation_; /**< Pointer to a tree operation object. */ + const Key key_; /**< Stored key. */ + const Value value_; /**< Stored value. */ + const int weight_; /**< Weight of the node. */ + const bool is_leaf_; /**< True if node is a leaf. */ + const bool is_sentinel_; /**< True if node is a sentinel. */ + AtomicNodePtr left_; /**< Pointer to left child node. */ + AtomicNodePtr right_; /**< Pointer to right child node. */ + AtomicFlag retired_; /**< Retired (marked for deletion) flag. */ + AtomicOperationPtr operation_; /**< Pointer to a tree operation object. */ }; /** @@ -692,24 +710,6 @@ class ChromaticTree { HazardNodePtr& grandparent); /** - * Checks whether the given node is a leaf. - * - * \param[IN] node Node to be checked - * - * \return \c true if the given node is a leaf, \c false otherwise - */ - bool IsLeaf(const Node* node) const; - - /** - * Checks whether the given node is a sentinel node. - * - * \param[IN] node Node to be checked - * - * \return \c true if the given node is a sentinel node, \c false otherwise - */ - bool IsSentinel(const Node* node) const; - - /** * Checks whether the given node has a specified child node. * * \param[IN] parent Parent node -- libgit2 0.26.0