From 8ded09ad94ed117b0c992a0e99fc7286bbbf1404 Mon Sep 17 00:00:00 2001 From: Danila Klimenko Date: Tue, 12 May 2015 18:23:44 +0200 Subject: [PATCH] chromatic_tree: Bug fuxes, additional tests --- containers_cpp/include/embb/containers/internal/lock_free_chromatic_tree-inl.h | 50 ++++++++++++++++++++++++++++++++++++++++---------- containers_cpp/include/embb/containers/lock_free_chromatic_tree.h | 33 ++++++++++++++++++++++++++++++--- containers_cpp/test/tree_test-inl.h | 276 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------- containers_cpp/test/tree_test.h | 56 ++++++++++++++++++++++++++++++++++++++------------------ 4 files changed, 334 insertions(+), 81 deletions(-) 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 5d5cca2..6218cfc 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 @@ -36,7 +36,7 @@ namespace internal { template ChromaticTreeNode:: -ChromaticTreeNode(const Key& key, const Value& value, const int& weight, +ChromaticTreeNode(const Key& key, const Value& value, const int weight, ChromaticTreeNode* const & left, ChromaticTreeNode* const & right) : key_(key), @@ -47,10 +47,10 @@ ChromaticTreeNode(const Key& key, const Value& value, const int& weight, template ChromaticTreeNode:: -ChromaticTreeNode(const Key& key, const Value& value) +ChromaticTreeNode(const Key& key, const Value& value, const int weight) : key_(key), value_(value), - weight_(1), + weight_(weight), left_(NULL), right_(NULL) {} @@ -74,7 +74,7 @@ const Value& ChromaticTreeNode::GetValue() const { } template -const int& ChromaticTreeNode::GetWeight() const { +int ChromaticTreeNode::GetWeight() const { return weight_; } @@ -150,19 +150,21 @@ TryInsert(const Key& key, const Value& value, Value& old_value) { NodePtr new_parent; bool added_violation = false; - NodePtr new_leaf = node_pool_.Allocate(key, value); - if (new_leaf == NULL) { - return false; - } - bool keys_are_equal = !(compare_(key, leaf->GetKey()) || compare_(leaf->GetKey(), key)); if (!IsSentinel(leaf) && keys_are_equal) { old_value = leaf->GetValue(); - new_parent = new_leaf; + new_parent = node_pool_.Allocate(key, value, leaf->GetWeight()); + if (new_parent == NULL) { + return false; + } } else { old_value = undefined_value_; + NodePtr new_leaf = node_pool_.Allocate(key, value); + if (new_leaf == NULL) { + return false; + } NodePtr new_sibling = node_pool_.Allocate(*leaf); if (new_sibling == NULL) { node_pool_.Free(new_leaf); @@ -679,6 +681,34 @@ GetHeight(const NodePtr& node) const { return height; } +template +bool ChromaticTree:: +IsBalanced() const { + return IsBalanced(entry_->GetLeft()); +} + +template +bool ChromaticTree:: +IsBalanced(const NodePtr& node) const { + // Overweight violation + bool has_violation = node->GetWeight() > 1; + + if (!has_violation && !IsLeaf(node)) { + NodePtr left = node->GetLeft(); + NodePtr right = node->GetRight(); + + // Red-red violation + has_violation = node->GetWeight() == 0 && + (left->GetWeight() == 0 || right->GetWeight() == 0); + + // Check children + if (!has_violation) { + has_violation = !IsBalanced(left) || !IsBalanced(right); + } + } + + return !has_violation; +} template bool ChromaticTree:: 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 9b191a7..9ead994 100644 --- a/containers_cpp/include/embb/containers/lock_free_chromatic_tree.h +++ b/containers_cpp/include/embb/containers/lock_free_chromatic_tree.h @@ -57,7 +57,7 @@ class ChromaticTreeNode { * \param[IN] left Pointer to the left child node * \param[IN] right Pointer to the right child node */ - ChromaticTreeNode(const Key& key, const Value& value, const int& weight, + ChromaticTreeNode(const Key& key, const Value& value, const int weight, ChromaticTreeNode* const & left, ChromaticTreeNode* const & right); @@ -68,7 +68,7 @@ class ChromaticTreeNode { * \param[IN] key Key of the new node * \param[IN] value Value of the new node */ - ChromaticTreeNode(const Key& key, const Value& value); + ChromaticTreeNode(const Key& key, const Value& value, const int weight = 1); /** * Creates a copy of a given node. @@ -97,7 +97,7 @@ class ChromaticTreeNode { * * \return Weight of the node */ - const int& GetWeight() const; + int GetWeight() const; /** * Accessor for the left child pointer. @@ -123,6 +123,14 @@ class ChromaticTreeNode { } // namespace internal +namespace test { +/** + * Forward declaration of the test class + */ +template +class TreeTest; + +} // namespace test /** * Chromatic balanced binary search tree @@ -148,6 +156,15 @@ template(2 * capacity + 7) tree nodes each of size @@ -447,7 +464,14 @@ class ChromaticTree { const NodePtr& uxl, const NodePtr& uxr); /** + * Friending the test class for white-box testing + */ + friend class test::TreeTest >; + + /** * Computes the hight of the subtree rooted at the given node. + * + * \notthreadsafe * * \param[IN] node Root of the subtree for which the height is requested * @@ -456,6 +480,9 @@ class ChromaticTree { */ int GetHeight(const NodePtr& node) const; + bool IsBalanced() const; + bool IsBalanced(const NodePtr& node) const; + const Key undefined_key_; /**< A dummy key used by the tree */ const Value undefined_value_; /**< A dummy value used by the tree */ const Compare compare_; /**< Comparator object for the keys */ diff --git a/containers_cpp/test/tree_test-inl.h b/containers_cpp/test/tree_test-inl.h index 42e1252..0873444 100644 --- a/containers_cpp/test/tree_test-inl.h +++ b/containers_cpp/test/tree_test-inl.h @@ -39,116 +39,292 @@ namespace test { template TreeTest::TreeTest() - : tree_(NULL) { - // Repeat twice to ensure that the tree remains operational after all the - // elements were removed from it + : tree_(NULL), + bad_key_(static_cast(-1)), + bad_value_(static_cast(-1)) { CreateUnit("TreeTestSingleThreadInsertDelete"). - Pre(&TreeTest::TreeTestSingleThreadInsertDelete_Pre, this). - Add(&TreeTest::TreeTestSingleThreadInsertDelete_ThreadMethod, this, 1, 2). - Post(&TreeTest::TreeTestSingleThreadInsertDelete_Post, this); + Pre(&TreeTest::TreeTestInsertDelete_Pre, this). + Add(&TreeTest::TreeTestInsertDeleteSingleThread_ThreadMethod, + this, 1, NUM_ITERATIONS). + Post(&TreeTest::TreeTestInsertDelete_Post, this); CreateUnit("TreeTestMultiThreadInsertDelete"). - Pre(&TreeTest::TreeTestSingleThreadInsertDelete_Pre, this). - Add(&TreeTest::TreeTestMultiThreadInsertDelete_ThreadMethod, this, - NUM_TEST_THREADS, 2). - Post(&TreeTest::TreeTestSingleThreadInsertDelete_Post, this); + Pre(&TreeTest::TreeTestInsertDelete_Pre, this). + Add(&TreeTest::TreeTestInsertDeleteMultiThread_ThreadMethod, this, + NUM_TEST_THREADS, NUM_ITERATIONS). + Post(&TreeTest::TreeTestInsertDelete_Post, this); + CreateUnit("TreeTestConcurrentGet"). + Pre(&TreeTest::TreeTestConcurrentGet_Pre, this). + Add(&TreeTest::TreeTestConcurrentGet_WriterMethod, this, + NUM_TEST_THREADS / 2, NUM_ITERATIONS). + Add(&TreeTest::TreeTestConcurrentGet_ReaderMethod, this, + NUM_TEST_THREADS / 2, NUM_ITERATIONS). + Post(&TreeTest::TreeTestConcurrentGet_Post, this); + CreateUnit("TreeTestConcurrentGetMinimal"). + Pre(&TreeTest::TreeTestConcurrentGetMinimal_Pre, this). + Add(&TreeTest::TreeTestConcurrentGet_WriterMethod, this, + NUM_TEST_THREADS / 2, NUM_ITERATIONS). + Add(&TreeTest::TreeTestConcurrentGet_ReaderMethod, this, + NUM_TEST_THREADS / 2, NUM_ITERATIONS). + Post(&TreeTest::TreeTestConcurrentGet_Post, this); + CreateUnit("TreeTestBalance"). + Pre(&TreeTest::TreeTestBalance_Pre, this). + Add(&TreeTest::TreeTestBalance_ThreadMethod, this, + NUM_TEST_THREADS, 1). + Post(&TreeTest::TreeTestBalance_Post, this); } template TreeTest::Worker:: -Worker(Tree& tree, size_t thread_id, int num_elements) - : tree_(tree), thread_id_(thread_id), num_elements_(num_elements) {} +Worker(TreePtr tree, int thread_id) + : tree_(tree), thread_id_(thread_id) {} template void TreeTest::Worker:: -Run() { - ElementVector elements; - for (int i = 0; i < num_elements_; ++i) { - Key key = i * 133 * 100 + thread_id_; - Value value = i * 133 * 100 + thread_id_; - elements.push_back(std::make_pair(key, value)); +InsertReplaceDelete(int num_elements) { + PrepareElements(num_elements); + InsertAll(); + ReplaceHalf(); + DeleteAll(); +} + +template +void TreeTest::Worker:: +PrepareElements(int num_elements) { + // Fill the "elements_" vector + elements_.clear(); + for (int i = 0; i < num_elements; ++i) { + Key key = static_cast(i * 100 + thread_id_); + Value value = static_cast(i * 100 + thread_id_); + elements_.push_back(std::make_pair(key, value)); } +} +template +void TreeTest::Worker:: +InsertAll() { // Insert elements into the tree - ::std::random_shuffle(elements.begin(), elements.end()); - for (ElementIterator it = elements.begin(); it != elements.end(); ++it) { - bool success = tree_.TryInsert(it->first, it->second); + ::std::random_shuffle(elements_.begin(), elements_.end()); + for (ElementIterator it = elements_.begin(); it != elements_.end(); ++it) { + Value old_value; + Value bad_value = tree_->GetUndefinedValue(); + bool success = tree_->TryInsert(it->first, it->second, old_value); PT_ASSERT_MSG(success, "Failed to insert element into the tree."); + PT_ASSERT_EQ_MSG(old_value, bad_value, "A key was already in the tree."); } // Verify that all inserted elements are available in the tree - ::std::random_shuffle(elements.begin(), elements.end()); - for (ElementIterator it = elements.begin(); it != elements.end(); ++it) { + ::std::random_shuffle(elements_.begin(), elements_.end()); + for (ElementIterator it = elements_.begin(); it != elements_.end(); ++it) { Value value; - bool success = tree_.Get(it->first, value); + bool success = tree_->Get(it->first, value); PT_ASSERT_MSG(success, "Failed to get an element from the tree."); - PT_ASSERT_MSG(it->second == value, "Wrong value retrieved from the tree."); + PT_ASSERT_EQ_MSG(it->second, value, "Wrong value retrieved from the tree."); } +} +template +void TreeTest::Worker:: +ReplaceHalf() { // Replace some of the elements that were inserted earlier - ::std::random_shuffle(elements.begin(), elements.end()); - ElementIterator elements_middle = elements.begin() + num_elements_ / 2; - for (ElementIterator it = elements.begin(); it != elements_middle; ++it) { + ::std::random_shuffle(elements_.begin(), elements_.end()); + ElementIterator elements_middle = elements_.begin() + elements_.size() / 2; + for (ElementIterator it = elements_.begin(); it != elements_middle; ++it) { + Value old_value; + Value expected = it->second; it->second *= 13; - bool success = tree_.TryInsert(it->first, it->second); + bool success = tree_->TryInsert(it->first, it->second, old_value); PT_ASSERT_MSG(success, "Failed to insert element into the tree."); + PT_ASSERT_EQ_MSG(old_value, expected, "Wrong value replaced in the tree."); } // Verify again that all elements are in the tree and have correct values - ::std::random_shuffle(elements.begin(), elements.end()); - for (ElementIterator it = elements.begin(); it != elements.end(); ++it) { + ::std::random_shuffle(elements_.begin(), elements_.end()); + for (ElementIterator it = elements_.begin(); it != elements_.end(); ++it) { Value value; - bool success = tree_.Get(it->first, value); + bool success = tree_->Get(it->first, value); PT_ASSERT_MSG(success, "Failed to get an element from the tree."); - PT_ASSERT_MSG(it->second == value, "Wrong value retrieved from the tree."); + PT_ASSERT_EQ_MSG(it->second, value, "Wrong value retrieved from the tree."); } +} +template +void TreeTest::Worker:: +DeleteAll() { // Delete elements from the tree - ::std::random_shuffle(elements.begin(), elements.end()); - for (ElementIterator it = elements.begin(); it != elements.end(); ++it) { - Value value; - bool success = tree_.TryDelete(it->first, value); + ::std::random_shuffle(elements_.begin(), elements_.end()); + for (ElementIterator it = elements_.begin(); it != elements_.end(); ++it) { + Value old_value; + Value expected = it->second; + bool success = tree_->TryDelete(it->first, old_value); PT_ASSERT_MSG(success, "Failed to delete element from the tree."); - PT_ASSERT_MSG(it->second == value, "Wrong value deleted from the tree."); + PT_ASSERT_EQ_MSG(expected, old_value, "Wrong value deleted from the tree."); } } template -void TreeTest:: -TreeTestSingleThreadInsertDelete_Pre() { - tree_ = new Tree(TREE_CAPACITY); +void TreeTest::Worker:: +DeleteHalf() { + // Delete half of the elements from the tree + ::std::random_shuffle(elements_.begin(), elements_.end()); + ElementIterator elements_middle = elements_.begin() + elements_.size() / 2; + for (ElementIterator it = elements_.begin(); it != elements_middle; ++it) { + Value old_value; + Value expected = it->second; + bool success = tree_->TryDelete(it->first, old_value); + PT_ASSERT_MSG(success, "Failed to delete element from the tree."); + PT_ASSERT_EQ_MSG(expected, old_value, "Wrong value deleted from the tree."); + } } template void TreeTest:: -TreeTestSingleThreadInsertDelete_ThreadMethod() { - size_t thread_id = partest::TestSuite::GetCurrentThreadID(); +TreeTestInsertDelete_Pre() { + tree_ = new Tree(TREE_CAPACITY, bad_key_, bad_value_); +} - Worker worker(*tree_, thread_id, TREE_CAPACITY); +template +void TreeTest:: +TreeTestInsertDeleteSingleThread_ThreadMethod() { + Worker worker(tree_, 0); - worker.Run(); + for (int i = 1; i <= 10; ++i) { + worker.InsertReplaceDelete(i * (TREE_CAPACITY / 10)); + } } template void TreeTest:: -TreeTestMultiThreadInsertDelete_ThreadMethod() { - size_t thread_id = partest::TestSuite::GetCurrentThreadID(); +TreeTestInsertDeleteMultiThread_ThreadMethod() { + int thread_id = static_cast(partest::TestSuite::GetCurrentThreadID()); + thread_id %= NUM_TEST_THREADS; int num_elements = TREE_CAPACITY / (NUM_TEST_THREADS + 1); if (thread_id == 0) { num_elements *= 2; } - Worker worker(*tree_, thread_id, num_elements); - worker.Run(); + Worker worker(tree_, thread_id); + + for (int i = 1; i <= 10; ++i) { + worker.InsertReplaceDelete(i * (num_elements / 10)); + } } template void TreeTest:: -TreeTestSingleThreadInsertDelete_Post() { +TreeTestInsertDelete_Post() { PT_ASSERT_MSG((tree_->IsEmpty()), "The tree must be empty at this point."); delete tree_; } +template +void TreeTest:: +TreeTestConcurrentGet_Pre() { + tree_ = new Tree(TREE_CAPACITY / 2, bad_key_, bad_value_); + + ElementVector elements; + for (int i = 0; i < TREE_CAPACITY / 2; ++i) { + Key key = static_cast(i); + Value value = static_cast(i); + elements.push_back(std::make_pair(key, value)); + } + + ::std::random_shuffle(elements.begin(), elements.end()); + for (ElementIterator it = elements.begin(); it != elements.end(); ++it) { + Value old_value; + Value bad_value = tree_->GetUndefinedValue(); + bool success = tree_->TryInsert(it->first, it->second, old_value); + PT_ASSERT_MSG(success, "Failed to insert element into the tree."); + PT_ASSERT_EQ_MSG(old_value, bad_value, "A key was already in the tree."); + } +} + +template +void TreeTest:: +TreeTestConcurrentGetMinimal_Pre() { + tree_ = new Tree(NUM_TEST_THREADS / 2, bad_key_, bad_value_); + + for (int i = 0; i < NUM_TEST_THREADS / 2; ++i) { + Key key = static_cast(TREE_CAPACITY / 4 + i); + Value value = static_cast(TREE_CAPACITY / 4 + i); + Value old_value; + Value bad_value = tree_->GetUndefinedValue(); + bool success = tree_->TryInsert(key, value, old_value); + PT_ASSERT_MSG(success, "Failed to insert element into the tree."); + PT_ASSERT_EQ_MSG(old_value, bad_value, "A key was already in the tree."); + } +} + +template +void TreeTest:: +TreeTestConcurrentGet_WriterMethod() { + int idx = static_cast(partest::TestSuite::GetCurrentThreadID()); + idx %= (NUM_TEST_THREADS / 2); + + Key key = static_cast(TREE_CAPACITY / 4 + idx); + Value value = static_cast(TREE_CAPACITY / 4 + idx); + + for (int i = 0; i < 1000; ++i) { + Value old_value; + bool success = tree_->TryDelete(key, old_value); + PT_ASSERT_MSG(success, "Failed to delete element from the tree."); + PT_ASSERT_EQ_MSG(old_value, value, "Wrong value deleted from the tree."); + + Value bad_value = tree_->GetUndefinedValue(); + success = tree_->TryInsert(key, value, old_value); + PT_ASSERT_MSG(success, "Failed to insert element into the tree."); + PT_ASSERT_EQ_MSG(old_value, bad_value, "A key was already in the tree."); + } +} + +template +void TreeTest:: +TreeTestConcurrentGet_ReaderMethod() { + int idx = static_cast(partest::TestSuite::GetCurrentThreadID()); + idx %= (NUM_TEST_THREADS / 2); + + Key key = static_cast(TREE_CAPACITY / 4 + idx); + + for (int i = 0; i < 1000; ++i) { + Value value; + tree_->Get(key, value); + } +} + +template +void TreeTest:: +TreeTestConcurrentGet_Post() { + delete tree_; +} + +template +void TreeTest:: +TreeTestBalance_Pre() { + tree_ = new Tree(TREE_CAPACITY, bad_key_, bad_value_); +} + +template +void TreeTest:: +TreeTestBalance_ThreadMethod() { + int thread_id = static_cast(partest::TestSuite::GetCurrentThreadID()); + thread_id %= NUM_TEST_THREADS; + + int num_elements = TREE_CAPACITY / NUM_TEST_THREADS; + + Worker worker(tree_, thread_id); + + worker.PrepareElements(num_elements); + worker.InsertAll(); + worker.DeleteHalf(); +} + +template +void TreeTest:: +TreeTestBalance_Post() { + PT_ASSERT_MSG((tree_->IsBalanced()), "The tree must balanced at this point."); + delete tree_; +} + } // namespace test } // namespace containers } // namespace embb diff --git a/containers_cpp/test/tree_test.h b/containers_cpp/test/tree_test.h index 1c42805..9fac203 100644 --- a/containers_cpp/test/tree_test.h +++ b/containers_cpp/test/tree_test.h @@ -37,32 +37,52 @@ class TreeTest : public partest::TestCase { TreeTest(); private: + typedef Tree* TreePtr; + typedef typename Tree::KeyType Key; + typedef typename Tree::ValueType Value; + typedef ::std::pair Element; + typedef ::std::vector ElementVector; + typedef typename ElementVector::iterator ElementIterator; + class Worker { public: - Worker(Tree& tree, size_t thread_id, int num_elements); - void Run(); + Worker(TreePtr tree, int thread_id); + void InsertReplaceDelete(int num_elements); + void PrepareElements(int num_elements); + void InsertAll(); + void ReplaceHalf(); + void DeleteAll(); + void DeleteHalf(); private: - Tree& tree_; - size_t thread_id_; - int num_elements_; - }; + Worker(const Worker&); + Worker& operator=(const Worker&); - typedef int Key; - typedef int Value; - typedef ::std::pair Element; - typedef ::std::vector ElementVector; - typedef ElementVector::iterator ElementIterator; + TreePtr tree_; + int thread_id_; + ElementVector elements_; + }; - static const int TREE_CAPACITY = 2000; - static const int NUM_TEST_THREADS = 3; + static const int TREE_CAPACITY = 100; + static const int NUM_TEST_THREADS = 4; + static const int NUM_ITERATIONS = 100; - void TreeTestSingleThreadInsertDelete_Pre(); - void TreeTestSingleThreadInsertDelete_ThreadMethod(); - void TreeTestMultiThreadInsertDelete_ThreadMethod(); - void TreeTestSingleThreadInsertDelete_Post(); + void TreeTestInsertDelete_Pre(); + void TreeTestInsertDeleteSingleThread_ThreadMethod(); + void TreeTestInsertDeleteMultiThread_ThreadMethod(); + void TreeTestInsertDelete_Post(); + void TreeTestConcurrentGet_Pre(); + void TreeTestConcurrentGetMinimal_Pre(); + void TreeTestConcurrentGet_WriterMethod(); + void TreeTestConcurrentGet_ReaderMethod(); + void TreeTestConcurrentGet_Post(); + void TreeTestBalance_Pre(); + void TreeTestBalance_ThreadMethod(); + void TreeTestBalance_Post(); - Tree *tree_; + TreePtr tree_; + Key bad_key_; + Value bad_value_; }; } // namespace test -- libgit2 0.26.0