Merge pull request #1800 from ApexAI/iox-1794-refactor-unique-merge-s…

…orted-containers iox-#1794 Move `uniqueMergeSortedContainers` to `WaitSet`
eclipse-iceoryx · Jan 19, 2023 · 754e13d · 754e13d
2 parents 43f4eaf + 4dde9a7
commit 754e13d
Show file tree

Hide file tree

Showing 8 changed files with 276 additions and 271 deletions.
diff --git a/doc/website/release-notes/iceoryx-unreleased.md b/doc/website/release-notes/iceoryx-unreleased.md
@@ -108,6 +108,7 @@
 - Replace uses of `std::cout`, `std::cerr` with the iceoryx logger [\#1756](https://github.com/eclipse-iceoryx/iceoryx/issues/1756)
 - Move `IOX_NO_DISCARD`, `IOX_FALLTHROUGH` and `IOX_MAYBE_UNUSED` to `iceoryx_platform` [\#1726](https://github.com/eclipse-iceoryx/iceoryx/issues/1726)
 - Move `cxx::static_storage` from `iceoryx_hoofs` to `iceoryx_dust` [\#1732](https://github.com/eclipse-iceoryx/iceoryx/issues/1732)
+- Remove `algorithm::uniqueMergeSortedContainers` from `algorithm.hpp`
 
 **Workflow:**
 

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/algorithm.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/algorithm.hpp
@@ -115,15 +115,6 @@ inline constexpr bool doesContainValue(const T) noexcept;
 template <typename T, typename... ValueList>
 inline constexpr bool
 doesContainValue(const T value, const T firstValueListEntry, const ValueList... remainingValueListEntries) noexcept;
-
-/// @brief Merging two sorted containers so that the result is a sorted container
-///        where every element is contained only once
-/// @tparam[in] Container container type which has to support emplace_back() and size()
-/// @param[in] v1 the first sorted input container
-/// @param[in] v2 the second sorted input container
-/// @return sorted container which contains the elements of v1 and v2 and where every element is unique
-template <typename Container>
-Container uniqueMergeSortedContainers(const Container& v1, const Container& v2) noexcept;
 } // namespace algorithm
 } // namespace iox
 

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/vector.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/vector.hpp
@@ -40,6 +40,7 @@ template <typename T, uint64_t Capacity>
 class vector final
 {
   public:
+    using value_type = T;
     using iterator = T*;
     using const_iterator = const T*;
 

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/algorithm.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/algorithm.inl
@@ -85,49 +85,6 @@ doesContainValue(const T value, const T firstValueListEntry, const ValueList...
     return (value == firstValueListEntry) ? true : doesContainValue(value, remainingValueListEntries...);
 }
 
-template <typename Container>
-inline Container uniqueMergeSortedContainers(const Container& v1, const Container& v2) noexcept
-{
-    Container mergedContainer;
-    uint64_t indexV1{0U};
-    uint64_t indexV2{0U};
-    const uint64_t v1Size{v1.size()};
-    const uint64_t v2Size{v2.size()};
-
-    while ((indexV1 < v1Size) && (indexV2 < v2Size))
-    {
-        if (v1[indexV1] == v2[indexV2])
-        {
-            IOX_DISCARD_RESULT(mergedContainer.emplace_back(v1[indexV1]));
-            ++indexV1;
-            ++indexV2;
-        }
-        else if (v1[indexV1] < v2[indexV2])
-        {
-            IOX_DISCARD_RESULT(mergedContainer.emplace_back(v1[indexV1]));
-            ++indexV1;
-        }
-        else
-        {
-            IOX_DISCARD_RESULT(mergedContainer.emplace_back(v2[indexV2]));
-            ++indexV2;
-        }
-    }
-
-    while (indexV2 < v2Size)
-    {
-        IOX_DISCARD_RESULT(mergedContainer.emplace_back(v2[indexV2++]));
-    }
-
-    while (indexV1 < v1Size)
-    {
-        IOX_DISCARD_RESULT(mergedContainer.emplace_back(v1[indexV1++]));
-    }
-
-    return mergedContainer;
-}
-
-
 } // namespace algorithm
 } // namespace iox
 

