From 6c4a29b498d27ecaec3adf5b4d3801998b1f89e0 Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Fri, 30 Dec 2016 11:56:08 +0000 Subject: [PATCH 1/2] Fixed bug in erase() --- src/iunordered_map.h | 11 +++++++++++ src/iunordered_multimap.h | 11 +++++++++++ src/iunordered_multiset.h | 11 +++++++++++ src/iunordered_set.h | 11 +++++++++++ test/test_unordered_map.cpp | 9 +++++++++ test/test_unordered_multimap.cpp | 9 +++++++++ test/test_unordered_multiset.cpp | 9 +++++++++ test/test_unordered_set.cpp | 9 +++++++++ 8 files changed, 80 insertions(+) diff --git a/src/iunordered_map.h b/src/iunordered_map.h index 2c1ff1bb..bfd5ef6c 100644 --- a/src/iunordered_map.h +++ b/src/iunordered_map.h @@ -872,6 +872,7 @@ namespace etl if (icurrent != bucket.end()) { bucket.erase_after(iprevious); + pnodepool->release(*icurrent); count = 1; } @@ -899,6 +900,7 @@ namespace etl } bucket.erase_after(iprevious); + pnodepool->release(*icurrent); return inext; } @@ -940,8 +942,17 @@ namespace etl } // Erase the range. + local_iterator irelease = iprevious; + ++irelease; + ibucket->erase_after(iprevious, iend); + while (irelease != iend) + { + pnodepool->release(*irelease); + ++irelease; + } + // At the end of this bucket? if (iend == ibucket->end()) { diff --git a/src/iunordered_multimap.h b/src/iunordered_multimap.h index 6b62924a..6f369585 100644 --- a/src/iunordered_multimap.h +++ b/src/iunordered_multimap.h @@ -758,6 +758,7 @@ namespace etl if (icurrent->key_value_pair.first == key) { bucket.erase_after(iprevious); + pnodepool->release(*icurrent); ++count; icurrent = iprevious; } @@ -793,6 +794,7 @@ namespace etl } bucket.erase_after(iprevious); + pnodepool->release(*icurrent); return inext; } @@ -834,8 +836,17 @@ namespace etl } // Erase the range. + local_iterator irelease = iprevious; + ++irelease; + ibucket->erase_after(iprevious, iend); + while (irelease != iend) + { + pnodepool->release(*irelease); + ++irelease; + } + // At the end of this bucket? if (iend == ibucket->end()) { diff --git a/src/iunordered_multiset.h b/src/iunordered_multiset.h index 29a75fc3..994c1e9d 100644 --- a/src/iunordered_multiset.h +++ b/src/iunordered_multiset.h @@ -750,6 +750,7 @@ namespace etl if (icurrent->key == key) { bucket.erase_after(iprevious); + pnodepool->release(*icurrent); ++count; icurrent = iprevious; } @@ -785,6 +786,7 @@ namespace etl } bucket.erase_after(iprevious); + pnodepool->release(*icurrent); return inext; } @@ -826,8 +828,17 @@ namespace etl } // Erase the range. + local_iterator irelease = iprevious; + ++irelease; + ibucket->erase_after(iprevious, iend); + while (irelease != iend) + { + pnodepool->release(*irelease); + ++irelease; + } + // At the end of this bucket? if (iend == ibucket->end()) { diff --git a/src/iunordered_set.h b/src/iunordered_set.h index af6c90c8..d47ef3c6 100644 --- a/src/iunordered_set.h +++ b/src/iunordered_set.h @@ -758,6 +758,7 @@ namespace etl if (icurrent != bucket.end()) { bucket.erase_after(iprevious); + pnodepool->release(*icurrent); count = 1; } @@ -785,6 +786,7 @@ namespace etl } bucket.erase_after(iprevious); + pnodepool->release(*icurrent); return inext; } @@ -826,8 +828,17 @@ namespace etl } // Erase the range. + local_iterator irelease = iprevious; + ++irelease; + ibucket->erase_after(iprevious, iend); + while (irelease != iend) + { + pnodepool->release(*irelease); + ++irelease; + } + // At the end of this bucket? if (iend == ibucket->end()) { diff --git a/test/test_unordered_map.cpp b/test/test_unordered_map.cpp index 9f33b8d7..22ffa1ee 100644 --- a/test/test_unordered_map.cpp +++ b/test/test_unordered_map.cpp @@ -435,6 +435,10 @@ namespace DataNDC::iterator idata = data.find(K5); CHECK(idata == data.end()); + + // Test that erase really does erase from the pool. + CHECK(!data.full()); + CHECK(!data.empty()); } //************************************************************************* @@ -462,6 +466,11 @@ namespace DataNDC::iterator idata_end = data.find(K8); idata = data.erase(idata, idata_end); // Erase K5, K6, K7 + + CHECK_EQUAL(initial_data.size() - 3, data.size()); + CHECK(!data.full()); + CHECK(!data.empty()); + CHECK(idata == data.find(K8)); idata = data.find(K0); diff --git a/test/test_unordered_multimap.cpp b/test/test_unordered_multimap.cpp index fe595ad4..97d570c9 100644 --- a/test/test_unordered_multimap.cpp +++ b/test/test_unordered_multimap.cpp @@ -396,6 +396,10 @@ namespace CHECK(idata == data.end()); CHECK(inext == iafter); + + // Test that erase really does erase from the pool. + CHECK(!data.full()); + CHECK(!data.empty()); } //************************************************************************* @@ -407,6 +411,11 @@ namespace DataNDC::iterator idata_end = data.find(K8); idata = data.erase(idata, idata_end); // Erase K5, K6, K7 + + CHECK_EQUAL(initial_data.size() - 3, data.size()); + CHECK(!data.full()); + CHECK(!data.empty()); + CHECK(idata == data.find(K8)); idata = data.find(K0); diff --git a/test/test_unordered_multiset.cpp b/test/test_unordered_multiset.cpp index 1800db01..c39a06df 100644 --- a/test/test_unordered_multiset.cpp +++ b/test/test_unordered_multiset.cpp @@ -321,6 +321,10 @@ namespace CHECK(idata == data.end()); CHECK(inext == iafter); + + // Test that erase really does erase from the pool. + CHECK(!data.full()); + CHECK(!data.empty()); } //************************************************************************* @@ -336,6 +340,11 @@ namespace test.assign(data.begin(), data.end()); idata = data.erase(idata, idata_end); // Erase N5, N6, N7 + + CHECK_EQUAL(initial_data.size() - 3, data.size()); + CHECK(!data.full()); + CHECK(!data.empty()); + CHECK(idata == data.find(N8)); test.assign(data.begin(), data.end()); diff --git a/test/test_unordered_set.cpp b/test/test_unordered_set.cpp index 919a06f7..ac152d8f 100644 --- a/test/test_unordered_set.cpp +++ b/test/test_unordered_set.cpp @@ -301,6 +301,10 @@ namespace CHECK(idata == data.end()); CHECK(inext == iafter); + + // Test that erase really does erase from the pool. + CHECK(!data.full()); + CHECK(!data.empty()); } //************************************************************************* @@ -316,6 +320,11 @@ namespace test.assign(data.begin(), data.end()); idata = data.erase(idata, idata_end); // Erase N5, N6, N7 + + CHECK_EQUAL(initial_data.size() - 3, data.size()); + CHECK(!data.full()); + CHECK(!data.empty()); + CHECK(idata == data.find(N8)); test.assign(data.begin(), data.end()); From 0f05e7d4be67a2b277c71562adbb70545c40b71e Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Fri, 30 Dec 2016 11:57:00 +0000 Subject: [PATCH 2/2] Improved erase_after --- src/intrusive_forward_list.h | 39 ++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/intrusive_forward_list.h b/src/intrusive_forward_list.h index 43f07e05..85f67169 100644 --- a/src/intrusive_forward_list.h +++ b/src/intrusive_forward_list.h @@ -527,7 +527,6 @@ namespace etl //************************************************************************* /// Erases the value at the specified position. - /// Clears the link after erasing if CHECKED. //************************************************************************* iterator erase_after(iterator position) { @@ -544,36 +543,32 @@ namespace etl //************************************************************************* /// Erases a range of elements. - /// Clears the links after erasing if CHECKED. //************************************************************************* iterator erase_after(iterator first, iterator last) { - link_type* p_first = first.p_value; - link_type* p_last = last.p_value; - link_type* p_next = p_first->etl_next; - - // Join the ends. - etl::link(p_first, p_last); - - p_first = p_next; - - // Erase the ones in between. - while (p_first != p_last) + if (first != last) { - // One less. - --current_size; + current_size -= std::distance(first, last) - 1; - p_next = p_first->etl_next; // Remember the next link. - p_first = p_next; // Move to the next link. - } + link_type* p_first = first.p_value; + link_type* p_last = last.p_value; + link_type* p_next = p_first->etl_next; - if (p_next == nullptr) - { - return end(); + // Join the ends. + etl::link(p_first, p_last); + + if (p_next == nullptr) + { + return end(); + } + else + { + return last; + } } else { - return iterator(*static_cast(p_last)); + return last; } }