From ca1f74d3080d126c4645f4296bf393c885ffbbc2 Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Wed, 29 Jul 2020 10:27:20 +0100 Subject: [PATCH] Optimised forward_list move constructor and assignment --- include/etl/forward_list.h | 97 ++++++++++++++++------- test/test_forward_list_shared_pool.cpp | 103 ++++++++++++++++++++++++- 2 files changed, 169 insertions(+), 31 deletions(-) diff --git a/include/etl/forward_list.h b/include/etl/forward_list.h index 1e52befa..93536730 100644 --- a/include/etl/forward_list.h +++ b/include/etl/forward_list.h @@ -1478,23 +1478,50 @@ namespace etl { this->initialise(); - node_t* p_last_node = &start_node; - - etl::iforward_list::iterator first = rhs.begin(); - etl::iforward_list::iterator last = rhs.end(); - - // Add all of the elements. - while (first != last) + if (!rhs.empty()) { - ETL_ASSERT(!full(), ETL_ERROR(forward_list_full)); + node_t* p_last_node = &this->start_node; + node_t* p_rhs_node = rhs.start_node.next; - data_node_t& data_node = this->allocate_data_node(etl::move(*first++)); - join(p_last_node, &data_node); - data_node.next = ETL_NULLPTR; - p_last_node = &data_node; + // Are we using the same pool? + if (this->get_node_pool() == rhs.get_node_pool()) + { + // Just link the nodes to the new forward_list. + do + { + node_t* p_node = p_rhs_node; + p_rhs_node = p_rhs_node->next; + + join(p_last_node, p_node); + p_node->next = ETL_NULLPTR; + p_last_node = p_node; + + ETL_INCREMENT_DEBUG_COUNT; + + } while (p_rhs_node != ETL_NULLPTR); + + rhs.ETL_RESET_DEBUG_COUNT; + rhs.start_node.next = ETL_NULLPTR; + } + else + { + // Add all of the elements. + etl::iforward_list::iterator first = rhs.begin(); + etl::iforward_list::iterator last = rhs.end(); + + while (first != last) + { + ETL_ASSERT(!full(), ETL_ERROR(forward_list_full)); + + data_node_t& data_node = this->allocate_data_node(etl::move(*first++)); + join(p_last_node, &data_node); + data_node.next = ETL_NULLPTR; + p_last_node = &data_node; + } + + rhs.initialise(); + } } - - rhs.initialise(); } } #endif @@ -1764,8 +1791,8 @@ namespace etl //************************************************************************* /// Copy constructor. //************************************************************************* - forward_list(const forward_list& other) - : etl::iforward_list(*other.p_node_pool, other.p_node_pool->max_size(), true) + forward_list(const forward_list& other, etl::ipool& node_pool) + : etl::iforward_list(node_pool, node_pool.max_size(), true) { this->assign(other.cbegin(), other.cend()); } @@ -1774,21 +1801,24 @@ namespace etl //************************************************************************* /// Move constructor. //************************************************************************* - forward_list(forward_list&& other) - : etl::iforward_list(*other.p_node_pool, other.p_node_pool->max_size(), true) + forward_list(forward_list&& other, etl::ipool& node_pool) + : etl::iforward_list(node_pool, node_pool.max_size(), true) { if (this != &other) { - this->initialise(); + this->move_container(std::move(other)); + } + } - typename etl::iforward_list::iterator itr = other.begin(); - while (itr != other.end()) - { - this->push_back(etl::move(*itr)); - ++itr; - } - - other.initialise(); + //************************************************************************* + /// Move constructor. + //************************************************************************* + forward_list(iforward_list&& other, etl::ipool& node_pool) + : etl::iforward_list(node_pool, node_pool.max_size(), true) + { + if (this != &other) + { + this->move_container(std::move(other)); } } #endif @@ -1841,7 +1871,10 @@ namespace etl //************************************************************************* forward_list& operator = (forward_list&& rhs) { - this->move_container(etl::move(rhs)); + if (&rhs != this) + { + this->move_container(etl::move(rhs)); + } return *this; } @@ -1860,6 +1893,14 @@ namespace etl this->set_node_pool(pool); } + + //************************************************************************* + /// Get the pool instance. + //************************************************************************* + etl::ipool& get_pool() const + { + return *p_node_pool; + } }; //************************************************************************* diff --git a/test/test_forward_list_shared_pool.cpp b/test/test_forward_list_shared_pool.cpp index d8bac294..05b42dca 100644 --- a/test/test_forward_list_shared_pool.cpp +++ b/test/test_forward_list_shared_pool.cpp @@ -220,13 +220,39 @@ namespace PoolNDC2 pool; DataNDC data1(sorted_data.begin(), sorted_data.end(), pool); DataNDC data2(unsorted_data.begin(), unsorted_data.end(), pool); - DataNDC other_data1(data1); - DataNDC other_data2(data2); + DataNDC other_data1(data1, pool); + DataNDC other_data2(data2, pool); CHECK(std::equal(data1.begin(), data1.end(), other_data1.begin())); CHECK(std::equal(data2.begin(), data2.end(), other_data2.begin())); } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_constructor) + { + PoolNDC2 pool; + DataNDC data1(sorted_data.begin(), sorted_data.end(), pool); + DataNDC data2(unsorted_data.begin(), unsorted_data.end(), data1.get_pool()); + DataNDC other_data1(std::move(data1), pool); + DataNDC other_data2(std::move(data2), pool); + + CHECK_EQUAL(0U, data1.size()); + CHECK(data1.empty()); + CHECK_EQUAL(pool.max_size(), data1.max_size()); + CHECK(data1.begin() == data1.end()); + + CHECK_EQUAL(0U, data2.size()); + CHECK(data2.empty()); + CHECK_EQUAL(pool.max_size(), data2.max_size()); + CHECK(data2.begin() == data2.end()); + + CHECK_EQUAL(sorted_data.size(), other_data1.size()); + CHECK_EQUAL(unsorted_data.size(), other_data2.size()); + + CHECK(std::equal(sorted_data.begin(), sorted_data.end(), other_data1.begin())); + CHECK(std::equal(unsorted_data.begin(), unsorted_data.end(), other_data2.begin())); + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_iterator) { @@ -1177,6 +1203,77 @@ namespace CHECK(are_equal); } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_assignment) + { + CompareDataNDC compare_data(sorted_data.begin(), sorted_data.end()); + PoolNDC4 pool; + DataNDC data1(sorted_data.begin(), sorted_data.end(), pool); + DataNDC data2(sorted_data.begin(), sorted_data.end(), data1.get_pool()); + DataNDC other_data1(pool); + DataNDC other_data2(pool); + + other_data1 = std::move(data1); + other_data2 = std::move(data2); + + CHECK_EQUAL(0U, data1.size()); + CHECK(data1.empty()); + CHECK_EQUAL(pool.max_size(), data1.max_size()); + CHECK(data1.begin() == data1.end()); + + CHECK_EQUAL(0U, data2.size()); + CHECK(data2.empty()); + CHECK_EQUAL(pool.max_size(), data2.max_size()); + CHECK(data2.begin() == data2.end()); + + CHECK_EQUAL(sorted_data.size(), other_data1.size()); + CHECK_EQUAL(sorted_data.size(), other_data2.size()); + + are_equal = std::equal(sorted_data.begin(), sorted_data.end(), other_data1.begin()); + CHECK(are_equal); + + are_equal = std::equal(sorted_data.begin(), sorted_data.end(), other_data2.begin()); + CHECK(are_equal); + } + + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_move_assignment_through_interface) + { + CompareDataNDC compare_data(sorted_data.begin(), sorted_data.end()); + PoolNDC4 pool; + DataNDC data1(sorted_data.begin(), sorted_data.end(), pool); + DataNDC data2(sorted_data.begin(), sorted_data.end(), data1.get_pool()); + DataNDC other_data1(pool); + DataNDC other_data2(pool); + + IDataNDC& idata1 = data1; + IDataNDC& idata2 = data2; + IDataNDC& iother_data1 = other_data1; + IDataNDC& iother_data2 = other_data2; + + iother_data1 = std::move(idata1); + iother_data2 = std::move(idata2); + + CHECK_EQUAL(0U, data1.size()); + CHECK(data1.empty()); + CHECK_EQUAL(pool.max_size(), data1.max_size()); + CHECK(data1.begin() == data1.end()); + + CHECK_EQUAL(0U, data2.size()); + CHECK(data2.empty()); + CHECK_EQUAL(pool.max_size(), data2.max_size()); + CHECK(data2.begin() == data2.end()); + + CHECK_EQUAL(sorted_data.size(), iother_data1.size()); + CHECK_EQUAL(sorted_data.size(), iother_data2.size()); + + are_equal = std::equal(sorted_data.begin(), sorted_data.end(), iother_data1.begin()); + CHECK(are_equal); + + are_equal = std::equal(sorted_data.begin(), sorted_data.end(), iother_data2.begin()); + CHECK(are_equal); + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_assignment_interface) { @@ -1199,7 +1296,7 @@ namespace CompareDataNDC compare_data(sorted_data.begin(), sorted_data.end()); PoolNDC2 pool; DataNDC data1(sorted_data.begin(), sorted_data.end(), pool); - DataNDC other_data(data1); + DataNDC other_data(data1, pool); other_data = other_data;