diff --git a/iceoryx_hoofs/test/moduletests/test_cxx_algorithm.cpp b/iceoryx_hoofs/test/moduletests/test_cxx_algorithm.cpp
@@ -113,220 +113,4 @@ TEST_F(algorithm_test, DoesContainValue_ValueListOfMultipleValuesDoesContainValu
     ::testing::Test::RecordProperty("TEST_ID", "64c87a80-e83b-4e70-8f76-476f24804f19");
     EXPECT_TRUE(doesContainValue(7353, 42, 73, 7353));
 }
-
-TEST_F(algorithm_test, MergeTwoDisjunctNonEmptySortedContainers)
-{
-    ::testing::Test::RecordProperty("TEST_ID", "4f39641f-de8a-434a-8a50-cd2b66b476da");
-    constexpr uint64_t OFFSET = 1337U;
-    constexpr uint64_t VECTOR_SIZE = 10U;
-    vector<uint64_t, VECTOR_SIZE> first;
-    vector<uint64_t, VECTOR_SIZE> second;
-
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        first.emplace_back(i + OFFSET);
-    }
-
-    for (uint64_t i = VECTOR_SIZE / 2U; i < VECTOR_SIZE; ++i)
-    {
-        second.emplace_back(i + OFFSET);
-    }
-
-    auto mergedContainer = uniqueMergeSortedContainers(first, second);
-    auto mergedContainerSwitched = uniqueMergeSortedContainers(second, first);
-
-    ASSERT_THAT(mergedContainer.size(), Eq(VECTOR_SIZE));
-    for (uint64_t i = 0U; i < VECTOR_SIZE; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i + OFFSET));
-    }
-    EXPECT_TRUE(mergedContainer == mergedContainerSwitched);
-}
-
-TEST_F(algorithm_test, MergeTwoDisjunctNonEmptySortedContainersWithAGap)
-{
-    ::testing::Test::RecordProperty("TEST_ID", "15d3c063-8bc5-47eb-84a4-35f055a1d82c");
-    constexpr uint64_t OFFSET = 41U;
-    constexpr uint64_t GAP = 13U;
-    constexpr uint64_t VECTOR_SIZE = 10U;
-    vector<uint64_t, VECTOR_SIZE> first;
-    vector<uint64_t, VECTOR_SIZE> second;
-
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        first.emplace_back(i + OFFSET);
-    }
-
-    for (uint64_t i = VECTOR_SIZE / 2U; i < VECTOR_SIZE; ++i)
-    {
-        second.emplace_back(i + OFFSET + GAP);
-    }
-
-    auto mergedContainer = uniqueMergeSortedContainers(first, second);
-    auto mergedContainerSwitched = uniqueMergeSortedContainers(second, first);
-
-    ASSERT_THAT(mergedContainer.size(), Eq(VECTOR_SIZE));
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i + OFFSET));
-    }
-    for (uint64_t i = VECTOR_SIZE / 2U; i < VECTOR_SIZE; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i + OFFSET + GAP));
-    }
-    EXPECT_TRUE(mergedContainer == mergedContainerSwitched);
-}
-
-TEST_F(algorithm_test, MergeTwoAlternatingDisjunctNonEmptySortedContainers)
-{
-    ::testing::Test::RecordProperty("TEST_ID", "02cc9514-6cfe-4e08-8806-f371561fef41");
-    constexpr uint64_t OFFSET = 4301U;
-    constexpr uint64_t VECTOR_SIZE = 10U;
-    vector<uint64_t, VECTOR_SIZE> first;
-    vector<uint64_t, VECTOR_SIZE> second;
-
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        first.emplace_back(i * 2 + OFFSET);
-    }
-
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        second.emplace_back(i * 2 + 1 + OFFSET);
-    }
-
-    auto mergedContainer = uniqueMergeSortedContainers(first, second);
-    auto mergedContainerSwitched = uniqueMergeSortedContainers(second, first);
-
-    ASSERT_THAT(mergedContainer.size(), Eq(VECTOR_SIZE));
-    for (uint64_t i = 0; i < VECTOR_SIZE; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i + OFFSET));
-    }
-    EXPECT_TRUE(mergedContainer == mergedContainerSwitched);
-}
-
-TEST_F(algorithm_test, MergingIdenticalContainerResultsInUnchangedContainer)
-{
-    ::testing::Test::RecordProperty("TEST_ID", "50f05cf2-62fa-49b8-8380-1dd0ac2470ec");
-    constexpr uint64_t OFFSET = 313U;
-    constexpr uint64_t VECTOR_SIZE = 10U;
-    vector<uint64_t, VECTOR_SIZE> someContainer;
-
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        someContainer.emplace_back(i * 2 + OFFSET);
-    }
-
-    auto mergedContainer = uniqueMergeSortedContainers(someContainer, someContainer);
-
-    ASSERT_THAT(mergedContainer.size(), Eq(VECTOR_SIZE / 2U));
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i * 2 + OFFSET));
-    }
-}
-
-TEST_F(algorithm_test, MergingWithOneEmptyContainerResultsInUnchangedContainer)
-{
-    ::testing::Test::RecordProperty("TEST_ID", "b0a0eb3a-08a3-4898-a8c9-a4f7eff0115c");
-    constexpr uint64_t OFFSET = 707U;
-    constexpr uint64_t VECTOR_SIZE = 10U;
-    vector<uint64_t, VECTOR_SIZE> someContainer;
-
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        someContainer.emplace_back(i * 3 + OFFSET);
-    }
-
-    auto mergedContainer = uniqueMergeSortedContainers(someContainer, vector<uint64_t, VECTOR_SIZE>());
-
-    ASSERT_THAT(mergedContainer.size(), Eq(5U));
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i * 3 + OFFSET));
-    }
-}
-
-TEST_F(algorithm_test, MergePartiallyOverlappingSortedContainers)
-{
-    ::testing::Test::RecordProperty("TEST_ID", "c57dda77-81a5-413f-b54b-e924e67b66a5");
-    constexpr uint64_t VECTOR_SIZE = 10U;
-    constexpr uint64_t MAX_OVERLAPPING_INDEX = 8U;
-    constexpr uint64_t OFFSET = 8055U;
-    vector<uint64_t, VECTOR_SIZE> first;
-    vector<uint64_t, VECTOR_SIZE> second;
-
-    for (uint64_t i = 3U; i < VECTOR_SIZE; ++i)
-    {
-        first.emplace_back(i + OFFSET);
-    }
-
-    for (uint64_t i = 0U; i < MAX_OVERLAPPING_INDEX; ++i)
-    {
-        second.emplace_back(i + OFFSET);
-    }
-
-    auto mergedContainer = uniqueMergeSortedContainers(first, second);
-    auto mergedContainerSwitched = uniqueMergeSortedContainers(second, first);
-
-    ASSERT_THAT(mergedContainer.size(), Eq(VECTOR_SIZE));
-    for (uint64_t i = 0U; i < VECTOR_SIZE; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i + OFFSET));
-    }
-    EXPECT_TRUE(mergedContainer == mergedContainerSwitched);
-}
-
-TEST_F(algorithm_test, MergeWithDisjunctOneElementContainer)
-{
-    ::testing::Test::RecordProperty("TEST_ID", "7a56b0f9-82d2-4f9a-881f-338dd572a453");
-    constexpr uint64_t OFFSET = 333331U;
-    constexpr uint64_t VECTOR_SIZE = 10U;
-    vector<uint64_t, VECTOR_SIZE> first;
-    vector<uint64_t, VECTOR_SIZE> second;
-
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        first.emplace_back(i + OFFSET);
-    }
-
-    second.emplace_back(VECTOR_SIZE / 2U + OFFSET);
-
-    auto mergedContainer = uniqueMergeSortedContainers(first, second);
-    auto mergedContainerSwitched = uniqueMergeSortedContainers(second, first);
-
-    ASSERT_THAT(mergedContainer.size(), Eq(VECTOR_SIZE / 2U + 1));
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U + 1; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i + OFFSET));
-    }
-    EXPECT_TRUE(mergedContainer == mergedContainerSwitched);
-}
-
-TEST_F(algorithm_test, MergeWithOverlappingOneElementContainer)
-{
-    ::testing::Test::RecordProperty("TEST_ID", "05fb7baf-51e9-4ff9-bb35-8ae4174b0216");
-    constexpr uint64_t OFFSET = 29292929U;
-    constexpr uint64_t VECTOR_SIZE = 10U;
-    vector<uint64_t, VECTOR_SIZE> first;
-    vector<uint64_t, VECTOR_SIZE> second;
-
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        first.emplace_back(i + OFFSET);
-    }
-
-    second.emplace_back(0 + OFFSET);
-
-    auto mergedContainer = uniqueMergeSortedContainers(first, second);
-    auto mergedContainerSwitched = uniqueMergeSortedContainers(second, first);
-
-    ASSERT_THAT(mergedContainer.size(), Eq(VECTOR_SIZE / 2U));
-    for (uint64_t i = 0U; i < VECTOR_SIZE / 2U; ++i)
-    {
-        EXPECT_THAT(mergedContainer[i], Eq(i + OFFSET));
-    }
-    EXPECT_TRUE(mergedContainer == mergedContainerSwitched);
-}
 } // namespace
diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/wait_set.inl b/iceoryx_posh/include/iceoryx_posh/internal/popo/wait_set.inl
@@ -23,6 +23,54 @@ namespace iox
 {
 namespace popo
 {
+namespace detail
+{
+inline ConditionListener::NotificationVector_t
+uniqueMergeSortedNotificationVector(const ConditionListener::NotificationVector_t& v1,
+                                    const ConditionListener::NotificationVector_t& v2) noexcept
+{
+    ConditionListener::NotificationVector_t mergedVector;
+    uint64_t indexV1{0U};
+    uint64_t indexV2{0U};
+    const uint64_t v1Size{v1.size()};
+    const uint64_t v2Size{v2.size()};
+
+    // Return value of 'emplace_back' is discarded as no overflow can happen since the notification vector stores only
+    // indices that represent active notifications
+    while ((indexV1 < v1Size) && (indexV2 < v2Size))
+    {
+        if (v1[indexV1] == v2[indexV2])
+        {
+            IOX_DISCARD_RESULT(mergedVector.emplace_back(v1[indexV1]));
+            ++indexV1;
+            ++indexV2;
+        }
+        else if (v1[indexV1] < v2[indexV2])
+        {
+            IOX_DISCARD_RESULT(mergedVector.emplace_back(v1[indexV1]));
+            ++indexV1;
+        }
+        else
+        {
+            IOX_DISCARD_RESULT(mergedVector.emplace_back(v2[indexV2]));
+            ++indexV2;
+        }
+    }
+
+    while (indexV2 < v2Size)
+    {
+        IOX_DISCARD_RESULT(mergedVector.emplace_back(v2[indexV2++]));
+    }
+
+    while (indexV1 < v1Size)
+    {
+        IOX_DISCARD_RESULT(mergedVector.emplace_back(v1[indexV1++]));
+    }
+
+    return mergedVector;
+}
+} // namespace detail
+
 template <uint64_t Capacity>
 inline WaitSet<Capacity>::WaitSet() noexcept
     : WaitSet(*runtime::PoshRuntime::getInstance().getMiddlewareConditionVariable())
@@ -317,7 +365,7 @@ inline void WaitSet<Capacity>::acquireNotifications(const WaitFunction& wait) no
     }
     else if (!notificationVector.empty())
     {
-        m_activeNotifications = algorithm::uniqueMergeSortedContainers(notificationVector, m_activeNotifications);
+        m_activeNotifications = detail::uniqueMergeSortedNotificationVector(notificationVector, m_activeNotifications);
     }
 }
 
@@ -353,7 +401,6 @@ inline constexpr uint64_t WaitSet<Capacity>::capacity() noexcept
     return Capacity;
 }
 
-
 } // namespace popo
 } // namespace iox
 

diff --git a/iceoryx_posh/include/iceoryx_posh/popo/wait_set.hpp b/iceoryx_posh/include/iceoryx_posh/popo/wait_set.hpp
@@ -46,7 +46,6 @@ enum class WaitSetError : uint8_t
     ALREADY_ATTACHED,
 };
 
-
 /// @brief Logical disjunction of a certain number of Triggers
 ///
 /// The WaitSet stores Triggers and allows the user to wait till one or more of those Triggers are triggered. It works